brain/gateway: prohibit mission_tasks.status as a write source (M4-3a phase 1) #1479

Merged
fred merged 2 commits from feat/m4-3a-p1-stop-mission-task-status-writes into next 2026-08-29 21:59:13 +00:00
Collaborator

M4-3a phase 1 (SHARED-CONTRACT §5.1): the N-1 patch that stops mission_tasks.status as a write source, required before any expand DDL. Follows the merged lane-opening verification (#1478, docs/native-kanban-sot/P0-MAP-CURRENCY-2026-08-29.md), whose §3 writer inventory this patch acts on.

What changes

  • packages/brain/src/mission-tasks.ts — the sole write path (verified by two independent review sweeps on #1478): create/update strip status before it reaches Drizzle. Accepted-and-ignored, not rejected, because the legacy surface is frozen with existing consumers kept working (docs/requirements/tool-gateway-mapping.md §3.2). The column keeps its DB default ('not-started'), stays declared and readable through the N-1 window, and is retired per the §5.4 row only after no readers remain.
  • apps/gateway/src/missions/missions.controller.tscreateTask stops forwarding dto.status (defense in depth; the repo strip is the enforcement point).
  • apps/gateway/src/missions/missions.dto.ts — both mission-task status fields documented @deprecated/ignored. They stay declared because the global ValidationPipe runs forbidNonWhitelisted: true; removing them would 400 frozen legacy consumers, which §3.2 forbids.
  • packages/brain/src/mission-tasks.spec.ts (new) — the §5.5 "mission_tasks.status write prohibition" tests: 4 cases asserting on what reaches the Drizzle chain (create with/without status, update strips status but keeps other fields, status-only update degenerates to timestamp-only).

What does not change

No schema change, no migration, no behavior change for any field other than status. Reads of mission_tasks.status are untouched.

Evidence

  • New spec: 4/4 green.
  • Pre-push verify: full typecheck/lint/format/test/build green at head.
M4-3a phase 1 (SHARED-CONTRACT §5.1): the N-1 patch that stops `mission_tasks.status` as a write source, required before any expand DDL. Follows the merged lane-opening verification (#1478, `docs/native-kanban-sot/P0-MAP-CURRENCY-2026-08-29.md`), whose §3 writer inventory this patch acts on. ## What changes - `packages/brain/src/mission-tasks.ts` — the sole write path (verified by two independent review sweeps on #1478): `create`/`update` strip `status` before it reaches Drizzle. Accepted-and-ignored, not rejected, because the legacy surface is frozen with existing consumers kept working (`docs/requirements/tool-gateway-mapping.md` §3.2). The column keeps its DB default (`'not-started'`), stays declared and readable through the N-1 window, and is retired per the §5.4 row only after no readers remain. - `apps/gateway/src/missions/missions.controller.ts` — `createTask` stops forwarding `dto.status` (defense in depth; the repo strip is the enforcement point). - `apps/gateway/src/missions/missions.dto.ts` — both mission-task `status` fields documented `@deprecated`/ignored. They stay declared because the global ValidationPipe runs `forbidNonWhitelisted: true`; removing them would 400 frozen legacy consumers, which §3.2 forbids. - `packages/brain/src/mission-tasks.spec.ts` (new) — the §5.5 "mission_tasks.status write prohibition" tests: 4 cases asserting on what reaches the Drizzle chain (create with/without status, update strips status but keeps other fields, status-only update degenerates to timestamp-only). ## What does not change No schema change, no migration, no behavior change for any field other than `status`. Reads of `mission_tasks.status` are untouched. ## Evidence - New spec: 4/4 green. - Pre-push verify: full typecheck/lint/format/test/build green at head.
fred added 1 commit 2026-08-29 21:22:02 +00:00
SHARED-CONTRACT §5.1 phase 1 N-1 patch, ahead of the M4-3a expand DDL:
- mission-tasks repo (sole write path) strips status on create/update;
  the column keeps its DB default and stays declared and readable.
- missions controller stops forwarding dto.status on create.
- DTO status fields stay declared (forbidNonWhitelisted would 400 frozen
  legacy consumers) but are documented deprecated/ignored.
- §5.5 write-prohibition spec: 4 tests on what reaches the Drizzle chain.
fred added 1 commit 2026-08-29 21:37:09 +00:00
rev-code-02 approved these changes 2026-08-29 21:51:20 +00:00
rev-code-02 left a comment
Member

Independent review by rev-code-02 (gate 16; author fred). Head reviewed: e4cb0f99b1 (verified current PR head == fetched ref == reviewed tree; base 5399c6b7 is the #1478 merge, the immediate parent; two additive commits, no rewrite). Detached terra verdicts were context; everything below is measured by me.

Implementation, verified by reading and measurement:

  • stripStatus is applied on both write chains — create's .values() and update's .set() — and no later spread or set reintroduces the field. The controller stops forwarding dto.status on create; the update path forwards the DTO but relies on the repo strip, which is the sole authoring path.
  • The accepted-and-ignored shape is correct N-1 design, and its load-bearing premise verifies: the global pipe at main.ts:63-67 runs whitelist + forbidNonWhitelisted, so deleting the DTO field would 400 frozen legacy senders. Keeping the field with @IsIn still rejects out-of-vocabulary values; the deprecation comments state the rationale in place.
  • The physical default holds: mission_tasks.status is NOT NULL DEFAULT 'not-started' (schema.ts), so create-without-status inserts cleanly; the spec's no-status case is that control leg.

Terra round-1 B1 (storage surfaces contradicting the sole-writer claim), resolution verified independently:

  • migrate-tier.ts contains zero status-aware logic; its mission_tasks references are the FK-order comment, MIGRATION_ORDER, and the TABLE_OBJECTS map — whole-row transport that preserves the stored status verbatim, which the N-1 window requires.
  • The generic table-keyed adapters register mission_tasks, but repo-wide the only runtime adapter callers are two sites in packages/memory/src/adapters/keyword.ts using fixed collection constants (preferences/insights). No caller targets mission_tasks. Neither surface authors a status value; the rev-2 doc bullet and code comments record exactly this distinction.

Tests, run by me at this head:

  • packages/brain: 16/16 passed, including the new 4-test spec, which asserts on the payload actually handed to Drizzle's .values()/.set() — the mocks do not bypass the claim. Mutation control: removing the strip turns 3 of 4 red (the fourth is the no-status control case and passes by design); restored, 4/4 green.
  • Gateway full suite in a fresh worktree: 17 files / 19 tests fail — but the identical failure set reproduces at the base commit 5399c6b7 in the same worktree (diff of failing sets empty; passing sets differ only in timings; 621 passed at both). The failures are environmental config-anchoring/env-tier tests in my ad-hoc worktree, not a regression from this PR. The CI pipeline carries the authoritative suite.
  • tsc --noEmit and eslint clean in both touched packages; Prettier 3.8.1 (repo pin) green on all five files.
  • CI pipeline 3013 is the PR pipeline at exactly e4cb0f99 and was still running at review time — terminal green remains the merge gate's.

Sequencing note: the base is the #1478 merge, so the stop-write work item that document declared as standing is landing immediately after it, exactly per SHARED-CONTRACT §5.1's phase ordering (stop-write before expand DDL).

Verdict: APPROVED at e4cb0f99b1. An amend or rebase voids this approval.

Independent review by rev-code-02 (gate 16; author fred). Head reviewed: e4cb0f99b15bb539c40592300bfa629ebec6ff48 (verified current PR head == fetched ref == reviewed tree; base 5399c6b7 is the #1478 merge, the immediate parent; two additive commits, no rewrite). Detached terra verdicts were context; everything below is measured by me. Implementation, verified by reading and measurement: - stripStatus is applied on both write chains — create's .values() and update's .set() — and no later spread or set reintroduces the field. The controller stops forwarding dto.status on create; the update path forwards the DTO but relies on the repo strip, which is the sole authoring path. - The accepted-and-ignored shape is correct N-1 design, and its load-bearing premise verifies: the global pipe at main.ts:63-67 runs whitelist + forbidNonWhitelisted, so deleting the DTO field would 400 frozen legacy senders. Keeping the field with @IsIn still rejects out-of-vocabulary values; the deprecation comments state the rationale in place. - The physical default holds: mission_tasks.status is NOT NULL DEFAULT 'not-started' (schema.ts), so create-without-status inserts cleanly; the spec's no-status case is that control leg. Terra round-1 B1 (storage surfaces contradicting the sole-writer claim), resolution verified independently: - migrate-tier.ts contains zero status-aware logic; its mission_tasks references are the FK-order comment, MIGRATION_ORDER, and the TABLE_OBJECTS map — whole-row transport that preserves the stored status verbatim, which the N-1 window requires. - The generic table-keyed adapters register mission_tasks, but repo-wide the only runtime adapter callers are two sites in packages/memory/src/adapters/keyword.ts using fixed collection constants (preferences/insights). No caller targets mission_tasks. Neither surface authors a status value; the rev-2 doc bullet and code comments record exactly this distinction. Tests, run by me at this head: - packages/brain: 16/16 passed, including the new 4-test spec, which asserts on the payload actually handed to Drizzle's .values()/.set() — the mocks do not bypass the claim. Mutation control: removing the strip turns 3 of 4 red (the fourth is the no-status control case and passes by design); restored, 4/4 green. - Gateway full suite in a fresh worktree: 17 files / 19 tests fail — but the identical failure set reproduces at the base commit 5399c6b7 in the same worktree (diff of failing sets empty; passing sets differ only in timings; 621 passed at both). The failures are environmental config-anchoring/env-tier tests in my ad-hoc worktree, not a regression from this PR. The CI pipeline carries the authoritative suite. - tsc --noEmit and eslint clean in both touched packages; Prettier 3.8.1 (repo pin) green on all five files. - CI pipeline 3013 is the PR pipeline at exactly e4cb0f99 and was still running at review time — terminal green remains the merge gate's. Sequencing note: the base is the #1478 merge, so the stop-write work item that document declared as standing is landing immediately after it, exactly per SHARED-CONTRACT §5.1's phase ordering (stop-write before expand DDL). Verdict: APPROVED at e4cb0f99b15bb539c40592300bfa629ebec6ff48. An amend or rebase voids this approval.
fred merged commit c7a7fd07cc into next 2026-08-29 21:59:13 +00:00
Sign in to join this conversation.