From 0c3b73c562b181b054356fea05dc6db40b9db625 Mon Sep 17 00:00:00 2001 From: developer-agent Date: Sat, 29 Aug 2026 08:20:52 -0400 Subject: [PATCH 1/3] Close the plugin's task-queue vocabulary drift, then gate it (vikunja#558, #543) The plugin carried four partial hand-written copies of a vocabulary it does not own -- task-queue-mcp's src/tools/queue.py. None of them learned about `routing-failed` when the MCP made it a first-class non-terminal status, so the status most in need of an operator sorted below `cancelled`, rendered the same grey as `parked`, and was not offered by the status filter. `manual-then-auto` was the same omission one field over. Fixed - routing-failed sorts above in-progress, renders in `error`, is filterable, and is a valid destination in the status-change control. Setting it by hand is not a dead end: the dispatcher's routing-failed pass picks up any such task whose retry window has passed, and a record with no next_retry_at is eligible immediately -- so it reads as "re-route now, skipping re-approval". - The detail view gains a banner saying what routing-failed means, how many retries are used, when the next attempt is due, and that the end of the budget is a dead letter. - A queued `manual-then-auto` now survives the Start button. toWorkflowMode collapsed it to `semi-auto`, re-pinning every task the session spawns back to `semi-auto` -- the exact failure vikunja#533 added the mode to fix. Added - src/vocabulary.ts: one copy of the vocabulary plus the UI maps keyed BY it. STATUS_ORDER and STATUS_COLOR are Record, so adding a status without a sort position and a colour is a tsc error rather than a silent fallthrough to `?? 9` and `muted`. That is the half the old code lacked -- Record accepts anything and covers nothing. - npm run gate:vocabulary, as its own CI step. Ported from task-dispatcher's test_task_queue_vocabulary.py (vikunja#324) keeping both properties that make it work: no skip-on-no-network path, and it tracks `main` not a pin. Proven to fire before shipping -- exit 0 in sync, 1 on real drift against an older upstream ref, 2 with no network, 2 on a traversal-shaped ref. - A Mode filter and a mode badge. A task with no recorded workflow_mode renders as unknown, not as the queue's default. Changed - The two NON_TERMINAL_STATUSES literals are one derived constant. - Relative imports use .ts, not .js: node --test runs the sources directly and does not rewrite a .js specifier to the .ts beside it, so the panels were not importable by a test. 110 tests pass; each new assertion verified red against the defect it covers. Co-Authored-By: Claude Opus 5 (1M context) agent-id: developer --- .github/workflows/ci.yml | 11 ++ AGENTS.md | 25 +++- CHANGELOG.md | 69 +++++++++++ README.md | 15 ++- manifest.json | 2 +- package.json | 5 +- src/dead-letters.ts | 2 +- src/gates/python-sets.ts | 74 ++++++++++++ src/gates/vocabulary-parity.ts | 206 +++++++++++++++++++++++++++++++++ src/index.ts | 18 +-- src/launch-policy.ts | 34 +++++- src/panels/dead-letters.ts | 6 +- src/panels/headless-runs.ts | 4 +- src/panels/styles.ts | 26 +++-- src/panels/task-detail.ts | 52 +++++++-- src/panels/task-list.ts | 98 ++++++++++------ src/server.ts | 25 +++- src/tests/python-sets.test.ts | 110 ++++++++++++++++++ src/tests/vocabulary.test.ts | 142 +++++++++++++++++++++++ src/types.ts | 17 +++ src/vocabulary.ts | 156 +++++++++++++++++++++++++ 21 files changed, 1011 insertions(+), 86 deletions(-) create mode 100644 src/gates/python-sets.ts create mode 100644 src/gates/vocabulary-parity.ts create mode 100644 src/tests/python-sets.test.ts create mode 100644 src/tests/vocabulary.test.ts create mode 100644 src/vocabulary.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9c86b9..1d48a6a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,17 @@ jobs: - name: Test run: npm test + # Its own step, deliberately, and after the unit tests. A red vocabulary means "go + # edit src/vocabulary.ts" — a different instruction from any unit failure — and + # folding the two into one command lets either hide the other. It reaches the + # network on purpose and has no skip path; see src/gates/vocabulary-parity.ts. + # + # Do NOT set TASK_QUEUE_MCP_REF here. It defaults to task-queue-mcp's `main`, which + # is the whole point: pinning it to anything else asserts parity with something + # nobody is running. + - name: Vocabulary parity with task-queue-mcp main + run: npm run gate:vocabulary + # Production dependencies only. The dev tree carries advisories that do not # apply to a plugin bundled at build time and never served by a dev server. - name: Audit production dependencies diff --git a/AGENTS.md b/AGENTS.md index 70f5d18..d9d7226 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,6 +33,9 @@ src/ launchLogName, fenced-block extraction, run timestamps. launch-policy.ts Reads and validates the shared launch roster, and builds the spawn argv. Same extraction reason. + vocabulary.ts The task-queue vocabulary (statuses, task types, workflow + modes) plus the UI maps keyed BY it. One copy, gated + against task-queue-mcp's main. See the invariant below. types.ts Shared types. The Task shape mirrors task-queue-mcp's YAML schema — keep them in step. panels/ @@ -42,13 +45,29 @@ src/ dead-letters.ts Collapsed dead-letter section, grouped by failure reason styles.ts Theme colours (read live from CloudCLI CSS vars), helpers ws-client.ts Reconnecting WebSocket client + gates/ + python-sets.ts Extracts `NAME = {"a","b"}` literals from Python source, + without executing any. Split out of the gate script so it + can be tested; that script exits at module scope. + vocabulary-parity.ts CI gate: fetches task-queue-mcp `main` and asserts the + vocabulary matches. Own npm script, own CI step. tests/ node --test ``` +**Relative imports use `.ts`, not `.js`.** `npm test` runs the sources directly under Node's +type stripping, which does not rewrite a `.js` specifier to the `.ts` file beside it — so a +module reached by a test through a `.js` import fails with `ERR_MODULE_NOT_FOUND`. The panels +all used `.js` and were simply never imported by a test; the first one that was, was not +importable. esbuild and `tsc` (`allowImportingTsExtensions`) both accept `.ts`. + ## Invariants - **The plugin never writes queue YAML directly.** Reads are direct (fast, watchable); every mutation goes through `control-api.ts` to the MCP control API, inheriting its transition validation, `fcntl` locking, and atomic writes. A new mutation means a new control-API action, never an `fs.writeFile`. - **`ControlAction` must match the MCP's route set.** The union type in `control-api.ts`, the route regex in `server.ts`, and the MCP's custom routes are three copies of one contract. Change one, change all three. The two copies that live in *this* repo are now pinned to each other by a source-level test in `control-api.test.ts` — nothing detected the drift before, because adding an action to the union alone compiles and the failure mode is a button that 404s against the plugin's own backend. The third copy is in another repo and still needs a human. +- **The task-queue vocabulary lives in `vocabulary.ts`, once, and is gated against its owner.** This plugin does not own the queue's statuses, task types, or workflow modes — `task-queue-mcp`'s `src/tools/queue.py` does. It used to carry four partial hand-written copies (`STATUS_ORDER`, `NON_TERMINAL_STATUSES`, `DETAIL_NON_TERMINAL_STATUSES`, the `statusColor` switch); none of them learned about `routing-failed`, so for months the status most in need of an operator sorted *below* `cancelled`, rendered the same grey as `parked`, and was not offered by the status filter. `manual-then-auto` was the same omission one field over. Two mechanisms hold the line and they are different in kind: `npm run gate:vocabulary` fetches the MCP's `main` and fails on any difference, which catches "upstream changed and we did not"; and the UI maps are `Record` keyed by the vocabulary itself, so adding a status without giving it a sort position and a colour is a `tsc` error rather than a silent fallthrough to `?? 9` and `muted`. Do not weaken either — a `Record` accepts anything and covers nothing, which is precisely how this happened. +- **The vocabulary gate has no skip-on-no-network path, and tracks `main`, not a pin.** It exits non-zero if it cannot read the upstream. A check that quietly passes when it could not reach its source of truth is indistinguishable from one that verified something, and that shape is how the dispatcher-side instance (vikunja#324) stayed open for months. It likewise exits non-zero if it parses zero set literals, rather than reporting a vacuous pass over an empty comparison. A vocabulary change merged upstream turning this repo's CI red is the alarm working. `TASK_QUEUE_MCP_REF` exists only for a paired pre-merge change from a shell; **CI must not set it**. +- **The gate is its own CI step.** A red vocabulary means "go edit `src/vocabulary.ts`" — a different instruction from any unit failure. Folded into `npm test` either could hide the other, and an expected-red check buried inside a longer step masks whether anything before it ran. +- **A queued `manual-then-auto` survives the Start button.** `toWorkflowMode` maps the UI's `review` to `semi-auto` *unless* the task was queued `manual-then-auto`, in which case it passes that through. Both gate this leg; only the second lets the tasks the session spawns run `auto`. Flattening it — which is what this did before #543 — re-pins the whole downstream chain to `semi-auto`, which is the failure vikunja#533 added the mode to fix. An explicit `auto` Start still overrides. - **The plugin acts as `operator`, never as an agent.** Every proxied mutation sends `actor: 'operator'`. This is what makes the MCP's `amend_task` authorization accept it; the plugin must never assert an agent's identity. - **Mutations fail closed without the secret.** `callControlApi` returns 500 and never attempts the fetch when `TASK_QUEUE_API_SECRET` is empty, and logs why. Do not add a fallback that proceeds without it. - **The version comes from `package.json`.** `server.ts` reads it at startup rather than hardcoding a copy — a hardcoded constant silently drifted and reported a stale version on `/health` for two releases. `package.json` and `manifest.json` must also agree. @@ -72,14 +91,16 @@ src/ ```bash npm install -npm run build # tsc --noEmit is the typecheck gate; esbuild bundles after it passes +npm run build # tsc --noEmit is the typecheck gate; esbuild bundles after it passes npm test +npm run gate:vocabulary # parity with task-queue-mcp main — needs network, by design ``` -Tests cover `control-api.ts` (the secret gate, task-id validation, header and body shape per action, transport-failure mapping, pass-through of the MCP's authorization rejections, and the union/route-regex drift gate), `dead-letters.ts` (the real dispatcher record shape including the `Date`-valued `created` js-yaml hands back, the id-less and reason-less fallbacks, and the grouping rule against the live seventeen-identical-reasons case), `ws-guard.ts` (all three upgrade cases, including the loopback-with-no-Origin one that v0.4.0 broke), `launch-policy.ts` (every closed-set rejection, whole-document rejection, and both argv shapes), `path-guard.ts` (a **real** symlink escape, traversal, the `/comms-other` sibling case — with real files in a tmpdir, because a mocked `fs` cannot demonstrate that realpath runs first), `launch-log.ts` (round-trip against the real `launchLogName`, the live bare-UUID orphans, path-shaped route ids, fence extraction including the dropped unterminated case, and the birthtime-after-mtime fallback), and the reconnect schedule. The UI panels are not unit-tested — verify them in CloudCLI after `./deploy.sh && pm2 restart cloudcli`. +Tests cover `control-api.ts` (the secret gate, task-id validation, header and body shape per action, transport-failure mapping, pass-through of the MCP's authorization rejections, and the union/route-regex drift gate), `dead-letters.ts` (the real dispatcher record shape including the `Date`-valued `created` js-yaml hands back, the id-less and reason-less fallbacks, and the grouping rule against the live seventeen-identical-reasons case), `ws-guard.ts` (all three upgrade cases, including the loopback-with-no-Origin one that v0.4.0 broke), `launch-policy.ts` (every closed-set rejection, whole-document rejection, and both argv shapes), `path-guard.ts` (a **real** symlink escape, traversal, the `/comms-other` sibling case — with real files in a tmpdir, because a mocked `fs` cannot demonstrate that realpath runs first), `launch-log.ts` (round-trip against the real `launchLogName`, the live bare-UUID orphans, path-shaped route ids, fence extraction including the dropped unterminated case, and the birthtime-after-mtime fallback), `vocabulary.ts` (every status has a sort position and a colour, `routing-failed` sorts above `in-progress` and is not muted, the derived non-terminal set, and the `manual-then-auto` pass-through through `buildLaunchArgv`), `gates/python-sets.ts` (the real `queue.py` shape including interleaved comments, and the two constructs that must NOT parse as string sets — a derived set and an annotated dict), and the reconnect schedule. The UI panels are not otherwise unit-tested — verify them in CloudCLI after `./deploy.sh && pm2 restart cloudcli`. Two build/test gotchas worth knowing before you touch either script: +- **`npm run gate:vocabulary` reaches the network and has no offline mode.** It is not part of `npm test` — run it separately, and expect it to fail on a disconnected machine. That is the design, not a bug; see the invariant above. - **`npm test` needs Node 22.18+.** `node --test` runs the `.ts` test files directly, relying on Node's built-in type stripping. On Node 20 every test fails with `ERR_UNKNOWN_FILE_EXTENSION`. The plugin's *runtime* requirement is still Node 20+ — `dist/` is bundled plain JS — so the README's stated minimum and CI's Node version differ on purpose. - **`npm run build` invokes `tsc`/`esbuild` from `node_modules/.bin`, not via bare `npx`.** `npx tsc` silently downloads an unrelated registry package named `tsc` when devDependencies are missing, replacing the typecheck gate with a stranger's binary. Do not "simplify" it back to `npx`. diff --git a/CHANGELOG.md b/CHANGELOG.md index f09b229..4432a34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,75 @@ All notable changes to this project will be documented in this file. +## [0.8.0] - 2026-08-29 + +Closes the plugin's task-queue vocabulary drift, then gates it so it cannot silently +reopen. Build `agent-workflow-interop-2026-08` Phase 2; vikunja#558, folding in #543. + +### The drift + +`task-queue-mcp` made `routing-failed` a first-class non-terminal status and counts it in +`active` on `GET /queue/summary`. The plugin carried four partial hand-written copies of a +vocabulary it does not own, and none of them were updated: + +| Site | Effect | +|---|---| +| `panels/task-list.ts` `STATUS_ORDER` | no entry, fell through `?? 9` — a retrying, failing task sorted **below `cancelled`**, dead last | +| `panels/task-list.ts` `NON_TERMINAL_STATUSES` | the status filter never offered the value, and the status-change control could not move a task into it | +| `panels/task-detail.ts` `DETAIL_NON_TERMINAL_STATUSES` | second copy of the same list, stale the same way | +| `panels/styles.ts` `statusColor` | no `case`, so `default: muted` — identical to `parked` and `cancelled`, i.e. "not urgent" | + +The status most in need of an operator was the one that sorted last and looked least +urgent. `manual-then-auto` (#543) was the same omission one field over. + +### Fixed + +- **`routing-failed` is a known status everywhere.** It sorts **above `in-progress`** — + first in its group — renders in the `error` colour rather than `muted`, is offered by the + status filter, and is a valid destination in the operator status-change control. Setting + it by hand is not a dead end: the dispatcher's routing-failed pass picks up any such task + whose retry window has passed, and a record with no `next_retry_at` is eligible + immediately, so it reads as "re-route now, skipping re-approval". +- **A `routing-failed` task explains itself.** The detail view gains a banner naming what + the dispatcher is doing, how many retries are used, when the next attempt is due, and that + the end of the budget is a dead letter. The status was previously a bare word. +- **A queued `manual-then-auto` survives the Start button.** `toWorkflowMode` collapsed it + to `semi-auto`, which re-pins every task the session spawns back to `semi-auto` — the + exact failure vikunja#533 added the mode to fix (four security→steward return tasks sat + unactioned for over a week). `review` on such a task now passes `manual-then-auto` + through; an explicit `auto` Start still overrides. The launch note says which it did. + +### Added + +- **`src/vocabulary.ts`** — one copy of the queue's statuses, task types and workflow modes, + plus the UI maps keyed *by* it. `STATUS_ORDER` and `STATUS_COLOR` are `Record`, + so adding a status without a sort position and a colour is now a `tsc` error rather than a + silent fallthrough. That is the half the old code lacked: `Record` accepts + anything and covers nothing. +- **`npm run gate:vocabulary`** — fetches `task-queue-mcp`'s `main` and asserts the sets + match, as its own CI step. Ported from `task-dispatcher`'s `test_task_queue_vocabulary.py` + (vikunja#324) with both of the properties that make that one work: **no + skip-on-no-network path** — it exits non-zero if it cannot read the upstream, because a + check that quietly passes when it could not reach its source of truth is + indistinguishable from one that verified something — and it tracks `main`, not a pin, so a + vocabulary change merged upstream turns this repo red. That is the alarm, not a + malfunction. It also fails rather than reporting a vacuous pass if it parses zero literals. + Proven to fire before shipping, in both directions and on all four outcomes. +- **A Mode filter and a mode badge.** The list can be filtered by `workflow_mode` and the + detail view names it with what it means; rows badge `auto` and `manual-then-auto` and stay + quiet about `semi-auto`, which is 98% of the queue. A task with no recorded mode renders as + unknown, not as the queue's default — older records predate the field and inventing a value + the queue never wrote is how this class of bug starts. + +### Changed + +- The two `NON_TERMINAL_STATUSES` literals are one derived constant + (`VALID_STATUSES - TERMINAL_STATUSES`, as upstream derives it). Two copies of one list is + how the second went stale. +- Relative imports now use `.ts`, not `.js`. `node --test` runs the sources directly and + does not rewrite a `.js` specifier to the `.ts` file beside it, so the panels — which all + used `.js` — were not importable by a test. The first one that needed to be, was not. + ## [0.7.0] - 2026-08-29 Adds a collapsed **Dead letters** section below the task list, and a Requeue button. diff --git a/README.md b/README.md index f56cfed..6ad6078 100644 --- a/README.md +++ b/README.md @@ -227,10 +227,14 @@ A task whose `target_agent` is absent from the file returns a clean `Unknown age rather than launching. > **Mode vocabulary.** Start sends `review | auto`; a launcher taking `--workflow-mode` -> receives `semi-auto | auto`, mapped explicitly. For a run-as agent, `review` is -> **prompt-enforced only** — the reference launcher sets `--dangerously-skip-permissions` -> itself and accepts no permission mode, so `--permission-mode plan` is not reachable. The -> UI says so on launch rather than implying a tool gate. +> receives `semi-auto | auto | manual-then-auto`, mapped explicitly. `auto` passes through. +> `review` becomes `semi-auto`, **except** for a task queued as `manual-then-auto`, which is +> passed through unchanged: both gate this leg, but only `manual-then-auto` lets the tasks +> that session spawns run unattended, and flattening it re-pins the whole chain to +> `semi-auto`. For a run-as agent, `review` is **prompt-enforced only** — the reference +> launcher sets `--dangerously-skip-permissions` itself and accepts no permission mode, so +> `--permission-mode plan` is not reachable. The UI says so on launch rather than implying a +> tool gate. ### Launch logs @@ -243,6 +247,9 @@ same shape and directory the reference dispatcher writes, so both are listable t npm install npm run build # tsc --noEmit (typecheck) + esbuild bundle to dist/ npm test # node --test — requires Node 22.18+ +npm run gate:vocabulary # asserts the queue vocabulary matches task-queue-mcp's main. + # Reaches the network and fails if it cannot — deliberately; + # a parity check that skips offline has verified nothing. ``` `npm run build` is the typecheck gate — `tsc --noEmit` runs first and the bundle only happens if it passes. diff --git a/manifest.json b/manifest.json index 18ef251..88f0af7 100644 --- a/manifest.json +++ b/manifest.json @@ -1,7 +1,7 @@ { "name": "task-queue", "displayName": "Task Queue", - "version": "0.7.0", + "version": "0.8.0", "description": "Task queue dashboard — view, filter, and launch agent tasks.", "author": "TadMSTR", "icon": "icon.svg", diff --git a/package.json b/package.json index beec379..751bc46 100644 --- a/package.json +++ b/package.json @@ -1,11 +1,12 @@ { "name": "cloudcli-plugin-task-queue", - "version": "0.7.0", + "version": "0.8.0", "private": true, "type": "module", "scripts": { "build": "tsc --noEmit && esbuild src/index.ts --bundle --format=esm --outfile=dist/index.js --sourcemap && esbuild src/server.ts --bundle --format=esm --platform=node --outfile=dist/server.js --sourcemap --external:ws", - "test": "node --test src/tests/*.test.ts" + "test": "node --test src/tests/*.test.ts", + "gate:vocabulary": "node src/gates/vocabulary-parity.ts" }, "devDependencies": { "@types/js-yaml": "^4.0.9", diff --git a/src/dead-letters.ts b/src/dead-letters.ts index 1e646c6..70b36a3 100644 --- a/src/dead-letters.ts +++ b/src/dead-letters.ts @@ -11,7 +11,7 @@ // without a DOM, and so the reason-grouping rule — the thing that turns seventeen rows // into one problem — is a pure function with tests rather than a loop inside a renderer. -import type { DeadLetter, DeadLetterGroup } from './types.js'; +import type { DeadLetter, DeadLetterGroup } from './types.ts'; /** The `failed_reason` shape task-dispatcher writes. Every field is optional in practice. */ interface RawFailedReason { diff --git a/src/gates/python-sets.ts b/src/gates/python-sets.ts new file mode 100644 index 0000000..31667eb --- /dev/null +++ b/src/gates/python-sets.ts @@ -0,0 +1,74 @@ +/** + * Extract module-level `NAME = {"a", "b"}` string-set literals from Python source. + * + * Split out of `vocabulary-parity.ts` because that script calls `process.exit` at module + * scope and so cannot be imported by a test — and a parser the vocabulary gate's verdict + * rests on, with no tests of its own, is the same "asserts a slot, not a behaviour" shape + * the gate exists to prevent. Nothing here executes Python or decides anything; it returns + * what it could parse and lets the caller decide what an empty result means. + * + * Deliberately narrow. Anything that is not a flat list of plain string literals — a + * derived set (`A - B`), a dict, a number, a backslash escape — yields `null` for that + * name, and the name is then simply absent from the map. Absent is loud at the call site + * (reported as "missing from task-queue-mcp — renamed upstream?"); a lenient parser that + * guessed would be quiet and wrong. + */ + +/** + * Parse the inside of a Python `{...}` as plain string literals. + * Returns null if the body contains anything else. + */ +export function parseStringSetBody(body: string): string[] | null { + const values: string[] = []; + let i = 0; + while (i < body.length) { + const ch = body[i]; + if (ch === ',' || /\s/.test(ch)) { i++; continue; } + // A `#` comment. Safe to strip here only because we have already established we are + // outside a string literal — the string branch below consumes its own content. + if (ch === '#') { while (i < body.length && body[i] !== '\n') i++; continue; } + if (ch === '"' || ch === "'") { + let j = i + 1; + let val = ''; + while (j < body.length && body[j] !== ch) { + // Refuse rather than mis-decode. None of the vocabulary values contain escapes, + // and a wrong decode here would be reported as a real drift. + if (body[j] === '\\') return null; + val += body[j]; + j++; + } + if (j >= body.length) return null; // unterminated + values.push(val); + i = j + 1; + continue; + } + return null; // identifier, dict colon, number, operator — not a plain string set + } + return values; +} + +/** + * Every module-level `NAME = { ... }` string-set literal in `source`. + * + * The name pattern requires column 0 and a bare ` = `, which excludes an annotated + * assignment (`NAME: dict[str, set[str]] = {`) — never one of these sets — as well as + * anything indented inside a function. + */ +export function literalSets(source: string): Map { + const found = new Map(); + const re = /^([A-Z][A-Z0-9_]*) = \{/gm; + let m: RegExpExecArray | null; + while ((m = re.exec(source)) !== null) { + let depth = 1; + let k = m.index + m[0].length; + while (k < source.length && depth > 0) { + if (source[k] === '{') depth++; + else if (source[k] === '}') depth--; + k++; + } + if (depth !== 0) continue; // unbalanced to EOF + const values = parseStringSetBody(source.slice(m.index + m[0].length, k - 1)); + if (values) found.set(m[1], values); + } + return found; +} diff --git a/src/gates/vocabulary-parity.ts b/src/gates/vocabulary-parity.ts new file mode 100644 index 0000000..be01e35 --- /dev/null +++ b/src/gates/vocabulary-parity.ts @@ -0,0 +1,206 @@ +/** + * Assert src/vocabulary.ts's task-queue vocabulary equals task-queue-mcp's. + * + * WHY THIS EXISTS (vikunja#558) + * + * This plugin is a reader and an operator surface for a queue whose schema lives in + * another repo. It carried hand-written copies of that vocabulary in four places, none of + * which learned about `routing-failed` when task-queue-mcp made it a first-class + * non-terminal status — so the status most in need of an operator sorted below + * `cancelled`, rendered in the same grey as `parked`, and was not offered by the filter. + * `manual-then-auto` (#543) was the same omission one field over. Nothing detected either: + * a missing key in a `Record` is not a type error, and a `switch` with a + * `default` has no missing case. + * + * This is the same check `task-dispatcher/tests/test_task_queue_vocabulary.py` runs at the + * dispatcher edge (vikunja#324), ported to this one. + * + * HOW + * + * The MCP is a separate, public repo, so this reads its source over HTTP and parses the + * set literals textually. Nothing fetched is ever executed, and no Python runs — a + * deliberately narrow tokenizer accepts a brace-delimited list of plain string literals + * and refuses everything else. Our own side is imported directly, because vocabulary.ts is + * a constants module with no side effects; that is the one asymmetry with the Python + * version, which reads its own source with `ast` because cli.py is not importable cheaply. + * + * DELIBERATELY NOT SKIPPABLE + * + * There is no "no network, skip" path, and no cached copy to fall back to. A vocabulary + * check that quietly passes when it could not read the upstream is indistinguishable from + * one that verified something, and that shape — a probe that reports success without + * asserting anything — is how #324 stayed open for months. If this cannot reach the + * upstream it exits non-zero, and the fix is to make the network work. + * + * The same reasoning covers the parse: if zero set literals come back, that is the + * extraction being broken rather than the vocabulary being in sync, and it fails rather + * than reporting a vacuous pass over an empty comparison. + * + * CONSEQUENCE WORTH KNOWING: this tracks task-queue-mcp's `main`, not a pin. A vocabulary + * change merged there turns this repo's CI red on the next push. That is the alarm, not a + * malfunction — the plugin is genuinely out of date at that moment. Update + * src/vocabulary.ts and it goes green. `tsc` will then hold it red until any new status + * has a sort position and a colour, which is the second half of the fix. + * + * Run: `npm run gate:vocabulary`. It is its OWN CI step, not part of `npm test` — a red + * vocabulary means "go edit src/vocabulary.ts", which is a different instruction from any + * unit failure, and folding the two together lets either hide the other. + */ + +import { literalSets } from './python-sets.ts'; +import { + VALID_STATUSES, + TERMINAL_STATUSES, + NON_TERMINAL_STATUSES, + VALID_TASK_TYPES, + VALID_WORKFLOW_MODES, +} from '../vocabulary.ts'; + +// The ref to compare against. `main` is the contract — that is what is deployed and what +// this plugin must agree with. +// +// The override exists for ONE situation, which is not hypothetical: a paired change lands +// in two repos and this one is legitimately ahead of the MCP's `main` until the MCP's PR +// merges. Without it the only way to see whether the rest of the pipeline is green is to +// merge first and find out. +// +// CI MUST NOT SET THIS. It defaults to `main` precisely so an unset environment gets the +// strict check; a pipeline that pinned it to a feature branch would be asserting parity +// with something nobody is running. Use it from a shell, note it in the PR, and let the +// post-merge run be the one that counts. +const UPSTREAM_REF = process.env.TASK_QUEUE_MCP_REF ?? 'main'; + +// The ref is interpolated into a URL path, so constrain it. `..` in particular would +// traverse out of this repo's path segment on raw.githubusercontent.com and fetch somebody +// else's queue.py — which this gate would then compare against and report as +// authoritative. It needs control of the environment to exploit, but it is free to close. +if (!/^[A-Za-z0-9._][A-Za-z0-9._/-]*$/.test(UPSTREAM_REF) || UPSTREAM_REF.includes('..')) { + console.error(`FATAL: refusing TASK_QUEUE_MCP_REF=${JSON.stringify(UPSTREAM_REF)} — not a plain git ref`); + process.exit(2); +} + +const UPSTREAM_URL = + `https://raw.githubusercontent.com/TadMSTR/task-queue-mcp/${UPSTREAM_REF}/src/tools/queue.py`; + +/** + * Every set both sides must agree on, upstream name -> our value. They happen to be spelled + * the same here; the mapping is kept explicit anyway, because what matters is the contents + * and a future rename should be a one-line edit rather than a restructure. + * + * NON_TERMINAL_STATUSES is absent on purpose: upstream derives it + * (`VALID_STATUSES - TERMINAL_STATUSES`) rather than writing a literal, so there is nothing + * to parse. It is checked separately, derived on both sides. + */ +const SHARED: Record = { + VALID_STATUSES, + TERMINAL_STATUSES, + VALID_TASK_TYPES, + VALID_WORKFLOW_MODES, +}; + +const failures: string[] = []; + +function check(cond: boolean, label: string): void { + console.log(cond ? ` ok ${label}` : ` FAIL ${label}`); + if (!cond) failures.push(label); +} + +/** + * Every module-level string-set literal upstream, or a hard failure. + * + * An empty result is the extraction being broken, not the vocabulary being in sync — so it + * exits rather than reporting a vacuous pass over an empty comparison. The parser itself is + * in `python-sets.ts` with its own tests; only this exit decision lives here. + */ +function upstreamSets(source: string, origin: string): Map { + const found = literalSets(source); + if (found.size === 0) { + console.error( + `FATAL: no set literals parsed from ${origin} — the extraction is broken, not the ` + + 'vocabulary. Failing rather than reporting a vacuous pass.', + ); + process.exit(2); + } + return found; +} + +async function fetchUpstream(): Promise { + try { + const resp = await fetch(UPSTREAM_URL, { signal: AbortSignal.timeout(30_000) }); + if (!resp.ok) throw new Error(`HTTP ${resp.status}`); + return await resp.text(); + } catch (err) { + console.error(`FATAL: could not read the vocabulary source of truth at ${UPSTREAM_URL}`); + console.error(` ${(err as Error).name}: ${(err as Error).message}`); + console.error( + ' This is a hard failure on purpose — see the header. A vocabulary check that ' + + 'skips when it cannot read the upstream reports the same result whether or not the ' + + 'two sides agree.', + ); + process.exit(2); + } +} + +function compare(label: string, ours: readonly string[], theirs: readonly string[]): void { + const o = new Set(ours); + const t = new Set(theirs); + const onlyOurs = [...o].filter(v => !t.has(v)).sort(); + const onlyTheirs = [...t].filter(v => !o.has(v)).sort(); + if (onlyOurs.length === 0 && onlyTheirs.length === 0) { + check(true, `${label} (${o.size} values)`); + return; + } + const detail: string[] = []; + if (onlyOurs.length) detail.push(`plugin-only: [${onlyOurs.join(', ')}]`); + if (onlyTheirs.length) detail.push(`mcp-only: [${onlyTheirs.join(', ')}]`); + check(false, `${label} — ${detail.join('; ')}`); +} + +async function main(): Promise { + console.log('task queue vocabulary parity (plugin ↔ task-queue-mcp)'); + console.log(` upstream: ${UPSTREAM_URL}`); + if (UPSTREAM_REF !== 'main') { + console.log( + ` NOTE: comparing against ref ${JSON.stringify(UPSTREAM_REF)}, NOT main. This is a ` + + 'pre-merge check and does not prove parity with what is deployed.', + ); + } + + const mcp = upstreamSets(await fetchUpstream(), 'task-queue-mcp/src/tools/queue.py'); + + for (const [name, ours] of Object.entries(SHARED)) { + const theirs = mcp.get(name); + if (!theirs) { + check(false, `${name} is missing from task-queue-mcp — renamed upstream?`); + continue; + } + compare(name, ours, theirs); + } + + // Derived on both sides, so this asserts our subtraction as well as the two sets feeding + // it. Skipped if either input failed to parse — reporting it too would be one drift + // counted twice. + const upStatuses = mcp.get('VALID_STATUSES'); + const upTerminal = mcp.get('TERMINAL_STATUSES'); + if (upStatuses && upTerminal) { + const upNonTerminal = upStatuses.filter(s => !upTerminal.includes(s)); + compare('NON_TERMINAL_STATUSES (derived both sides)', NON_TERMINAL_STATUSES, upNonTerminal); + } + + console.log(''); + if (failures.length > 0) { + console.error(`VOCABULARY DRIFT (${failures.length}):`); + for (const f of failures) console.error(` - ${f}`); + console.error(''); + console.error( + 'Fix by editing src/vocabulary.ts to match task-queue-mcp, then giving any new ' + + 'status a STATUS_ORDER position and a STATUS_COLOR entry — tsc will insist. ' + + 'Do not edit this gate to make it pass.', + ); + return 1; + } + console.log('vocabulary is in sync'); + return 0; +} + +process.exit(await main()); diff --git a/src/index.ts b/src/index.ts index a325bd7..6227b60 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,10 +1,10 @@ -import type { PluginAPI, PluginContext, Task, ThemeColors, HeadlessRun, HeadlessRunDetail, DeadLetter } from './types.js'; -import { themeColors, injectGlobalStyles, MONO } from './panels/styles.js'; -import { renderTaskList } from './panels/task-list.js'; -import { renderTaskDetail } from './panels/task-detail.js'; -import { renderHeadlessRuns } from './panels/headless-runs.js'; -import { renderDeadLetters } from './panels/dead-letters.js'; -import { createWsClient, WsClient } from './panels/ws-client.js'; +import type { PluginAPI, PluginContext, Task, ThemeColors, HeadlessRun, HeadlessRunDetail, DeadLetter } from './types.ts'; +import { themeColors, injectGlobalStyles, MONO } from './panels/styles.ts'; +import { renderTaskList, type TaskFilters } from './panels/task-list.ts'; +import { renderTaskDetail } from './panels/task-detail.ts'; +import { renderHeadlessRuns } from './panels/headless-runs.ts'; +import { renderDeadLetters } from './panels/dead-letters.ts'; +import { createWsClient, WsClient } from './panels/ws-client.ts'; // ── State ────────────────────────────────────────────────────────────── @@ -16,7 +16,7 @@ interface AppState { loading: boolean; error: string | null; wsConnected: boolean; - filters: { agent: string; status: string; taskType: string }; + filters: TaskFilters; headlessRuns: HeadlessRun[]; selectedRunId: string | null; selectedRun: HeadlessRunDetail | null; @@ -39,7 +39,7 @@ export function mount(container: HTMLElement, api: PluginAPI): void { loading: true, error: null, wsConnected: false, - filters: { agent: '', status: '', taskType: '' }, + filters: { agent: '', status: '', taskType: '', mode: '' }, headlessRuns: [], selectedRunId: null, selectedRun: null, diff --git a/src/launch-policy.ts b/src/launch-policy.ts index 5dfd59a..c386f7b 100644 --- a/src/launch-policy.ts +++ b/src/launch-policy.ts @@ -21,6 +21,7 @@ import path from 'node:path'; import fs from 'node:fs'; import { load as yamlLoad } from 'js-yaml'; +import type { WorkflowMode } from './vocabulary.ts'; // Closed sets. A policy file cannot introduce a user, a launcher directory, or a // project root outside these — it selects from them. @@ -169,17 +170,33 @@ export function loadLaunchPolicy(filePath: string, home: string): LaunchPolicy { /** What the plugin's Start button sends. */ export type StartMode = 'review' | 'auto'; -/** What task-queue-mcp and run-steward.sh accept. */ -export type WorkflowMode = 'semi-auto' | 'auto' | 'manual-then-auto'; +/** + * What task-queue-mcp and run-steward.sh accept. Sourced from `vocabulary.ts` rather than + * spelled again here — this file had its own copy, and copies of this list are vikunja#558. + */ +export type { WorkflowMode }; /** * The plugin's Start vocabulary is not the queue's. `review` is a UI word for "let me * see it before it runs"; the queue's nearest value is `semi-auto`. Mapped explicitly * rather than passed through: `review` is not in task-queue-mcp's VALID_WORKFLOW_MODES, * and run-steward.sh would reject it by name. See vikunja#533 for the wider unification. + * + * `queuedMode` is the mode the task was SUBMITTED with, and it exists for exactly one + * value. `manual-then-auto` gates its own leg like `semi-auto` but hands `auto` to + * everything the session spawns; run-steward.sh implements that downgrade and rejects + * nothing here. Collapsing it to `semi-auto` on the way out — which is what this function + * did before #543 — silently pins every downstream handoff back to `semi-auto`, which is + * the failure vikunja#533 added the mode to fix: four security->steward return tasks sat + * unactioned for over a week. `review` means "gate this leg", and `manual-then-auto` is + * the spelling of that which preserves the rest of the chain. + * + * An explicit `auto` Start still wins: the operator has said run the whole thing + * unattended, and a `manual-then-auto` task's children were going to be `auto` regardless. */ -export function toWorkflowMode(mode: StartMode): WorkflowMode { - return mode === 'review' ? 'semi-auto' : 'auto'; +export function toWorkflowMode(mode: StartMode, queuedMode?: string): WorkflowMode { + if (mode === 'auto') return 'auto'; + return queuedMode === 'manual-then-auto' ? 'manual-then-auto' : 'semi-auto'; } // ── argv construction ──────────────────────────────────────────────────────── @@ -203,6 +220,7 @@ export function buildLaunchArgv( mode: StartMode, prompt: string, claudeBin: string, + queuedMode?: string, ): LaunchArgv { if (entry.runAsUser && entry.launcher) { // -n: never prompt for a password. The sudoers grant is NOPASSWD, so a prompt @@ -212,7 +230,7 @@ export function buildLaunchArgv( const argv = [ 'sudo', '-n', '-u', entry.runAsUser, entry.launcher, - '--workflow-mode', toWorkflowMode(mode), + '--workflow-mode', toWorkflowMode(mode, queuedMode), '--', prompt, ]; // run-steward.sh sets --dangerously-skip-permissions itself and accepts no @@ -220,9 +238,13 @@ export function buildLaunchArgv( // the way it is for one launched directly. The review prompt still instructs the // agent to stop and wait, but that is a prompt-level control, not a tool gate. // Say so rather than implying a guarantee that is not there. - const note = mode === 'review' + let note = mode === 'review' ? 'review is prompt-enforced for this agent — its launcher does not accept a permission mode' : undefined; + if (mode === 'review' && queuedMode === 'manual-then-auto') { + note = (note ? note + '. ' : '') + + 'queued manual-then-auto — this leg is gated, everything it spawns runs auto'; + } return { argv, note }; } diff --git a/src/panels/dead-letters.ts b/src/panels/dead-letters.ts index 66fbad2..9e48d0f 100644 --- a/src/panels/dead-letters.ts +++ b/src/panels/dead-letters.ts @@ -1,6 +1,6 @@ -import type { ThemeColors, DeadLetter } from '../types.js'; -import { escHtml, ago, MONO } from './styles.js'; -import { groupByReason } from '../dead-letters.js'; +import type { ThemeColors, DeadLetter } from '../types.ts'; +import { escHtml, ago, MONO } from './styles.ts'; +import { groupByReason } from '../dead-letters.ts'; /** * The Dead letters section — tasks task-dispatcher gave up routing. diff --git a/src/panels/headless-runs.ts b/src/panels/headless-runs.ts index 61cf872..6689e2f 100644 --- a/src/panels/headless-runs.ts +++ b/src/panels/headless-runs.ts @@ -1,5 +1,5 @@ -import type { ThemeColors, HeadlessRun, HeadlessRunDetail } from '../types.js'; -import { escHtml, ago, statusColor, MONO } from './styles.js'; +import type { ThemeColors, HeadlessRun, HeadlessRunDetail } from '../types.ts'; +import { escHtml, ago, statusColor, MONO } from './styles.ts'; /** * The Headless runs section — a read-only view of agent runs that had no operator diff --git a/src/panels/styles.ts b/src/panels/styles.ts index aac78b7..c90e812 100644 --- a/src/panels/styles.ts +++ b/src/panels/styles.ts @@ -1,4 +1,5 @@ -import type { ThemeColors } from '../types.js'; +import type { ThemeColors } from '../types.ts'; +import { STATUS_COLOR } from '../vocabulary.ts'; const MONO = "'JetBrains Mono', 'Fira Code', ui-monospace, monospace"; @@ -94,18 +95,19 @@ export function ago(iso: string): string { return `${d}d ago`; } +/** + * Colour for a status. Takes `string`, not `Status` — the callers render whatever the queue + * YAML says, including a value written by something newer than this build. + * + * The mapping itself lives in `vocabulary.ts` as a `Record`, so a status the + * plugin knows about but has not been given a colour is a compile error. This used to be a + * `switch` with a `default: return c.muted`, which silently absorbed `routing-failed` and + * rendered it the same grey as `cancelled` (vikunja#558). The fallback below still exists, + * but now it only catches statuses this build has genuinely never heard of. + */ export function statusColor(status: string, c: ThemeColors): string { - switch (status) { - case 'approved': return c.ok; - case 'in-progress': return c.accent; - case 'submitted': return c.muted; - case 'pending-approval': return c.warn; - case 'parked': return c.muted; - case 'completed': return c.ok; - case 'failed': return c.error; - case 'cancelled': return c.muted; - default: return c.muted; - } + const key = (STATUS_COLOR as Record)[status]; + return key ? c[key] : c.muted; } export function priorityColor(priority: string, c: ThemeColors): string { diff --git a/src/panels/task-detail.ts b/src/panels/task-detail.ts index 15fccff..dd3d02c 100644 --- a/src/panels/task-detail.ts +++ b/src/panels/task-detail.ts @@ -1,5 +1,11 @@ -import type { ThemeColors, Task } from '../types.js'; -import { escHtml, ago, statusColor, priorityColor, MONO } from './styles.js'; +import type { ThemeColors, Task } from '../types.ts'; +import { escHtml, ago, statusColor, priorityColor, MONO } from './styles.ts'; +import { + NON_TERMINAL_STATUSES, + WORKFLOW_MODE_DISPLAY, + isTerminal, + isWorkflowMode, +} from '../vocabulary.ts'; interface TaskDetailOptions { task: Task; @@ -21,9 +27,6 @@ interface TaskDetailOptions { onOpenRun?: () => void; } -const DETAIL_TERMINAL_STATUSES = ['completed', 'failed', 'cancelled']; -const DETAIL_NON_TERMINAL_STATUSES = ['submitted', 'pending-approval', 'approved', 'in-progress', 'parked']; - export function renderTaskDetail(container: HTMLElement, opts: TaskDetailOptions): void { const { task, contextPreviews, colors: c, onBack, onApprove, onStart, onCancel, onPark, onUnpark, onAmend, onSetStatus } = opts; const isParked = task.status === 'parked'; @@ -31,6 +34,10 @@ export function renderTaskDetail(container: HTMLElement, opts: TaskDetailOptions const sc = statusColor(task.status, c); const priority = task.payload.priority ?? 'normal'; const pc = priorityColor(priority, c); + // Absent on tasks queued before task-queue-mcp wrote the field. Rendered as unknown + // rather than as the queue's default — this panel reports what the record says. + const mode = task.workflow_mode; + const modeDisplay = isWorkflowMode(mode) ? WORKFLOW_MODE_DISPLAY[mode] : null; const wrapper = document.createElement('div'); wrapper.className = 'tq-up'; @@ -80,6 +87,12 @@ export function renderTaskDetail(container: HTMLElement, opts: TaskDetailOptions Priority${escHtml(priority)} Risk${escHtml(task.risk_level)} Approval${task.requires_approval ? 'required' : 'auto'} + Mode${ + modeDisplay + ? `${escHtml(mode as string)}` + + ` — ${escHtml(modeDisplay.hint)}` + : `${mode ? escHtml(mode) + ' (not a known mode)' : 'not recorded'}` + } Created${ago(task.created)} (${new Date(task.created).toLocaleString()}) TTL${task.ttl_days}d `; @@ -98,7 +111,7 @@ export function renderTaskDetail(container: HTMLElement, opts: TaskDetailOptions } // Lifecycle controls: cancel, park/unpark, and amend — all non-terminal only. Parking a // terminal task is meaningless, and amendments to a finished task have no reader. - if (!DETAIL_TERMINAL_STATUSES.includes(task.status)) { + if (!isTerminal(task.status)) { actions.appendChild(makeActionButton('Cancel', c.error, c, () => onCancel(task.id))); actions.appendChild( isParked @@ -119,8 +132,26 @@ export function renderTaskDetail(container: HTMLElement, opts: TaskDetailOptions wrapper.appendChild(banner); } + // Routing-failed banner. The status is written by the dispatcher when it cannot route a + // task, and it backs off exponentially before retrying; after the retry budget the task + // is dead-lettered. None of that is guessable from the word, and this is the status most + // likely to want a human — say what it means and what happens next. + if (task.status === 'routing-failed') { + const policy = task.retry_policy ?? {}; + const attempts = typeof policy.retry_count === 'number' ? policy.retry_count : null; + const next = policy.next_retry_at; + const banner = document.createElement('div'); + banner.style.cssText = `padding:8px 12px;margin-bottom:16px;font-size:12px;background:${c.surface};border:1px solid ${c.error};border-left-width:3px;border-radius:4px;color:${c.text};`; + banner.textContent = + 'Routing failed — the dispatcher could not hand this to its agent and is backing off. ' + + (attempts !== null ? `${attempts} retr${attempts === 1 ? 'y' : 'ies'} used. ` : '') + + (next ? `Next attempt ${new Date(next).toLocaleString()}. ` : 'It will be retried on the next dispatcher pass. ') + + 'When the retry budget runs out it is dead-lettered — see the dead-letters section.'; + wrapper.appendChild(banner); + } + // Status-change control — advance a task an agent missed (audited operator override). - if (!DETAIL_TERMINAL_STATUSES.includes(task.status)) { + if (!isTerminal(task.status)) { const statusRow = document.createElement('div'); statusRow.style.cssText = `display:flex;align-items:center;gap:8px;margin-bottom:16px;`; @@ -134,7 +165,12 @@ export function renderTaskDetail(container: HTMLElement, opts: TaskDetailOptions placeholder.value = ''; placeholder.textContent = 'advance to…'; select.appendChild(placeholder); - for (const s of DETAIL_NON_TERMINAL_STATUSES) { + // Every non-terminal status, including `routing-failed`. Setting it by hand is not a + // dead end: the dispatcher's routing-failed pass picks up any such task whose retry + // window has passed, and a record with no `next_retry_at` is eligible immediately — so + // this reads as "re-route now, skipping re-approval", which is a thing an operator + // wants. The MCP accepts any non-terminal -> non-terminal move on the override path. + for (const s of NON_TERMINAL_STATUSES) { if (s === task.status) continue; const opt = document.createElement('option'); opt.value = s; diff --git a/src/panels/task-list.ts b/src/panels/task-list.ts index db9d6c2..58574ef 100644 --- a/src/panels/task-list.ts +++ b/src/panels/task-list.ts @@ -1,5 +1,26 @@ -import type { ThemeColors, Task } from '../types.js'; -import { escHtml, ago, statusColor, priorityColor, priorityIcon, MONO } from './styles.js'; +import type { ThemeColors, Task } from '../types.ts'; +import { escHtml, ago, statusColor, priorityColor, priorityIcon, MONO } from './styles.ts'; +import { + STATUS_ORDER, + VALID_STATUSES, + VALID_TASK_TYPES, + VALID_WORKFLOW_MODES, + WORKFLOW_MODE_DISPLAY, + isTerminal, + isWorkflowMode, +} from '../vocabulary.ts'; + +/** + * The list's filter state. `mode` filters on `workflow_mode` — added with #543, because the + * queue's three modes are the difference between "an operator has to press Start" and "this + * and everything downstream of it runs unattended", and there was no way to ask which. + */ +export interface TaskFilters { + agent: string; + status: string; + taskType: string; + mode: string; +} interface TaskListOptions { tasks: Task[]; @@ -12,42 +33,27 @@ interface TaskListOptions { onUnpark: (taskId: string) => void; onAmend: (taskId: string) => void; onSetStatus: (taskId: string, status: string) => void; - filters: { agent: string; status: string; taskType: string }; - onFilterChange: (filters: { agent: string; status: string; taskType: string }) => void; + filters: TaskFilters; + onFilterChange: (filters: TaskFilters) => void; } -const STATUS_ORDER: Record = { - 'in-progress': 0, - 'approved': 1, - 'pending-approval': 2, - 'submitted': 3, - // Parked sorts below the live statuses but above the terminal ones — it is paused work, - // not finished work, and it should not compete for attention with what is actionable. - 'parked': 4, - 'completed': 5, - 'failed': 6, - 'cancelled': 7, -}; - -// Non-terminal statuses an operator can move a task between (the status-change control). -const NON_TERMINAL_STATUSES = ['submitted', 'pending-approval', 'approved', 'in-progress', 'parked']; -const TERMINAL_STATUSES = ['completed', 'failed', 'cancelled']; - -export { NON_TERMINAL_STATUSES, TERMINAL_STATUSES }; - const PRIORITY_ORDER: Record = { 'urgent': 0, 'high': 1, 'normal': 2, }; -function sortTasks(tasks: Task[]): Task[] { +export function sortTasks(tasks: Task[]): Task[] { return [...tasks].sort((a, b) => { const pa = PRIORITY_ORDER[a.payload.priority ?? 'normal'] ?? 2; const pb = PRIORITY_ORDER[b.payload.priority ?? 'normal'] ?? 2; if (pa !== pb) return pa - pb; - const sa = STATUS_ORDER[a.status] ?? 9; - const sb = STATUS_ORDER[b.status] ?? 9; + // The `?? 99` is for a status written by something newer than this build, not for one + // the plugin knows about: STATUS_ORDER is keyed by the vocabulary, so a known status + // without a position does not compile. `routing-failed` reaching this fallback and + // sorting below `cancelled` is vikunja#558. + const sa = STATUS_ORDER[a.status as keyof typeof STATUS_ORDER] ?? 99; + const sb = STATUS_ORDER[b.status as keyof typeof STATUS_ORDER] ?? 99; if (sa !== sb) return sa - sb; return new Date(b.created).getTime() - new Date(a.created).getTime(); }); @@ -71,18 +77,24 @@ export function renderTaskList(container: HTMLElement, opts: TaskListOptions): v // Union the observed statuses with the known vocabulary, so `parked` is selectable even // when nothing is parked yet — otherwise a brand-new status is undiscoverable until the // operator has already used it somewhere else. - const statuses = [...new Set([ - ...tasks.map(t => t.status), - ...NON_TERMINAL_STATUSES, - ...TERMINAL_STATUSES, + const statuses = [...new Set([...tasks.map(t => t.status), ...VALID_STATUSES])].sort(); + const types = [...new Set([...tasks.map(t => t.task_type), ...VALID_TASK_TYPES])].sort(); + // Same union for modes. Not derived from the tasks alone: `manual-then-auto` exists to be + // asked for, and until one is queued nothing would offer it (vikunja#543). + const modes = [...new Set([ + ...tasks.map(t => t.workflow_mode).filter((m): m is string => !!m), + ...VALID_WORKFLOW_MODES, ])].sort(); - const types = [...new Set(tasks.map(t => t.task_type))].sort(); // Apply filters let filtered = tasks; if (filters.agent) filtered = filtered.filter(t => t.target_agent === filters.agent); if (filters.status) filtered = filtered.filter(t => t.status === filters.status); if (filters.taskType) filtered = filtered.filter(t => t.task_type === filters.taskType); + // A record with no `workflow_mode` matches no mode filter rather than being folded into + // the queue's default. Older tasks predate the field; claiming they are `semi-auto` would + // be this plugin inventing a value the queue never wrote. + if (filters.mode) filtered = filtered.filter(t => t.workflow_mode === filters.mode); const sorted = sortTasks(filtered); const grouped = groupByAgent(sorted); @@ -114,12 +126,23 @@ export function renderTaskList(container: HTMLElement, opts: TaskListOptions): v ${types.map(t => ``).join('')} + ${filtered.length} of ${tasks.length} tasks `; container.appendChild(filterBar); // Bind filter events - for (const [id, key] of [['tq-filter-agent', 'agent'], ['tq-filter-status', 'status'], ['tq-filter-type', 'taskType']] as const) { + for (const [id, key] of [ + ['tq-filter-agent', 'agent'], + ['tq-filter-status', 'status'], + ['tq-filter-type', 'taskType'], + ['tq-filter-mode', 'mode'], + ] as const) { const el = filterBar.querySelector(`#${id}`) as HTMLSelectElement; el?.addEventListener('change', () => { onFilterChange({ ...filters, [key]: el.value }); @@ -169,12 +192,21 @@ export function renderTaskList(container: HTMLElement, opts: TaskListOptions): v const shortId = task.id.slice(0, 8); + // Mode marker, shown only when the mode changes what happens without an operator. + // `semi-auto` is 98% of the queue and is what a reader already assumes; badging it + // too would make the column noise instead of signal. + const mode = task.workflow_mode; + const modeBadge = isWorkflowMode(mode) && mode !== 'semi-auto' + ? `${escHtml(mode)}` + : ''; + row.innerHTML = ` ${escHtml(shortId)} ${pIcon ? `${pIcon}` : ''} ${escHtml(task.status)} ${escHtml(task.task_type)} + ${modeBadge} ${escHtml(task.summary)} ${ago(task.created)} `; @@ -197,7 +229,7 @@ export function renderTaskList(container: HTMLElement, opts: TaskListOptions): v // Lifecycle controls: cancel and park/unpark, non-terminal tasks only. Amend lives // in the detail view — it needs the description in front of you to write a useful one. - if (!TERMINAL_STATUSES.includes(task.status)) { + if (!isTerminal(task.status)) { actions.appendChild(makeButton('Cancel', c.error, c, () => onCancel(task.id))); actions.appendChild( isParked diff --git a/src/server.ts b/src/server.ts index f5c89b7..33f68f5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -87,6 +87,9 @@ const LAUNCH_POLICY: { policy: LaunchPolicy | null; error: string | null } = (() // ── Task operations (direct YAML file reader) ───────────────────────── +// Deliberately looser than the `Task` in types.ts: this parses arbitrary YAML off disk, +// so every field is optional and unknown keys pass through to the client untouched. The UI +// side gets the strict shape. Only the fields this file actually reads are named. interface Task { id?: string; task_type?: string; @@ -94,6 +97,8 @@ interface Task { status?: string; summary?: string; created?: unknown; + /** Passed to the launcher on Start so a `manual-then-auto` task keeps its chain. */ + workflow_mode?: string; payload?: Record; [key: string]: unknown; } @@ -230,7 +235,16 @@ function resolveBin(name: string): string | null { return null; } -function launchSession(taskId: string, targetAgent: string, mode: StartMode): { ok: boolean; error?: string; note?: string } { +/** + * `queuedMode` is the task's own `workflow_mode`, passed through to the launcher. It is + * read from the queue record by the caller and never inferred here — see toWorkflowMode. + */ +function launchSession( + taskId: string, + targetAgent: string, + mode: StartMode, + queuedMode?: string, +): { ok: boolean; error?: string; note?: string } { if (!VALID_ID.test(taskId)) return { ok: false, error: `Invalid task id: ${taskId}` }; if (!LAUNCH_POLICY.policy) { @@ -269,7 +283,7 @@ function launchSession(taskId: string, targetAgent: string, mode: StartMode): { ? `You have a pending task (id=${taskId}). Read it from task-queue-mcp via get_task. Present a summary of the work entailed. Do NOT begin execution — wait for operator approval.` : `You have a pending task (id=${taskId}). Read it from task-queue-mcp via get_task. Claim it (update status to in-progress), then execute the task.`; - const { argv, note } = buildLaunchArgv(entry, mode, prompt, claudeBin); + const { argv, note } = buildLaunchArgv(entry, mode, prompt, claudeBin, queuedMode); // Per-launch log replaces stdio:'ignore' so a failed launch is diagnosable. // cwd:projectDir is how Claude Code resolves project config — `--project` is not a valid flag. @@ -581,7 +595,12 @@ const server = http.createServer(async (req, res) => { const { mode } = JSON.parse(body) as { mode: StartMode }; const taskData = getTask(startMatch[1]); if (!taskData) { res.statusCode = 404; res.end(JSON.stringify({ error: 'task not found' })); return; } - const result = launchSession(startMatch[1], taskData.target_agent ?? '', mode ?? 'review'); + const result = launchSession( + startMatch[1], + taskData.target_agent ?? '', + mode ?? 'review', + taskData.workflow_mode, + ); res.statusCode = result.ok ? 200 : 400; res.end(JSON.stringify(result)); return; diff --git a/src/tests/python-sets.test.ts b/src/tests/python-sets.test.ts new file mode 100644 index 0000000..42a2ab6 --- /dev/null +++ b/src/tests/python-sets.test.ts @@ -0,0 +1,110 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { parseStringSetBody, literalSets } from '../gates/python-sets.ts'; + +// A verbatim slice of task-queue-mcp's src/tools/queue.py, comments included. The comments +// are not decoration: the real file interleaves multi-line prose with the values, and a +// parser that only worked on the tidied version would report a vacuous zero on the real one. +const REAL = `VALID_RISK_LEVELS = {"low", "medium", "high"} +VALID_STATUSES = { + "submitted", + "approved", + "pending-approval", + "in-progress", + "parked", + "routing-failed", + "completed", + "failed", + "cancelled", +} +VALID_PRIORITIES = {"normal", "high", "urgent"} +# \`docs\` is the writer's work-list type, introduced when doc-update-queue.jsonl was retired +# (task-queue-lifecycle-and-doc-queue-2026-08 Phase 5). +VALID_TASK_TYPES = { + "build", # trailing comment on a value line + "deploy", +} +VALID_WORKFLOW_MODES = {"semi-auto", "auto", "manual-then-auto"} + +SELF_TERMINAL_TASK_TYPES = {"notify"} + +TERMINAL_STATUSES = {"completed", "failed", "cancelled"} +NON_TERMINAL_STATUSES = VALID_STATUSES - TERMINAL_STATUSES + +VALID_TRANSITIONS: dict[str, set[str]] = { + "in-progress": {"approved"}, + "completed": {"in-progress"}, +} +`; + +test('parses the real queue.py shape, multi-line and single-line', () => { + const sets = literalSets(REAL); + assert.deepEqual(sets.get('VALID_STATUSES'), [ + 'submitted', 'approved', 'pending-approval', 'in-progress', 'parked', + 'routing-failed', 'completed', 'failed', 'cancelled', + ]); + assert.deepEqual(sets.get('VALID_WORKFLOW_MODES'), ['semi-auto', 'auto', 'manual-then-auto']); + assert.deepEqual(sets.get('TERMINAL_STATUSES'), ['completed', 'failed', 'cancelled']); + assert.deepEqual(sets.get('SELF_TERMINAL_TASK_TYPES'), ['notify']); + assert.deepEqual(sets.get('VALID_RISK_LEVELS'), ['low', 'medium', 'high']); +}); + +test('comments between and beside values are stripped, not parsed as values', () => { + assert.deepEqual(literalSets(REAL).get('VALID_TASK_TYPES'), ['build', 'deploy']); +}); + +// The two upstream constructs that MUST NOT be mistaken for string sets. A derived set +// parsed as empty would make NON_TERMINAL_STATUSES look like it drifted to nothing, and a +// dict parsed loosely would compare its keys against a status list. Both must be absent. +test('a derived set is not parsed as a literal', () => { + assert.equal(literalSets(REAL).has('NON_TERMINAL_STATUSES'), false); +}); + +test('an annotated dict assignment is not parsed as a set', () => { + assert.equal(literalSets(REAL).has('VALID_TRANSITIONS'), false); +}); + +test('rejects non-string members rather than dropping them', () => { + assert.equal(parseStringSetBody('"a", 3'), null); + assert.equal(parseStringSetBody('"a", SOMETHING'), null); + assert.equal(parseStringSetBody('"k": "v"'), null); +}); + +test('rejects an escape rather than mis-decoding it', () => { + assert.equal(parseStringSetBody('"a\\\\nb"'), null); +}); + +test('rejects an unterminated string', () => { + assert.equal(parseStringSetBody('"a'), null); +}); + +test('single and double quotes are both accepted', () => { + assert.deepEqual(parseStringSetBody(`'a', "b"`), ['a', 'b']); +}); + +test('a source with no set literals yields an empty map, for the caller to reject', () => { + // The gate turns this into a hard exit(2). The parser must not decide that itself, and + // must not invent values: an empty map is the honest answer, and an empty COMPARISON is + // the failure mode the gate's vacuous-pass guard exists for. + assert.equal(literalSets('def f():\n return 1\n').size, 0); +}); + +test('an unbalanced brace to EOF is skipped, not parsed as far as it got', () => { + assert.equal(literalSets('VALID_STATUSES = {\n "submitted",\n').size, 0); +}); + +test('an indented assignment inside a function is not a module-level set', () => { + assert.equal(literalSets('def f():\n VALID_STATUSES = {"x"}\n').size, 0); +}); + +test('an annotated assignment is skipped even when its body is a valid string set', () => { + // Two independent layers exclude `VALID_TRANSITIONS: dict[...] = {...}`: the name regex + // requires a bare ` = `, and the body parser refuses the `:`. Only the second is + // exercised by the real file, so pin the first here — otherwise loosening the regex is a + // silent change. Matching the Python gate's behaviour, which handles `ast.Assign` and not + // `ast.AnnAssign`. If upstream ever annotates one of these sets the gate reports it as + // missing, which is a loud one-character fix rather than a quiet wrong answer. + assert.equal(literalSets('VALID_STATUSES: set[str] = {"submitted"}\n').size, 0); + assert.deepEqual(literalSets('VALID_STATUSES = {"submitted"}\n').get('VALID_STATUSES'), ['submitted']); +}); diff --git a/src/tests/vocabulary.test.ts b/src/tests/vocabulary.test.ts new file mode 100644 index 0000000..65b3bcc --- /dev/null +++ b/src/tests/vocabulary.test.ts @@ -0,0 +1,142 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + VALID_STATUSES, + TERMINAL_STATUSES, + NON_TERMINAL_STATUSES, + VALID_WORKFLOW_MODES, + STATUS_ORDER, + STATUS_COLOR, + WORKFLOW_MODE_DISPLAY, + isTerminal, + isWorkflowMode, +} from '../vocabulary.ts'; +import { statusColor } from '../panels/styles.ts'; +import { sortTasks } from '../panels/task-list.ts'; +import { toWorkflowMode, buildLaunchArgv, type AgentLaunch } from '../launch-policy.ts'; +import type { ThemeColors, Task } from '../types.ts'; + +// Distinguishable sentinels, so an assertion can say WHICH theme slot was chosen rather +// than only that two colours differ. +const C: ThemeColors = { + bg: 'BG', surface: 'SURFACE', border: 'BORDER', text: 'TEXT', muted: 'MUTED', + accent: 'ACCENT', dim: 'DIM', ok: 'OK', warn: 'WARN', error: 'ERROR', +}; + +// ── The four sites that missed `routing-failed` (vikunja#558) ──────────────── + +test('every valid status has a sort position and a colour', () => { + // These are Record, so a gap is already a tsc error. Asserted at runtime too + // because the type only binds while the keys stay literal — a future refactor to + // Record would silently restore the `?? 9` / `default: muted` fallthrough + // that is the whole of #558. + for (const s of VALID_STATUSES) { + assert.equal(typeof STATUS_ORDER[s], 'number', `${s} has no STATUS_ORDER position`); + assert.ok(STATUS_COLOR[s], `${s} has no STATUS_COLOR entry`); + } +}); + +test('routing-failed sorts above in-progress, and above every terminal status', () => { + assert.ok(STATUS_ORDER['routing-failed'] < STATUS_ORDER['in-progress']); + for (const s of TERMINAL_STATUSES) { + assert.ok(STATUS_ORDER['routing-failed'] < STATUS_ORDER[s], `sorts below ${s}`); + } +}); + +test('routing-failed does not render as muted', () => { + // The defect was not "no colour" — it was `default: return c.muted`, which is + // indistinguishable from `parked` and `cancelled`, i.e. "nothing to do here". + assert.notEqual(statusColor('routing-failed', C), C.muted); + assert.equal(statusColor('routing-failed', C), C.error); + assert.equal(statusColor('cancelled', C), C.muted); +}); + +test('a status this build has never heard of still renders, muted', () => { + assert.equal(statusColor('some-future-status', C), C.muted); +}); + +test('the operator status-change control offers routing-failed', () => { + // The detail panel builds its dropdown from NON_TERMINAL_STATUSES. The old hand-written + // copy omitted routing-failed, so a task could never be moved into it and the list's + // status filter never offered the value. + assert.ok(NON_TERMINAL_STATUSES.includes('routing-failed')); + assert.equal(isTerminal('routing-failed'), false); +}); + +test('NON_TERMINAL_STATUSES is exactly VALID_STATUSES minus TERMINAL_STATUSES', () => { + assert.deepEqual( + [...NON_TERMINAL_STATUSES].sort(), + VALID_STATUSES.filter(s => !(TERMINAL_STATUSES as readonly string[]).includes(s)).sort(), + ); + assert.equal(NON_TERMINAL_STATUSES.length + TERMINAL_STATUSES.length, VALID_STATUSES.length); +}); + +// The end-to-end version of the above: the sort, not the table it reads. +test('a routing-failed task sorts to the top of its group, not the bottom', () => { + const task = (status: string, id: string): Task => ({ + id, created: '2026-08-01T00:00:00Z', source_agent: 'research', target_agent: 'developer', + task_type: 'build', risk_level: 'low', requires_approval: false, status, + summary: id, ttl_days: 30, payload: { description: '' }, + result: { output: null, completed_by: null, completed_at: null }, history: [], + }); + + const sorted = sortTasks([ + task('cancelled', 'c'), + task('completed', 'd'), + task('in-progress', 'p'), + task('routing-failed', 'r'), + ]); + assert.deepEqual(sorted.map(t => t.id), ['r', 'p', 'd', 'c']); +}); + +// ── manual-then-auto (vikunja#543) ─────────────────────────────────────────── + +test('every workflow mode has a display entry', () => { + for (const m of VALID_WORKFLOW_MODES) { + assert.ok(WORKFLOW_MODE_DISPLAY[m], `${m} has no display entry`); + assert.ok(WORKFLOW_MODE_DISPLAY[m].hint.length > 0); + } +}); + +test('manual-then-auto renders distinctly from both other modes', () => { + const tone = (m: 'semi-auto' | 'auto' | 'manual-then-auto') => WORKFLOW_MODE_DISPLAY[m].tone; + assert.notEqual(tone('manual-then-auto'), tone('auto')); + assert.notEqual(tone('manual-then-auto'), tone('semi-auto')); +}); + +test('isWorkflowMode accepts the vocabulary and nothing else', () => { + for (const m of VALID_WORKFLOW_MODES) assert.equal(isWorkflowMode(m), true); + assert.equal(isWorkflowMode('review'), false); // the UI word, not a queue value + assert.equal(isWorkflowMode(undefined), false); +}); + +test('a review Start preserves a queued manual-then-auto instead of flattening it', () => { + // Flattening it to `semi-auto` pins every task the session spawns back to `semi-auto`, + // which is exactly what vikunja#533 added the mode to stop. + assert.equal(toWorkflowMode('review', 'manual-then-auto'), 'manual-then-auto'); + assert.equal(toWorkflowMode('review', 'semi-auto'), 'semi-auto'); + assert.equal(toWorkflowMode('review', undefined), 'semi-auto'); + assert.equal(toWorkflowMode('review'), 'semi-auto'); +}); + +test('an explicit auto Start still wins over the queued mode', () => { + assert.equal(toWorkflowMode('auto', 'manual-then-auto'), 'auto'); + assert.equal(toWorkflowMode('auto', 'semi-auto'), 'auto'); +}); + +test('the queued mode reaches the launcher argv, and is explained in the note', () => { + const entry: AgentLaunch = { + projectDir: '/home/testuser/.claude/projects/steward', + runAsUser: 'agent-steward', + launcher: '/usr/local/sbin/forge/run-steward.sh', + }; + const { argv, note } = buildLaunchArgv(entry, 'review', 'do the thing', '', 'manual-then-auto'); + assert.deepEqual(argv, [ + 'sudo', '-n', '-u', 'agent-steward', + '/usr/local/sbin/forge/run-steward.sh', + '--workflow-mode', 'manual-then-auto', + '--', 'do the thing', + ]); + assert.match(note ?? '', /manual-then-auto/); +}); diff --git a/src/types.ts b/src/types.ts index a4071a5..dc3327c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -67,8 +67,25 @@ export interface Task { status: string; summary: string; ttl_days: number; + /** + * `semi-auto` | `auto` | `manual-then-auto` — see `vocabulary.ts`. Optional because + * records written before task-queue-mcp added the field carry none, and a task with no + * recorded mode is rendered as unknown rather than as the queue's default. + */ + workflow_mode?: string; /** Status to return to on unpark. Present only while status === 'parked'. */ parked_from?: string; + /** + * Written by task-dispatcher while a task sits at `routing-failed`. Read-only here — it + * is what lets the detail panel say when the next attempt is due instead of leaving the + * status as a bare word. + */ + retry_policy?: { + retry_count?: number; + max_retries?: number; + next_retry_at?: string | null; + last_failure_reason?: string; + }; payload: { description: string; context_refs?: string[]; diff --git a/src/vocabulary.ts b/src/vocabulary.ts new file mode 100644 index 0000000..e9d5a5c --- /dev/null +++ b/src/vocabulary.ts @@ -0,0 +1,156 @@ +/** + * The task-queue vocabulary, as this plugin understands it. + * + * WHY THIS FILE EXISTS (vikunja#558, #543) + * + * The plugin does not own this vocabulary — `task-queue-mcp`'s `src/tools/queue.py` does. + * It carried four hand-written copies of parts of it instead (`STATUS_ORDER` and + * `NON_TERMINAL_STATUSES` in `panels/task-list.ts`, `DETAIL_NON_TERMINAL_STATUSES` in + * `panels/task-detail.ts`, the `switch` in `panels/styles.ts`), and every one of them + * missed `routing-failed` when the MCP made it a first-class non-terminal status. The + * result: the status most in need of an operator sorted below `cancelled`, rendered in the + * same grey as `parked`, and was not offered by the status filter. `manual-then-auto` (#543) + * was the same omission one field over. + * + * Two mechanisms keep this from recurring, and they are different in kind: + * + * 1. `gates/vocabulary-parity.ts` fetches the MCP's `main` and asserts the sets below + * match it. That catches "upstream changed and we did not". It runs in CI as its own + * step and has no skip-on-no-network path — see the script's header. + * + * 2. The `Record` maps below are typed by the vocabulary itself, so adding a + * value to `VALID_STATUSES` without also giving it a sort position and a colour is a + * `tsc` error, not a silent fallthrough to `?? 9` and `muted`. That is the half the + * old code lacked: `STATUS_ORDER` was `Record`, which accepts anything + * and covers nothing. + * + * The chain is: upstream adds a status -> the parity gate goes red -> you add it here -> + * the build stays red until it has a position and a colour. Do not loosen either link. + */ + +import type { ThemeColors } from './types.ts'; + +// ── The sets task-queue-mcp owns ───────────────────────────────────────────── +// +// These are compared literally against `src/tools/queue.py` on the MCP's `main`. Keep them +// as flat `as const` arrays of string literals: the gate reads them by importing this +// module, but a human diffing the two files should be able to do it by eye. + +export const VALID_STATUSES = [ + 'submitted', + 'approved', + 'pending-approval', + 'in-progress', + 'parked', + 'routing-failed', + 'completed', + 'failed', + 'cancelled', +] as const; + +export const TERMINAL_STATUSES = ['completed', 'failed', 'cancelled'] as const; + +export const VALID_TASK_TYPES = [ + 'build', + 'deploy', + 'fix', + 'research', + 'review', + 'audit', + 'notify', + 'docs', + 'ticket_audit', + 'ticket_audit_complete', +] as const; + +export const VALID_WORKFLOW_MODES = ['semi-auto', 'auto', 'manual-then-auto'] as const; + +export type Status = (typeof VALID_STATUSES)[number]; +export type TaskType = (typeof VALID_TASK_TYPES)[number]; +export type WorkflowMode = (typeof VALID_WORKFLOW_MODES)[number]; + +/** + * Derived, exactly as upstream derives it (`VALID_STATUSES - TERMINAL_STATUSES`). Written + * as a subtraction rather than a second list because the two hand-written copies this + * replaces are the whole reason this file exists. + */ +export const NON_TERMINAL_STATUSES: readonly Status[] = VALID_STATUSES.filter( + (s): s is Status => !(TERMINAL_STATUSES as readonly string[]).includes(s), +); + +export function isTerminal(status: string): boolean { + return (TERMINAL_STATUSES as readonly string[]).includes(status); +} + +// ── UI mappings, keyed by the vocabulary ───────────────────────────────────── + +/** + * Sort position within an agent group. Lower sorts first. + * + * `routing-failed` sorts ABOVE `in-progress`: it is a task the dispatcher could not route + * and is backing off on, so it is the row most likely to need a human, and under the old + * `?? 9` fallthrough it sorted below `cancelled` — dead last, beneath work nobody intends + * to do. Terminal statuses stay at the bottom; `parked` sits between live and terminal + * because it is paused work, not finished work. + */ +export const STATUS_ORDER: Record = { + 'routing-failed': 0, + 'in-progress': 1, + 'approved': 2, + 'pending-approval': 3, + 'submitted': 4, + 'parked': 5, + 'completed': 6, + 'failed': 7, + 'cancelled': 8, +}; + +/** + * Which theme colour each status renders in. + * + * `routing-failed` is `error` rather than `warn`. `warn` already carries + * `pending-approval`, which is a routine state every task passes through; a status that + * means "the dispatcher gave up on this leg and is counting down to a dead letter" has to + * outrank it. It shares `error` with `failed`, which is acceptable — they are adjacent in + * meaning and the label sits next to the dot. What is not acceptable is the `muted` it + * used to fall through to, which read identically to `cancelled`. + */ +export const STATUS_COLOR: Record = { + 'submitted': 'muted', + 'approved': 'ok', + 'pending-approval': 'warn', + 'in-progress': 'accent', + 'parked': 'muted', + 'routing-failed': 'error', + 'completed': 'ok', + 'failed': 'error', + 'cancelled': 'muted', +}; + +/** + * How each workflow mode is shown. The distinction that matters is the third one: + * `manual-then-auto` gates only its own leg, and everything the resulting session spawns + * inherits `auto`. Rendering it as just another word next to `semi-auto` loses that, and + * losing it is how four security->steward return tasks sat unactioned (vikunja#533). + */ +export const WORKFLOW_MODE_DISPLAY: Record< + WorkflowMode, + { tone: keyof ThemeColors; hint: string } +> = { + 'semi-auto': { + tone: 'muted', + hint: 'operator Start required; every task this one spawns is gated the same way', + }, + 'auto': { + tone: 'warn', + hint: 'launches without an operator, and so does everything it spawns', + }, + 'manual-then-auto': { + tone: 'accent', + hint: 'operator Start required for this task only — everything it spawns runs auto', + }, +}; + +export function isWorkflowMode(value: unknown): value is WorkflowMode { + return typeof value === 'string' && (VALID_WORKFLOW_MODES as readonly string[]).includes(value); +} From 91acf66c585edb56350cd688482ac18e35c05a73 Mon Sep 17 00:00:00 2001 From: developer-agent Date: Sat, 29 Aug 2026 08:26:05 -0400 Subject: [PATCH 2/3] security: an unreadable next_retry_at says so, not "Invalid Date" Caught by the pre-audit baseline. The routing-failed banner interpolated `new Date(next_retry_at).toLocaleString()` directly, so a malformed value in the queue YAML rendered the literal string "Invalid Date" at the operator. It reaches textContent, not innerHTML, so this was never an injection -- it is the same failure as the launch-log birthtime case: a date the code could not read, presented as though it could. Now the banner names it as unreadable and keeps the rest of the message. Co-Authored-By: Claude Opus 5 (1M context) agent-id: developer --- src/panels/task-detail.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/panels/task-detail.ts b/src/panels/task-detail.ts index dd3d02c..13824a6 100644 --- a/src/panels/task-detail.ts +++ b/src/panels/task-detail.ts @@ -139,13 +139,22 @@ export function renderTaskDetail(container: HTMLElement, opts: TaskDetailOptions if (task.status === 'routing-failed') { const policy = task.retry_policy ?? {}; const attempts = typeof policy.retry_count === 'number' ? policy.retry_count : null; - const next = policy.next_retry_at; + // An unparseable timestamp says so, rather than rendering the string "Invalid Date" at + // an operator. Same rule as the launch-log birthtime handling: a date we cannot read is + // reported as unknown, never dressed up as a real one. + const nextAt = policy.next_retry_at ? new Date(policy.next_retry_at) : null; + const next = nextAt && !Number.isNaN(nextAt.getTime()) ? nextAt.toLocaleString() : null; + const nextUnreadable = !!policy.next_retry_at && next === null; const banner = document.createElement('div'); banner.style.cssText = `padding:8px 12px;margin-bottom:16px;font-size:12px;background:${c.surface};border:1px solid ${c.error};border-left-width:3px;border-radius:4px;color:${c.text};`; banner.textContent = 'Routing failed — the dispatcher could not hand this to its agent and is backing off. ' + (attempts !== null ? `${attempts} retr${attempts === 1 ? 'y' : 'ies'} used. ` : '') - + (next ? `Next attempt ${new Date(next).toLocaleString()}. ` : 'It will be retried on the next dispatcher pass. ') + + (next + ? `Next attempt ${next}. ` + : nextUnreadable + ? 'Its next-attempt time is unreadable. ' + : 'It will be retried on the next dispatcher pass. ') + 'When the retry budget runs out it is dead-lettered — see the dead-letters section.'; wrapper.appendChild(banner); } From 678678c5203670fb753fc0c40554e22091dc29c4 Mon Sep 17 00:00:00 2001 From: developer-agent Date: Sat, 29 Aug 2026 08:35:48 -0400 Subject: [PATCH 3/3] docs: record the audit's SSRF-02 acceptance at the call site agent-workflow-interop-2026-08-phase2 audit: 0 findings, 2 Info, both no action. INFO 1 was the deliberate absence of `redirect: "manual"` on the vocabulary gate's fetch, self-disclosed before the audit ran and confirmed acceptable. Marked SECURITY[accepted] where the decision lives, with the row in host-forge-knowledge-base/security/accepted-risks.md. Also adds the in-build "Invalid Date" fix (91acf66) to the 0.8.0 changelog entry, which it was missing. Co-Authored-By: Claude Opus 5 (1M context) agent-id: developer --- CHANGELOG.md | 5 +++++ src/gates/vocabulary-parity.ts | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4432a34..97e589f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,11 @@ urgent. `manual-then-auto` (#543) was the same omission one field over. - **A `routing-failed` task explains itself.** The detail view gains a banner naming what the dispatcher is doing, how many retries are used, when the next attempt is due, and that the end of the budget is a dead letter. The status was previously a bare word. +- **An unreadable `next_retry_at` says so.** The `routing-failed` banner interpolated + `new Date(...).toLocaleString()` directly, so a malformed value in the queue YAML rendered + the literal string `Invalid Date` at the operator. Same failure as the launch-log + `birthtime` case: a date the code could not read, presented as though it could. Caught by + this build's own pre-audit baseline, not by the audit. - **A queued `manual-then-auto` survives the Start button.** `toWorkflowMode` collapsed it to `semi-auto`, which re-pins every task the session spawns back to `semi-auto` — the exact failure vikunja#533 added the mode to fix (four security→steward return tasks sat diff --git a/src/gates/vocabulary-parity.ts b/src/gates/vocabulary-parity.ts index be01e35..fb385e7 100644 --- a/src/gates/vocabulary-parity.ts +++ b/src/gates/vocabulary-parity.ts @@ -126,6 +126,16 @@ function upstreamSets(source: string, origin: string): Map { async function fetchUpstream(): Promise { try { + // SECURITY[accepted]: no `redirect: "manual"`, deviating from baseline pattern SSRF-02. + // The URL is not caller-supplied — host and path are literals, only the ref segment + // varies, and it is charset-checked and `..`-rejected above. The response is never + // executed: python-sets.ts accepts only a brace-delimited list of plain string literals. + // Worst case from a hostile redirect is a false red (loud, blocks CI) or a false green + // requiring the attacker to serve the exact correct value sets, which achieves nothing. + // Following redirects beats breaking the gate if GitHub ever 302s this path. Reviewed + // and accepted 2026-08-29 — agent-workflow-interop-2026-08-phase2 audit, INFO 1; row in + // host-forge-knowledge-base/security/accepted-risks.md. Revisit if this is ever made to + // accept a caller-supplied URL or host. const resp = await fetch(UPSTREAM_URL, { signal: AbortSignal.timeout(30_000) }); if (!resp.ok) throw new Error(`HTTP ${resp.status}`); return await resp.text();