review r1 fix: sole-authoring-path precision; storage transport/adapter disposition recorded in code and inventory doc
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
This commit is contained in:
@@ -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` /
|
||||||
|
|||||||
@@ -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[]) {
|
||||||
|
|||||||
@@ -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;
|
||||||
|
|||||||
Reference in New Issue
Block a user