review r1 fix: sole-authoring-path precision; storage transport/adapter disposition recorded in code and inventory doc
ci/woodpecker/pr/ci Pipeline was successful

This commit is contained in:
fred
2026-08-29 16:36:26 -05:00
parent 931542ee19
commit e4cb0f99b1
3 changed files with 29 additions and 11 deletions
@@ -73,12 +73,22 @@ SHARED-CONTRACT.md §5.1 phase 1 requires an N-1 patch that stops
`mission_tasks.status` as a write source, plus a writer inventory, before any `mission_tasks.status` as a write source, plus a writer inventory, before any
expand DDL. expand DDL.
- **Sole status write path:** `packages/brain/src/mission-tasks.ts` - **Sole authoring write path:** `packages/brain/src/mission-tasks.ts`
`create`/`update` (Drizzle insert/update on `mission_tasks`), invoked by `create`/`update` (Drizzle insert/update on `mission_tasks`), invoked by
`apps/gateway/src/missions/missions.controller.ts`. `update` accepts `apps/gateway/src/missions/missions.controller.ts`. `update` accepts
`Partial<NewMissionTask>`, so `status` is writable through both DTOs today. `Partial<NewMissionTask>`, so `status` is writable through both DTOs today.
The same module also exposes `remove`/`removeByMission` DELETE paths — The same module also exposes `remove`/`removeByMission` DELETE paths —
immaterial to `status` writes, listed for inventory completeness. immaterial to `status` writes, listed for inventory completeness.
- **Storage-layer surfaces that touch the column without authoring it**
(added 2026-08-29 after independent review of the phase-1 patch):
`packages/storage/src/migrate-tier.ts` copies whole `mission_tasks` rows
between storage tiers and must preserve the stored `status` verbatim — row
transport, exempt from the write prohibition (stripping there would corrupt
data inside the N-1 window). The generic table-keyed storage adapters
(`adapters/postgres.ts`, `adapters/pglite.ts`) register `mission_tasks` in
their table maps but have no caller that targets it: measured at this head,
every runtime adapter caller passes a fixed collection constant
(preferences/insights). Neither surface authors a new `status` value.
- **Read-only consumers of `mission_tasks`:** federation verb services - **Read-only consumers of `mission_tasks`:** federation verb services
(`get-query.service.ts`, `list-query.service.ts`) select only. The MCP (`get-query.service.ts`, `list-query.service.ts`) select only. The MCP
`brain_*` tools do not touch `mission_tasks` at all; `brain_create_task` / `brain_*` tools do not touch `mission_tasks` at all; `brain_create_task` /
+7 -5
View File
@@ -2,11 +2,13 @@ import { describe, it, expect, vi } from 'vitest';
import { createMissionTasksRepo } from './mission-tasks.js'; import { createMissionTasksRepo } from './mission-tasks.js';
/** /**
* SHARED-CONTRACT §5.5 "mission_tasks.status write prohibition": the repo is * SHARED-CONTRACT §5.5 "mission_tasks.status write prohibition": this repo is
* the sole write path, and it must never forward a caller-supplied status to * the sole path that authors mission_tasks.status from caller input (storage
* the database on create or update. Callers keep working (the field is * tier migration is row transport and preserves stored values; the generic
* accepted and ignored), so these tests assert on what reaches the Drizzle * storage adapters have no mission_tasks caller), and it must never forward a
* chain, not on rejection. * caller-supplied status to the database on create or update. Callers keep
* working (the field is accepted and ignored), so these tests assert on what
* reaches the Drizzle chain, not on rejection.
*/ */
function makeInsertDb(returned: unknown[]) { function makeInsertDb(returned: unknown[]) {
+11 -5
View File
@@ -4,11 +4,17 @@ export type MissionTask = typeof missionTasks.$inferSelect;
export type NewMissionTask = typeof missionTasks.$inferInsert; export type NewMissionTask = typeof missionTasks.$inferInsert;
// SHARED-CONTRACT §5.1 phase 1 / §5.4: mission_tasks.status is prohibited as a // SHARED-CONTRACT §5.1 phase 1 / §5.4: mission_tasks.status is prohibited as a
// write source through the N-1 window. This repo is the sole write path, so the // write source through the N-1 window. This repo is the sole path that authors
// field is stripped here — accepted and ignored rather than rejected, because // status from caller input, so the field is stripped here — accepted and
// the legacy surface is frozen with existing consumers kept working // ignored rather than rejected, because the legacy surface is frozen with
// (tool-gateway-mapping.md §3.2). The column keeps its DB default, stays // existing consumers kept working (tool-gateway-mapping.md §3.2). Two other
// declared and readable, and is retired only after no readers remain. // surfaces touch the column and are deliberately NOT stripped:
// packages/storage/migrate-tier.ts copies whole rows between storage tiers and
// must preserve the stored value verbatim, and the generic table-keyed storage
// adapters register mission_tasks but have no caller that targets it (runtime
// callers use fixed collection constants). Neither authors a new status. The
// column keeps its DB default, stays declared and readable, and is retired
// only after no readers remain.
function stripStatus<T extends { status?: unknown }>(data: T): Omit<T, 'status'> { function stripStatus<T extends { status?: unknown }>(data: T): Omit<T, 'status'> {
const rest = { ...data }; const rest = { ...data };
delete rest.status; delete rest.status;