Explorar o código

chore: Fix many little niggly bugs, test flakes, etc

jamesread hai 2 semanas
pai
achega
f4b3c4289a

+ 3 - 0
.github/workflows/build-and-release.yml

@@ -52,12 +52,15 @@ jobs:
         if: github.event_name != 'pull_request'
         uses: actions/setup-node@v6.4.0
         with:
+          node-version: '22'
           cache: 'npm'
           cache-dependency-path: frontend/package-lock.json
 
       - name: Setup node
         if: github.event_name == 'pull_request'
         uses: actions/setup-node@v6.4.0
+        with:
+          node-version: '22'
 
       - name: Setup Go
         uses: actions/setup-go@v6

+ 6 - 0
.github/workflows/codeql-analysis.yml

@@ -57,6 +57,12 @@ jobs:
           cache: true
           cache-dependency-path: 'service/go.mod'
 
+      - name: Setup Node
+        if: matrix.language == 'javascript'
+        uses: actions/setup-node@v4
+        with:
+          node-version: '22'
+
       # Initializes the CodeQL tools for scanning.
       - name: Initialize CodeQL
         uses: github/codeql-action/init@v3

+ 5 - 0
.github/workflows/codestyle.yml

@@ -31,5 +31,10 @@ jobs:
       - name: service
         run: make -wC service codestyle
 
+      - name: Setup Node
+        uses: actions/setup-node@v4
+        with:
+          node-version: '22'
+
       - name: frontend
         run: make -wC frontend codestyle

+ 2 - 0
.gitignore

@@ -12,6 +12,8 @@ frontend/dist/
 frontend/node_modules
 custom-frontend
 integration-tests/screenshots/
+integration-tests/flakey-test-runs.log
+integration-tests/flakey-test-runs.jsonl
 .vscode/
 webui/
 server.log

+ 0 - 2
.goreleaser.yml

@@ -72,8 +72,6 @@ archives:
   - formats: tar.gz
     files:
       - config.yaml
-      - config.macos.yaml
-      - app.olivetin.olivetin.plist
       - LICENSE
       - README.md
       - src: Dockerfile.singlearch

+ 8 - 3
frontend/resources/vue/views/LogsListView.vue

@@ -115,7 +115,7 @@
 </template>
 
 <script setup>
-import { ref, onMounted, watch } from 'vue'
+import { ref, onMounted, onUnmounted, watch } from 'vue'
 import { useRoute, useRouter } from 'vue-router'
 import { ConnectError, Code } from '@connectrpc/connect'
 import Pagination from 'picocrank/vue/components/Pagination.vue'
@@ -215,8 +215,6 @@ function scheduleFetchLogs() {
 
 function clearSearch() {
   searchText.value = ''
-  currentPage.value = 1
-  fetchLogs()
 }
 
 function clearDateFilter() {
@@ -259,6 +257,13 @@ function handlePageSizeChange(newPageSize) {
 onMounted(() => {
   updateDateFromRoute()
 })
+
+onUnmounted(() => {
+  if (fetchTimer) {
+    clearTimeout(fetchTimer)
+    fetchTimer = null
+  }
+})
 </script>
 
 <style scoped>

+ 4 - 1
integration-tests/Makefile

@@ -11,6 +11,9 @@ find-flakey-tests:
 	echo "Running test-run infinately"
 	sh -c "while make test-run; do :; done"
 
+find-flakey-tests-inf: test-install
+	node scripts/find-flakey-tests-inf.mjs
+
 nginx:
 	podman-compose up -d nginx
 
@@ -21,4 +24,4 @@ getsnapshot:
 	rm -rf /opt/OliveTin-snapshot/*
 	gh run download -D /opt/OliveTin-snapshot/
 
-.PHONY: default
+.PHONY: default find-flakey-tests find-flakey-tests-inf

+ 10 - 2
integration-tests/lib/elements.js

@@ -48,14 +48,22 @@ export function takeScreenshot (webdriver, title) {
   })
 }
 
-export async function waitForDashboardLoaded(timeoutMs = DEFAULT_UI_WAIT_MS) {
+export async function waitForDashboardLoaded(timeoutMs = DEFAULT_UI_WAIT_MS, expectedTitle = null) {
   await webdriver.wait(new Condition('wait for loaded-dashboard', async function () {
     const body = await webdriver.findElement(By.tagName('body'))
     const attr = await body.getAttribute('loaded-dashboard')
 
     console.log('loaded-dashboard: ', attr)
 
-    return attr != null && attr !== ''
+    if (attr == null || attr === '') {
+      return false
+    }
+
+    if (expectedTitle != null) {
+      return attr === expectedTitle
+    }
+
+    return true
   }), timeoutMs)
 }
 

+ 44 - 31
integration-tests/package-lock.json

@@ -13,9 +13,9 @@
       },
       "devDependencies": {
         "chai": "^6.2.2",
-        "eslint": "^10.4.0",
+        "eslint": "^10.5.0",
         "mocha": "^11.7.6",
-        "selenium-webdriver": "^4.44.0"
+        "selenium-webdriver": "^4.45.0"
       }
     },
     "node_modules/@aashutoshrathi/word-wrap": {
@@ -128,9 +128,9 @@
       }
     },
     "node_modules/@eslint/plugin-kit": {
-      "version": "0.7.1",
-      "resolved": "https://registry.npmjs.org/@eslint/plugin-kit/-/plugin-kit-0.7.1.tgz",
-      "integrity": "sha512-rZAP3aVgB9ds9KOeUSL+zZ21hPmo8dh6fnIFwRQj5EAZl9gzR7wxYbYXYysAM8CTqGmUGyp2S4kUdV17MnGuWQ==",
+      "version": "0.7.2",
+      "resolved": "https://registry.npmjs.org/@eslint/plugin-kit/-/plugin-kit-0.7.2.tgz",
+      "integrity": "sha512-+CNAzxglkrpNf/kKywqQfk74QjtceuOE7Qm+AF8miRvPF/wmmK5+OJOgVh3AVTT3RP2mH3+FOaxlE5v72owk0A==",
       "dev": true,
       "license": "Apache-2.0",
       "dependencies": {
@@ -783,18 +783,21 @@
       }
     },
     "node_modules/eslint": {
-      "version": "10.4.0",
-      "resolved": "https://registry.npmjs.org/eslint/-/eslint-10.4.0.tgz",
-      "integrity": "sha512-loXy6bWOoP3EP6JA7jo6p5jMpBJmHmsNZM5SFRHLdh1MGOPurMnNBj4ZlAbaqUAaQWbCr7jHV4P7gzAyryZWkQ==",
+      "version": "10.5.0",
+      "resolved": "https://registry.npmjs.org/eslint/-/eslint-10.5.0.tgz",
+      "integrity": "sha512-1y+7C+vi12bUK1IpZeaV3gsH9fHLBmPvYmPx42pvT/E9yG0IC8g3PUZZgp0+JLJl7ZDK0flc2gc+Aw9dpCvIsQ==",
       "dev": true,
       "license": "MIT",
+      "workspaces": [
+        "packages/*"
+      ],
       "dependencies": {
         "@eslint-community/eslint-utils": "^4.8.0",
         "@eslint-community/regexpp": "^4.12.2",
         "@eslint/config-array": "^0.23.5",
         "@eslint/config-helpers": "^0.6.0",
         "@eslint/core": "^1.2.1",
-        "@eslint/plugin-kit": "^0.7.1",
+        "@eslint/plugin-kit": "^0.7.2",
         "@humanfs/node": "^0.16.6",
         "@humanwhocodes/module-importer": "^1.0.1",
         "@humanwhocodes/retry": "^0.4.2",
@@ -1051,16 +1054,16 @@
       }
     },
     "node_modules/form-data": {
-      "version": "4.0.5",
-      "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.5.tgz",
-      "integrity": "sha512-8RipRLol37bNs2bhoV67fiTEvdTrbMUYcFTiy3+wuuOnUog2QBHCZWXDRijWQfAkhBj2Uf5UnVaiWwA5vdd82w==",
+      "version": "4.0.6",
+      "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.6.tgz",
+      "integrity": "sha512-vKatAh4SlVfgbv+YtmhiRjhEMJsYpsG1Y2rMQtR+SVSbytsSD1YGzDIcrAJmdFec88u/+VoGmxnl+80gL1tRCQ==",
       "license": "MIT",
       "dependencies": {
         "asynckit": "^0.4.0",
         "combined-stream": "^1.0.8",
         "es-set-tostringtag": "^2.1.0",
-        "hasown": "^2.0.2",
-        "mime-types": "^2.1.12"
+        "hasown": "^2.0.4",
+        "mime-types": "^2.1.35"
       },
       "engines": {
         "node": ">= 6"
@@ -1230,9 +1233,9 @@
       }
     },
     "node_modules/hasown": {
-      "version": "2.0.3",
-      "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.3.tgz",
-      "integrity": "sha512-ej4AhfhfL2Q2zpMmLo7U1Uv9+PyhIZpgQLGT1F9miIGmiCJIoCgSmczFdrc97mWT4kVY72KA+WnnhJ5pghSvSg==",
+      "version": "2.0.4",
+      "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.4.tgz",
+      "integrity": "sha512-T2UbfbBEF32wiepXIsMlTW9+dDYC6wMh/t/vYA4tuOMKqWz/n3vr1NFSxQiyP+zk2mXsoMA/i/7qV6LKut1t1A==",
       "license": "MIT",
       "dependencies": {
         "function-bind": "^1.1.2"
@@ -1390,10 +1393,20 @@
       }
     },
     "node_modules/js-yaml": {
-      "version": "4.1.1",
-      "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz",
-      "integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==",
+      "version": "4.2.0",
+      "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.2.0.tgz",
+      "integrity": "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw==",
       "dev": true,
+      "funding": [
+        {
+          "type": "github",
+          "url": "https://github.com/sponsors/puzrin"
+        },
+        {
+          "type": "github",
+          "url": "https://github.com/sponsors/nodeca"
+        }
+      ],
       "license": "MIT",
       "dependencies": {
         "argparse": "^2.0.1"
@@ -1866,9 +1879,9 @@
       "dev": true
     },
     "node_modules/selenium-webdriver": {
-      "version": "4.44.0",
-      "resolved": "https://registry.npmjs.org/selenium-webdriver/-/selenium-webdriver-4.44.0.tgz",
-      "integrity": "sha512-7RbYoKK0zET+KMVak11UDCtKvNulOU6gFZp8HI5GN9K8+BhqrliIJU/FP6QADrvRAXFMr3wHxfE3JHOcAxO3GQ==",
+      "version": "4.45.0",
+      "resolved": "https://registry.npmjs.org/selenium-webdriver/-/selenium-webdriver-4.45.0.tgz",
+      "integrity": "sha512-Cb2nqvJiwXVOtRTCYHX9D1FJR5+Ls7aL3Nev0t6n4CpXsQ//YGiiUmSCbvTDDeLtbV85SZ46qmLab4SIYKXWRw==",
       "dev": true,
       "funding": [
         {
@@ -1884,8 +1897,8 @@
       "dependencies": {
         "@bazel/runfiles": "^6.5.0",
         "jszip": "^3.10.1",
-        "tmp": "^0.2.5",
-        "ws": "^8.20.1"
+        "tmp": "^0.2.7",
+        "ws": "^8.21.0"
       },
       "engines": {
         "node": ">= 20.0.0"
@@ -2079,9 +2092,9 @@
       }
     },
     "node_modules/tmp": {
-      "version": "0.2.5",
-      "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.2.5.tgz",
-      "integrity": "sha512-voyz6MApa1rQGUxT3E+BK7/ROe8itEx7vD8/HEvt4xwXucvQ5G5oeEiHkmHZJuBO21RpOf+YYm9MOivj709jow==",
+      "version": "0.2.7",
+      "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.2.7.tgz",
+      "integrity": "sha512-e0votIpp4Uo2AJYSzVHV6xCcawuiez3DzqDAbrTc3YxBkplN6e+dM13ZeIcZnDg/QpSuU2zfZ3rzwY8ukEnaXw==",
       "dev": true,
       "license": "MIT",
       "engines": {
@@ -2259,9 +2272,9 @@
       }
     },
     "node_modules/ws": {
-      "version": "8.20.1",
-      "resolved": "https://registry.npmjs.org/ws/-/ws-8.20.1.tgz",
-      "integrity": "sha512-It4dO0K5v//JtTXuPkfEOaI3uUN87iYPnqo/ZzqCoG3g8uhA66QUMs/SrM0YK7/NAu+r4LMh/9dq2A7k+rHs+w==",
+      "version": "8.21.0",
+      "resolved": "https://registry.npmjs.org/ws/-/ws-8.21.0.tgz",
+      "integrity": "sha512-Vsp28b7DRcimFQvrqu2Wek3z1iYxDCWqHYB8Qsnk/S4RfaCQzPGPyBNuVjJV3cd6UiKtUtp6sNM77gWvzcCH+g==",
       "dev": true,
       "license": "MIT",
       "engines": {

+ 2 - 2
integration-tests/package.json

@@ -12,9 +12,9 @@
   "license": "AGPL-3.0-only",
   "devDependencies": {
     "chai": "^6.2.2",
-    "eslint": "^10.4.0",
+    "eslint": "^10.5.0",
     "mocha": "^11.7.6",
-    "selenium-webdriver": "^4.44.0"
+    "selenium-webdriver": "^4.45.0"
   },
   "dependencies": {
     "wait-on": "^9.0.10"

+ 164 - 0
integration-tests/scripts/find-flakey-tests-inf.mjs

@@ -0,0 +1,164 @@
+#!/usr/bin/env node
+import { spawn } from 'node:child_process'
+import {
+  appendFileSync,
+  writeFileSync,
+  readFileSync,
+  unlinkSync,
+} from 'node:fs'
+import { dirname, join } from 'node:path'
+import { fileURLToPath } from 'node:url'
+import { tmpdir } from 'node:os'
+import { randomUUID } from 'node:crypto'
+
+const rootDir = join(dirname(fileURLToPath(import.meta.url)), '..')
+const logFile = process.env.FLAKEY_LOG_FILE || join(rootDir, 'flakey-test-runs.log')
+const jsonlFile = process.env.FLAKEY_JSONL_FILE || join(rootDir, 'flakey-test-runs.jsonl')
+
+function formatFailure (failure) {
+  const err = failure.err || {}
+  const lines = [
+    `FAILURE: ${failure.fullTitle || failure.title}`,
+    `  file: ${failure.file || 'unknown'}`,
+    `  message: ${(err.message || 'unknown').trim()}`,
+  ]
+
+  if (err.stack) {
+    lines.push('  stack:')
+    for (const line of err.stack.split('\n').slice(0, 8)) {
+      lines.push(`    ${line}`)
+    }
+  }
+
+  return lines.join('\n')
+}
+
+function appendRunLog (run, exitCode, report, durationMs, spawnError) {
+  const timestamp = new Date().toISOString()
+  const stats = report?.stats || {}
+  const passes = stats.passes ?? '?'
+  const failures = stats.failures ?? '?'
+  const pending = stats.pending ?? 0
+  const passed = exitCode === 0 && !spawnError
+  const result = passed ? 'PASS' : 'FAIL'
+  const durationSec = (durationMs / 1000).toFixed(1)
+
+  const block = [
+    `=== RUN ${run} | ${timestamp} | ${result} | ${passes} pass ${failures} fail ${pending} pending | ${durationSec}s ===`,
+  ]
+
+  if (spawnError) {
+    block.push(`SPAWN_ERROR: ${spawnError}`)
+  }
+
+  if (report?.failures?.length) {
+    for (const failure of report.failures) {
+      block.push(formatFailure(failure))
+    }
+  } else if (!passed && !report) {
+    block.push('No JSON report captured (mocha may have crashed before writing results)')
+  }
+
+  block.push('')
+  appendFileSync(logFile, `${block.join('\n')}\n`)
+
+  const jsonl = {
+    run,
+    timestamp,
+    exitCode,
+    durationMs,
+    passes,
+    failures,
+    pending,
+    failureDetails: (report?.failures || []).map((failure) => ({
+      fullTitle: failure.fullTitle || failure.title,
+      file: failure.file,
+      message: failure.err?.message,
+      stack: failure.err?.stack,
+    })),
+  }
+  appendFileSync(jsonlFile, `${JSON.stringify(jsonl)}\n`)
+}
+
+function runMochaOnce () {
+  const reportPath = join(tmpdir(), `mocha-flakey-${randomUUID()}.json`)
+
+  return new Promise((resolve) => {
+    const proc = spawn('npx', [
+      'mocha',
+      'tests',
+      '--recursive',
+      '-t',
+      '10000',
+      '--reporter',
+      'json',
+      '--reporter-option',
+      `output=${reportPath}`,
+    ], {
+      cwd: rootDir,
+      stdio: ['ignore', 'inherit', 'inherit'],
+    })
+
+    proc.on('close', (exitCode) => {
+      let report = null
+      try {
+        report = JSON.parse(readFileSync(reportPath, 'utf8'))
+      } catch {
+        report = null
+      }
+
+      try {
+        unlinkSync(reportPath)
+      } catch {
+        // ignore missing temp report
+      }
+
+      resolve({ exitCode: exitCode ?? 1, report })
+    })
+
+    proc.on('error', (spawnError) => {
+      resolve({ exitCode: 1, report: null, spawnError: spawnError.message })
+    })
+  })
+}
+
+async function main () {
+  const header = [
+    `# Flaky test run log started ${new Date().toISOString()}`,
+    `# Log file: ${logFile}`,
+    `# JSONL file: ${jsonlFile}`,
+    '',
+  ].join('\n')
+
+  writeFileSync(logFile, `${header}\n`)
+  writeFileSync(jsonlFile, '')
+
+  console.log(`Logging flaky test runs to ${logFile}`)
+  console.log(`Structured run data: ${jsonlFile}`)
+
+  let run = 0
+
+  while (true) {
+    run += 1
+    console.log(`\n--- Starting run ${run} ---`)
+
+    const start = Date.now()
+    const { exitCode, report, spawnError } = await runMochaOnce()
+    const durationMs = Date.now() - start
+
+    appendRunLog(run, exitCode, report, durationMs, spawnError)
+
+    const summary = exitCode === 0 ? 'PASS' : 'FAIL'
+    console.log(`Run ${run}: ${summary} (${(durationMs / 1000).toFixed(1)}s) — logged`)
+
+    if (exitCode !== 0) {
+      console.log(`Failure on run ${run}, stopping. See ${logFile}`)
+      process.exit(exitCode)
+    }
+  }
+}
+
+main().catch((err) => {
+  console.error(err)
+  process.exit(1)
+})

+ 1 - 1
integration-tests/tests/general/general.mjs

@@ -23,7 +23,7 @@ describe('config: general', function () {
   });
 
   it('Page title', async function () {
-    await webdriver.get(runner.baseUrl())
+    await getRootAndWait()
 
     const title = await webdriver.getTitle()
     expect(title).to.be.equal("Actions - OliveTin")

+ 3 - 0
integration-tests/tests/multi-dashboard-includes/multi-dashboard-includes.mjs

@@ -2,11 +2,13 @@ import { describe, it, before, after } from 'mocha'
 import { expect, assert } from 'chai'
 import { By } from 'selenium-webdriver'
 import {
+  DEFAULT_UI_WAIT_MS,
   getRootAndWait,
   getActionButtons,
   getNavigationLinks,
   openSidebar,
   takeScreenshotOnFailure,
+  waitForDashboardLoaded,
 } from '../../lib/elements.js'
 
 describe('config: multi-dashboard-includes', function () {
@@ -41,6 +43,7 @@ describe('config: multi-dashboard-includes', function () {
     assert.strictEqual(matching.length, 1, `Expected exactly one navigation link with title "${title}"`)
 
     await matching[0].click()
+    await waitForDashboardLoaded(DEFAULT_UI_WAIT_MS, title)
   }
 
   async function getActionTitlesOnDashboard (dashboardTitle = null) {

+ 17 - 0
service/internal/executor/executor.go

@@ -631,6 +631,23 @@ func (e *Executor) registerOrQueueRequest(req *ExecutionRequest, wg *sync.WaitGr
 		return true, false
 	}
 
+	if e.finishIfConcurrencyBlocked(req) {
+		return true, false
+	}
+
+	return e.queueRequestIfGroupLimited(req, wg)
+}
+
+func (e *Executor) finishIfConcurrencyBlocked(req *ExecutionRequest) bool {
+	if stepConcurrencyCheck(req) {
+		return false
+	}
+
+	e.finishExecChain(req)
+	return true
+}
+
+func (e *Executor) queueRequestIfGroupLimited(req *ExecutionRequest, wg *sync.WaitGroup) (finished bool, queued bool) {
 	if !actionNeedsGroupLimit(req) || e.groupsHaveCapacityForActive(req) {
 		return false, false
 	}

+ 41 - 0
service/internal/executor/group_concurrency_test.go

@@ -163,6 +163,47 @@ func TestPerActionConcurrencyStillBlocksWithoutQueue(t *testing.T) {
 	assert.False(t, snapshot.Queued)
 }
 
+func TestPerActionConcurrencyBlocksSameBindingBeforeGroupQueue(t *testing.T) {
+	t.Parallel()
+
+	action := &config.Action{
+		Title:         "Single binding grouped",
+		Shell:         "sleep 1",
+		MaxConcurrent: 1,
+		Groups:        []string{"unity"},
+	}
+
+	e, cfg := testGroupExecutor(
+		[]*config.Action{action},
+		map[string]*config.ActionGroup{
+			"unity": {MaxConcurrent: 1},
+		},
+	)
+	binding := e.FindBindingWithNoEntity(action)
+
+	wg1, tracking1 := e.ExecRequest(&ExecutionRequest{
+		Binding:           binding,
+		Cfg:               cfg,
+		AuthenticatedUser: auth.UserFromSystem(cfg, "testuser"),
+	})
+
+	waitUntilExecutionStarted(t, e, tracking1)
+
+	wg2, tracking2 := e.ExecRequest(&ExecutionRequest{
+		Binding:           binding,
+		Cfg:               cfg,
+		AuthenticatedUser: auth.UserFromSystem(cfg, "testuser"),
+	})
+
+	wg1.Wait()
+	wg2.Wait()
+
+	snapshot, ok := e.SnapshotLog(tracking2)
+	require.True(t, ok)
+	assert.True(t, snapshot.Blocked)
+	assert.False(t, snapshot.Queued)
+}
+
 func waitUntilExecutionStarted(t *testing.T, e *Executor, trackingID string) {
 	t.Helper()
 

+ 16 - 4
service/internal/executor/logfilter.go

@@ -31,13 +31,25 @@ func applyLogFilter(entries []*InternalLogEntry, program *vm.Program) ([]*Intern
 func filterEntries(entries []*InternalLogEntry, program *vm.Program) ([]*InternalLogEntry, error) {
 	filtered := make([]*InternalLogEntry, 0, len(entries))
 	for _, entry := range entries {
-		matched, err := entryMatchesFilter(entry, program)
+		var err error
+		filtered, err = appendMatchingEntry(filtered, entry, program)
 		if err != nil {
 			return nil, err
 		}
-		if matched {
-			filtered = append(filtered, entry)
-		}
+	}
+	return filtered, nil
+}
+
+func appendMatchingEntry(filtered []*InternalLogEntry, entry *InternalLogEntry, program *vm.Program) ([]*InternalLogEntry, error) {
+	if entry == nil {
+		return nil, fmt.Errorf("log entry is nil")
+	}
+	matched, err := entryMatchesFilter(entry, program)
+	if err != nil {
+		return nil, err
+	}
+	if matched {
+		filtered = append(filtered, entry)
 	}
 	return filtered, nil
 }

+ 17 - 0
service/internal/executor/logfilter_test.go

@@ -0,0 +1,17 @@
+package executor
+
+import (
+	"testing"
+
+	"github.com/OliveTin/OliveTin/internal/logfilter"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFilterEntriesRejectsNilEntry(t *testing.T) {
+	program, err := logfilter.Compile(`Status == Completed`)
+	require.NoError(t, err)
+
+	_, err = filterEntries([]*InternalLogEntry{nil}, program)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "log entry is nil")
+}

+ 44 - 6
service/internal/logfilter/filter.go

@@ -3,6 +3,7 @@ package logfilter
 import (
 	"fmt"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/expr-lang/expr"
@@ -14,6 +15,17 @@ const maxFilterLength = 512
 var (
 	comparePattern  = regexp.MustCompile(`(?i)\b(Status|Action|User|ExitCode|Blocked|TimedOut|Running)\s*(==|!=)\s*("[^"]*"|\S+)`)
 	containsPattern = regexp.MustCompile(`(?i)\b(Status|Action|User|Output)\s+contains\s+("[^"]*"|\S+)`)
+
+	fieldNameByLower = map[string]string{
+		"status":   "Status",
+		"action":   "Action",
+		"user":     "User",
+		"exitcode": "ExitCode",
+		"blocked":  "Blocked",
+		"timedout": "TimedOut",
+		"running":  "Running",
+		"output":   "Output",
+	}
 )
 
 // Compile parses and compiles a filter expression. Returns an error for invalid syntax.
@@ -44,25 +56,31 @@ func compileNormalized(normalized string) (*vm.Program, error) {
 }
 
 func includes(params ...any) (any, error) {
+	if len(params) < 2 {
+		return nil, fmt.Errorf("includes expects 2 arguments, got %d", len(params))
+	}
 	haystack, ok := params[0].(string)
 	if !ok {
-		return false, nil
+		return nil, fmt.Errorf("expected string for haystack")
 	}
 	needle, ok := params[1].(string)
 	if !ok {
-		return false, nil
+		return nil, fmt.Errorf("expected string for needle")
 	}
 	return strings.Contains(strings.ToLower(haystack), strings.ToLower(needle)), nil
 }
 
 func hasTag(params ...any) (any, error) {
+	if len(params) < 2 {
+		return nil, fmt.Errorf("hasTag expects 2 arguments, got %d", len(params))
+	}
 	tags, ok := params[0].([]string)
 	if !ok {
-		return false, nil
+		return nil, fmt.Errorf("expected []string for tags")
 	}
 	needle, ok := params[1].(string)
 	if !ok {
-		return false, nil
+		return nil, fmt.Errorf("expected string for needle")
 	}
 	return tagListIncludes(tags, needle), nil
 }
@@ -145,7 +163,7 @@ func positiveSearchExpression(term string) string {
 func replaceContainsOperators(expression string) string {
 	return containsPattern.ReplaceAllStringFunc(expression, func(match string) string {
 		parts := containsPattern.FindStringSubmatch(match)
-		field := parts[1]
+		field := normalizeFieldName(parts[1])
 		value := quoteIfNeeded(parts[2])
 		return fmt.Sprintf("includes(%s, %s)", field, value)
 	})
@@ -154,13 +172,20 @@ func replaceContainsOperators(expression string) string {
 func replaceComparisons(expression string) string {
 	return comparePattern.ReplaceAllStringFunc(expression, func(match string) string {
 		parts := comparePattern.FindStringSubmatch(match)
-		field := parts[1]
+		field := normalizeFieldName(parts[1])
 		operator := parts[2]
 		value := quoteIfNeeded(parts[3])
 		return fmt.Sprintf("%s %s %s", field, operator, value)
 	})
 }
 
+func normalizeFieldName(field string) string {
+	if canonical, ok := fieldNameByLower[strings.ToLower(field)]; ok {
+		return canonical
+	}
+	return field
+}
+
 func replaceBooleanWords(expression string) string {
 	replacer := strings.NewReplacer(" and ", " && ", " AND ", " && ", " or ", " || ", " OR ", " || ")
 	return replacer.Replace(expression)
@@ -170,9 +195,22 @@ func quoteIfNeeded(value string) string {
 	if strings.HasPrefix(value, "\"") {
 		return value
 	}
+	if isBooleanLiteral(value) || isIntegerLiteral(value) {
+		return strings.ToLower(value)
+	}
 	return quoteLiteral(value)
 }
 
+func isBooleanLiteral(value string) bool {
+	lower := strings.ToLower(value)
+	return lower == "true" || lower == "false"
+}
+
+func isIntegerLiteral(value string) bool {
+	_, err := strconv.ParseInt(value, 10, 64)
+	return err == nil
+}
+
 func quoteLiteral(value string) string {
 	return "\"" + strings.ReplaceAll(value, "\"", "\\\"") + "\""
 }

+ 56 - 0
service/internal/logfilter/filter_test.go

@@ -32,6 +32,27 @@ func TestCompileContainsAndBooleanWords(t *testing.T) {
 	assert.False(t, mustMatch(t, program, Record{Status: "Blocked", Action: "Nightly backup"}))
 }
 
+func TestCompileNormalizesFieldNamesAndValueTypes(t *testing.T) {
+	cases := []struct {
+		expression string
+		record     Record
+		want       bool
+	}{
+		{"status == completed", Record{Status: "completed"}, true},
+		{"ExitCode == 0", Record{ExitCode: 0}, true},
+		{"exitcode == 0", Record{ExitCode: 0}, true},
+		{"Blocked == true", Record{Blocked: true}, true},
+		{"blocked == false", Record{Blocked: false}, true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.expression, func(t *testing.T) {
+			program, err := Compile(tc.expression)
+			require.NoError(t, err)
+			assert.Equal(t, tc.want, mustMatch(t, program, tc.record))
+		})
+	}
+}
+
 func TestCompileRejectsOverlongExpression(t *testing.T) {
 	_, err := Compile(string(make([]byte, maxFilterLength+1)))
 	require.Error(t, err)
@@ -42,6 +63,41 @@ func TestCompileRejectsUnknownField(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestIncludesReturnsErrorsForMalformedArguments(t *testing.T) {
+	_, err := includes("only-one")
+	require.Error(t, err)
+
+	_, err = includes(123, "needle")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "haystack")
+
+	_, err = includes("hay", 123)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "needle")
+}
+
+func TestHasTagReturnsErrorsForMalformedArguments(t *testing.T) {
+	_, err := hasTag("only-one")
+	require.Error(t, err)
+
+	_, err = hasTag("not-tags", "x")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "tags")
+
+	_, err = hasTag([]string{"a"}, 123)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "needle")
+}
+
+func TestMatchesSurfacesIncludesTypeErrors(t *testing.T) {
+	program, err := Compile(`Action contains 123`)
+	require.NoError(t, err)
+
+	_, err = Matches(program, Record{Action: "test 123"})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "needle")
+}
+
 func mustMatch(t *testing.T, program *vm.Program, record Record) bool {
 	t.Helper()
 	matched, err := Matches(program, record)