brain/gateway: prohibit mission_tasks.status as a write source (M4-3a phase 1)
ci/woodpecker/pr/ci Pipeline was canceled
ci/woodpecker/pr/ci Pipeline was canceled
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.
This commit is contained in:
@@ -108,11 +108,13 @@ export class MissionsController {
|
|||||||
) {
|
) {
|
||||||
const mission = await this.brain.missions.findByIdAndUser(missionId, user.id);
|
const mission = await this.brain.missions.findByIdAndUser(missionId, user.id);
|
||||||
if (!mission) throw new NotFoundException('Mission not found');
|
if (!mission) throw new NotFoundException('Mission not found');
|
||||||
|
// dto.status is deliberately not forwarded: mission_tasks.status is
|
||||||
|
// write-prohibited through the N-1 window (SHARED-CONTRACT §5.1 phase 1);
|
||||||
|
// the repo strips it as well.
|
||||||
return this.brain.missionTasks.create({
|
return this.brain.missionTasks.create({
|
||||||
missionId,
|
missionId,
|
||||||
taskId: dto.taskId,
|
taskId: dto.taskId,
|
||||||
userId: user.id,
|
userId: user.id,
|
||||||
status: dto.status,
|
|
||||||
description: dto.description,
|
description: dto.description,
|
||||||
notes: dto.notes,
|
notes: dto.notes,
|
||||||
pr: dto.pr,
|
pr: dto.pr,
|
||||||
|
|||||||
@@ -77,6 +77,12 @@ export class CreateMissionTaskDto {
|
|||||||
@IsUUID()
|
@IsUUID()
|
||||||
taskId?: string;
|
taskId?: string;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @deprecated Accepted for N-1 wire compatibility but ignored: mission_tasks.status
|
||||||
|
* is write-prohibited through the migration window (SHARED-CONTRACT §5.1 phase 1).
|
||||||
|
* The field stays declared because the global ValidationPipe runs with
|
||||||
|
* forbidNonWhitelisted, and removing it would 400 frozen legacy consumers.
|
||||||
|
*/
|
||||||
@IsOptional()
|
@IsOptional()
|
||||||
@IsIn(taskStatuses)
|
@IsIn(taskStatuses)
|
||||||
status?: 'not-started' | 'in-progress' | 'blocked' | 'done' | 'cancelled';
|
status?: 'not-started' | 'in-progress' | 'blocked' | 'done' | 'cancelled';
|
||||||
@@ -102,6 +108,12 @@ export class UpdateMissionTaskDto {
|
|||||||
@IsUUID()
|
@IsUUID()
|
||||||
taskId?: string;
|
taskId?: string;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @deprecated Accepted for N-1 wire compatibility but ignored: mission_tasks.status
|
||||||
|
* is write-prohibited through the migration window (SHARED-CONTRACT §5.1 phase 1).
|
||||||
|
* The field stays declared because the global ValidationPipe runs with
|
||||||
|
* forbidNonWhitelisted, and removing it would 400 frozen legacy consumers.
|
||||||
|
*/
|
||||||
@IsOptional()
|
@IsOptional()
|
||||||
@IsIn(taskStatuses)
|
@IsIn(taskStatuses)
|
||||||
status?: 'not-started' | 'in-progress' | 'blocked' | 'done' | 'cancelled';
|
status?: 'not-started' | 'in-progress' | 'blocked' | 'done' | 'cancelled';
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
|
import { createMissionTasksRepo } from './mission-tasks.js';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* SHARED-CONTRACT §5.5 "mission_tasks.status write prohibition": the repo is
|
||||||
|
* the sole write path, and it must never forward a 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[]) {
|
||||||
|
const values = vi.fn((_v: unknown) => ({ returning: vi.fn().mockResolvedValue(returned) }));
|
||||||
|
return { db: { insert: vi.fn(() => ({ values })) }, values };
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeUpdateDb(returned: unknown[]) {
|
||||||
|
const set = vi.fn((_v: unknown) => ({
|
||||||
|
where: vi.fn(() => ({ returning: vi.fn().mockResolvedValue(returned) })),
|
||||||
|
}));
|
||||||
|
return { db: { update: vi.fn(() => ({ set })) }, set };
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('createMissionTasksRepo — status write prohibition', () => {
|
||||||
|
it('create strips a caller-supplied status before insert', async () => {
|
||||||
|
const { db, values } = makeInsertDb([{ id: 'mt1', status: 'not-started' }]);
|
||||||
|
const repo = createMissionTasksRepo(db as never);
|
||||||
|
|
||||||
|
const result = await repo.create({
|
||||||
|
missionId: 'm1',
|
||||||
|
userId: 'u1',
|
||||||
|
status: 'done',
|
||||||
|
description: 'd',
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
expect(values).toHaveBeenCalledTimes(1);
|
||||||
|
const inserted = values.mock.calls[0]![0] as Record<string, unknown>;
|
||||||
|
expect('status' in inserted).toBe(false);
|
||||||
|
expect(inserted.missionId).toBe('m1');
|
||||||
|
expect(inserted.description).toBe('d');
|
||||||
|
expect(result.id).toBe('mt1');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('create without status still inserts (DB default applies)', async () => {
|
||||||
|
const { db, values } = makeInsertDb([{ id: 'mt2' }]);
|
||||||
|
const repo = createMissionTasksRepo(db as never);
|
||||||
|
|
||||||
|
await repo.create({ missionId: 'm1', userId: 'u1' } as never);
|
||||||
|
|
||||||
|
const inserted = values.mock.calls[0]![0] as Record<string, unknown>;
|
||||||
|
expect('status' in inserted).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('update strips a caller-supplied status but keeps the other fields', async () => {
|
||||||
|
const { db, set } = makeUpdateDb([{ id: 'mt1', notes: 'n' }]);
|
||||||
|
const repo = createMissionTasksRepo(db as never);
|
||||||
|
|
||||||
|
const result = await repo.update('mt1', { status: 'done', notes: 'n' } as never);
|
||||||
|
|
||||||
|
expect(set).toHaveBeenCalledTimes(1);
|
||||||
|
const updated = set.mock.calls[0]![0] as Record<string, unknown>;
|
||||||
|
expect('status' in updated).toBe(false);
|
||||||
|
expect(updated.notes).toBe('n');
|
||||||
|
expect(updated.updatedAt).toBeInstanceOf(Date);
|
||||||
|
expect(result?.id).toBe('mt1');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('update with only status degenerates to a timestamp-only update', async () => {
|
||||||
|
const { db, set } = makeUpdateDb([{ id: 'mt1' }]);
|
||||||
|
const repo = createMissionTasksRepo(db as never);
|
||||||
|
|
||||||
|
await repo.update('mt1', { status: 'blocked' } as never);
|
||||||
|
|
||||||
|
const updated = set.mock.calls[0]![0] as Record<string, unknown>;
|
||||||
|
expect(Object.keys(updated)).toEqual(['updatedAt']);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -3,6 +3,18 @@ import { eq, and, type Db, missionTasks } from '@mosaicstack/db';
|
|||||||
export type MissionTask = typeof missionTasks.$inferSelect;
|
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
|
||||||
|
// write source through the N-1 window. This repo is the sole write path, so the
|
||||||
|
// field is stripped here — accepted and ignored rather than rejected, because
|
||||||
|
// the legacy surface is frozen with existing consumers kept working
|
||||||
|
// (tool-gateway-mapping.md §3.2). 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'> {
|
||||||
|
const rest = { ...data };
|
||||||
|
delete rest.status;
|
||||||
|
return rest;
|
||||||
|
}
|
||||||
|
|
||||||
export function createMissionTasksRepo(db: Db) {
|
export function createMissionTasksRepo(db: Db) {
|
||||||
return {
|
return {
|
||||||
async findByMission(missionId: string): Promise<MissionTask[]> {
|
async findByMission(missionId: string): Promise<MissionTask[]> {
|
||||||
@@ -30,14 +42,14 @@ export function createMissionTasksRepo(db: Db) {
|
|||||||
},
|
},
|
||||||
|
|
||||||
async create(data: NewMissionTask): Promise<MissionTask> {
|
async create(data: NewMissionTask): Promise<MissionTask> {
|
||||||
const rows = await db.insert(missionTasks).values(data).returning();
|
const rows = await db.insert(missionTasks).values(stripStatus(data)).returning();
|
||||||
return rows[0]!;
|
return rows[0]!;
|
||||||
},
|
},
|
||||||
|
|
||||||
async update(id: string, data: Partial<NewMissionTask>): Promise<MissionTask | undefined> {
|
async update(id: string, data: Partial<NewMissionTask>): Promise<MissionTask | undefined> {
|
||||||
const rows = await db
|
const rows = await db
|
||||||
.update(missionTasks)
|
.update(missionTasks)
|
||||||
.set({ ...data, updatedAt: new Date() })
|
.set({ ...stripStatus(data), updatedAt: new Date() })
|
||||||
.where(eq(missionTasks.id, id))
|
.where(eq(missionTasks.id, id))
|
||||||
.returning();
|
.returning();
|
||||||
return rows[0];
|
return rows[0];
|
||||||
|
|||||||
Reference in New Issue
Block a user