fix(kanban): enforce terminal/owner guards — closes #137 #4

Closed

Mark wants to merge 1 commit from 137-kanban-guards-and-watcher-branches into main

Mark commented

2026-09-14 23:19:44 +00:00

Owner

Implements the four-guard state model spec (plan §1, Vikunja #137) on Mark/kanban. Every rule is enforced in code with explicit guards — no convention-only enforcement.

Rule-to-guard mapping

#	Spec rule	Code guard	Test surface
1	Any transition into a `done` state sets `terminal=true`; transitions out of `done` are rejected at the guard layer regardless of source	`server/transition_guards.ts` (computeFlagUpdatesForTransition, line 91 sets `terminal: 1` on done entry; line 132 rejects done→X with `kind: done_is_terminal`); wired into `server/storage.ts::transitionTask` (line 290 atomic flag update); surfaced as 409 by `server/routes.ts`	(a) `four_guards_integration.test.ts:142` — 2 tests
2	`blocked` cards only return to active via `kanban_unblock`; nothing else flips them back	`server/watcher.ts::branch_blocked` (line 150 — unsubscribe, only re-add on `kanban_unblock` event in `deliverKanbanEvent`, line 287); storage mirror at `server/storage.ts::transitionTask` (line 236 — rejects `blocked→active` with typed error referencing `kanbanUnblock`); sole handler at `server/storage.ts::kanbanUnblock` (line 55 — only callable surface that flips blocked→assigned)	(b) `four_guards_integration.test.ts:251` — 3 tests
3	`running` is owned exclusively by the watcher; no other actor may transition a running card	`server/transition_guards.ts::computeFlagUpdatesForTransition` (line 92 — sets `owner` to caller on running entry); `server/transition_guards.ts::assertCallerAllowedForRunning` (rejects non-watcher with `kind: foreign_running_write`, missing/null caller with `kind: missing_caller_for_running`); `server/watcher.ts::branch_running` (line 170 — assertion at tick entry: `card.owner !== WATCHER_PRINCIPAL` throws `WatcherError{kind: foreign_running_write}`, sweep catches per-card); HTTP trust boundary at `server/principal.ts::deriveCallerFromRequest` (body-supplied caller ignored, server-derived from `X-Internal-Actor` header, closed set: `watcher` only)	(c) `four_guards_integration.test.ts:382` — 5 tests
4	Done cards are never re-watched; the watcher must drop them on entry and not re-subscribe	`server/watcher.ts::branch_done` (line 131 — drop_done action; `_watchSet` mutation only at line 181 [legit running dispatch] and line 287 [`kanban_unblock` arm]); terminal flag is the persistence signal checked at sweep time; `KANBAN_EVENT_CATALOG` exposes no event that re-adds a done card	(d) `four_guards_integration.test.ts:552` — 3 tests

Supporting tests

Beyond the four named integration tests above, the diff also includes:

server/__tests__/transition_guards.test.ts — 23 pure-logic guard tests
server/__tests__/principal.test.ts — 16 tests for the HTTP trust-boundary helper (forged-but-lies, missing header, header smuggling, closed-set rejection)
server/__tests__/routes.test.ts — 21 tests with caller/owner flow including HTTP surface (409 vs 400 precedence)
server/__tests__/storage.test.ts — 35 tests incl. legacy-DB migration

Total: 157 tests, all passing on branch HEAD a0bc081d9a53d77d9a1e294ab275ef64864c377f.

Diff stats vs. `origin/main`

13 files changed, 3612 insertions(+), 22 deletions(-).

shared/schema.ts — terminal (integer, default 0) + owner (text, nullable) columns on tasks; caller field dropped from insertTransitionSchema (body field silently stripped at the zod layer)
migrations/0001_add_terminal_owner.sql — schema migration with safe defaults
scripts/run-migrations.ts — migrations runner
server/transition_guards.ts (new) — pure-logic guard module with typed TransitionGuardError{kind:…}
server/principal.ts (new) — server-derived caller trust boundary (deriveCallerFromRequest, deriveOwnerFromRequest)
server/storage.ts — transitionTask runs the guard before ALLOWED_TRANSITIONS; kanbanUnblock() storage method (sole handler for blocked→assigned); setTaskFlags() for non-transition flag mutation
server/routes.ts — X-Internal-Actor header → caller derivation; PATCH /flags route; 409 surface for guard errors
server/watcher.ts (new) — in-process Watcher class with Sweep() three-branch logic + deliverKanbanEvent() catalog
server/__tests__/four_guards_integration.test.ts (new) — 17 tests across 5 describe blocks covering the four named cases (a)(b)(c)(d) plus (e) owner-assertion failing-input surface

Acceptance

All four rules enforced in code with explicit guards (not just convention)
kanban_unblock is the only path out of blocked (storage-layer mirror + watcher catalog + HTTP route all agree)
Watcher drop-list for done verified by test (5 sweeps, no re-subscribe on a done card)
HTTP trust boundary: forged body caller=watcher (no header) reaches storage as caller=unknown and is rejected

Closes #137 (Vikunja issue tracker). Tracked locally as Mark/kanban#5.

Refs: kanban t_d3f147f9, t_e49638bb, t_002084ad, t_c657a9cc, t_383734ad, t_fc944a5a.

Implements the four-guard state model spec (plan §1, Vikunja #137) on Mark/kanban. Every rule is enforced in code with explicit guards — no convention-only enforcement. ## Rule-to-guard mapping | # | Spec rule | Code guard | Test surface | |---|---|---|---| | 1 | Any transition into a `done` state sets `terminal=true`; transitions out of `done` are rejected at the guard layer regardless of source | `server/transition_guards.ts` (computeFlagUpdatesForTransition, line 91 sets `terminal: 1` on done entry; line 132 rejects done→X with `kind: done_is_terminal`); wired into `server/storage.ts::transitionTask` (line 290 atomic flag update); surfaced as 409 by `server/routes.ts` | (a) `four_guards_integration.test.ts:142` — 2 tests | | 2 | `blocked` cards only return to active via `kanban_unblock`; nothing else flips them back | `server/watcher.ts::branch_blocked` (line 150 — unsubscribe, only re-add on `kanban_unblock` event in `deliverKanbanEvent`, line 287); storage mirror at `server/storage.ts::transitionTask` (line 236 — rejects `blocked→active` with typed error referencing `kanbanUnblock`); sole handler at `server/storage.ts::kanbanUnblock` (line 55 — only callable surface that flips blocked→assigned) | (b) `four_guards_integration.test.ts:251` — 3 tests | | 3 | `running` is owned exclusively by the watcher; no other actor may transition a running card | `server/transition_guards.ts::computeFlagUpdatesForTransition` (line 92 — sets `owner` to caller on running entry); `server/transition_guards.ts::assertCallerAllowedForRunning` (rejects non-watcher with `kind: foreign_running_write`, missing/null caller with `kind: missing_caller_for_running`); `server/watcher.ts::branch_running` (line 170 — assertion at tick entry: `card.owner !== WATCHER_PRINCIPAL` throws `WatcherError{kind: foreign_running_write}`, sweep catches per-card); HTTP trust boundary at `server/principal.ts::deriveCallerFromRequest` (body-supplied caller ignored, server-derived from `X-Internal-Actor` header, closed set: `watcher` only) | (c) `four_guards_integration.test.ts:382` — 5 tests | | 4 | Done cards are never re-watched; the watcher must drop them on entry and not re-subscribe | `server/watcher.ts::branch_done` (line 131 — drop_done action; `_watchSet` mutation only at line 181 [legit running dispatch] and line 287 [`kanban_unblock` arm]); terminal flag is the persistence signal checked at sweep time; `KANBAN_EVENT_CATALOG` exposes no event that re-adds a done card | (d) `four_guards_integration.test.ts:552` — 3 tests | ## Supporting tests Beyond the four named integration tests above, the diff also includes: - `server/__tests__/transition_guards.test.ts` — 23 pure-logic guard tests - `server/__tests__/principal.test.ts` — 16 tests for the HTTP trust-boundary helper (forged-but-lies, missing header, header smuggling, closed-set rejection) - `server/__tests__/routes.test.ts` — 21 tests with caller/owner flow including HTTP surface (409 vs 400 precedence) - `server/__tests__/storage.test.ts` — 35 tests incl. legacy-DB migration Total: **157 tests, all passing** on branch HEAD `a0bc081d9a53d77d9a1e294ab275ef64864c377f`. ## Diff stats vs. `origin/main` 13 files changed, 3612 insertions(+), 22 deletions(-). - `shared/schema.ts` — `terminal` (integer, default 0) + `owner` (text, nullable) columns on `tasks`; `caller` field dropped from `insertTransitionSchema` (body field silently stripped at the zod layer) - `migrations/0001_add_terminal_owner.sql` — schema migration with safe defaults - `scripts/run-migrations.ts` — migrations runner - `server/transition_guards.ts` (new) — pure-logic guard module with typed `TransitionGuardError{kind:…}` - `server/principal.ts` (new) — server-derived caller trust boundary (`deriveCallerFromRequest`, `deriveOwnerFromRequest`) - `server/storage.ts` — `transitionTask` runs the guard before ALLOWED_TRANSITIONS; `kanbanUnblock()` storage method (sole handler for blocked→assigned); `setTaskFlags()` for non-transition flag mutation - `server/routes.ts` — `X-Internal-Actor` header → caller derivation; PATCH `/flags` route; 409 surface for guard errors - `server/watcher.ts` (new) — in-process `Watcher` class with `Sweep()` three-branch logic + `deliverKanbanEvent()` catalog - `server/__tests__/four_guards_integration.test.ts` (new) — 17 tests across 5 describe blocks covering the four named cases (a)(b)(c)(d) plus (e) owner-assertion failing-input surface ## Acceptance - All four rules enforced in code with explicit guards (not just convention) - `kanban_unblock` is the only path out of `blocked` (storage-layer mirror + watcher catalog + HTTP route all agree) - Watcher drop-list for `done` verified by test (5 sweeps, no re-subscribe on a done card) - HTTP trust boundary: forged body `caller=watcher` (no header) reaches storage as `caller=unknown` and is rejected Closes #137 (Vikunja issue tracker). Tracked locally as Mark/kanban#5. Refs: kanban t_d3f147f9, t_e49638bb, t_002084ad, t_c657a9cc, t_383734ad, t_fc944a5a.

Mark referenced this pull request

2026-09-14 23:20:41 +00:00

Vikunja #137 — Four-guard state model spec (terminal/done, blocked/unblock, running/watcher-owner, done/no-rewatch) #5

Mark referenced this pull request

2026-09-14 23:22:05 +00:00

Vikunja #137 — Four-guard state model spec (terminal/done, blocked/unblock, running/watcher-owner, done/no-rewatch) #5

Mark referenced this pull request from a commit

2026-09-14 23:22:30 +00:00

Add CI workflow to branch so PR #4 triggers Forgejo Actions

Mark added 1 commit

2026-09-14 23:22:30 +00:00

Add CI workflow to branch so PR #4 triggers Forgejo Actions

CI / ci (pull_request) Successful in 1m55s

Details

CI / ci (push) Successful in 2m22s

Details

ebbf4fa0cc

The four-guard branch was based on b165eed (before the CI workflow was
added at c494b8e). Without a workflow file at the branch tip, Forgejo
Actions does not dispatch CI on PR #4.

Adding the same workflow as origin/main to make CI run on the PR head.
The merge to main will fast-forward the file (no-op since main already
has it).

Mark referenced this pull request from Mark/vllm_mm

2026-09-18 23:15:34 +00:00

[CRITICAL] start_model memory check + status update nested inside 'stop other active models' loop (processes.py:90-140) #39

Mark closed this pull request

2026-09-21 01:31:32 +00:00