Files
stack/docs/reports/native-kanban-sot/canon-initial-review-no-go.md
jason.woltje 49e8a54105
All checks were successful
ci/woodpecker/push/publish Pipeline was successful
ci/woodpecker/push/ci Pipeline was successful
docs(#751): Publish native Kanban/SOT canon (#752)
2026-07-14 17:08:09 +00:00

24 KiB
Raw Blame History

Independent Review — Native Kanban/SOT Canon

Reviewer: enhance-sol (independent of author planner-sol) Date: 2026-07-13 Review mode: design/contract only; read-only against the staged canon Source plan: /home/hermes/agent-work/planning/mosaic-native-kanban-sot-plan.md (sha256:96ea4fb91436ec9a53f371d27276e27f62ecf817662599ff9152df0db55296e5) Canon reviewed: every listed artifact under /home/hermes/agent-work/planning/kanban-canon/, including the four TypeScript contracts; the author scratchpad was also read as validation context.

Executive verdict

NO-GO

The canon is not freeze-ready. I found 8 BLOCKERs, 7 MAJORs, and 1 MINOR. The prose preserves the ratified authority model well, but the frozen types/schema leave concrete fail-closed, approval, fencing, tenant, outage-proposal, migration, and parallelization gaps. Those gaps would force implementation lanes either to invent contract semantics or to ship paths that violate fixed invariants.

Blocking findings

  1. Health/write authorization can be represented as contradictory, stale, or caller-asserted state.
  2. Coordinator failures collapse authoritative denial, unknown transport outcome, and version conflict into one permissive shape.
  3. Assignment proposals and approval proofs have no authoritative relational binding; lease acquisition accepts a forgeable proof DTO.
  4. Fencing uniqueness is present, but monotonic fencing and same-task lease/checkpoint binding are not.
  5. Workspace-safe accountable-owner, assignment-principal, and evidence/artifact relationships are not frozen.
  6. Attributable post-recovery proposals have neither a canonical table nor command contract.
  7. The slice graph starts schema/UI work before prerequisite threat and exact API/DTO freezes and contradicts coder4 lane order.
  8. P0 claims a migration map while publishing only generic rules; the concrete N-1 transition from current origin/main is absent.

Findings

KCR-001 — BLOCKER — “Healthy” is not a proof and can be contradictory or stale

Location

  • contracts/health-state.v1.ts:21-31KanbanHealthResponseV1 permits every combination of state, readHealthProven, and writeHealthProven.
  • contracts/mechanical-coordinator.v1.ts:40-49CoordinatorContextV1 accepts a caller-supplied healthState enum only.
  • contracts/mechanical-coordinator.v1.ts:255-293 — every Coordinator operation, including mutating operations, accepts that context.
  • SHARED-CONTRACT.md:171-184 — mutations are allowed only after live PostgreSQL read/write probes.

Violation

Fixed invariant 2 / REQ-SOT-002: mutations must fail closed unless write health is positively proven. The current type permits { state: 'healthy', writeHealthProven: false }, and the Coordinator mutation boundary can be invoked with a stale or fabricated { healthState: 'healthy' }. A Valkey/client-derived enum could therefore be mistaken for write authorization.

Minimal fix

  1. Make KanbanHealthResponseV1 a discriminated union with only these legal combinations: healthy => read=true/write=true, read-only-degraded => read=true/write=false, and write-unavailable => read=false/write=false.
  2. Do not accept write authority from a public DTO. Require Gateway/domain code to obtain and revalidate a fresh internal PostgreSQL write-health proof at mutation time (including checkedAt, bounded validity/policy revision, and transaction-local enforcement).
  3. Split pure evaluation context from mutation context; mutation methods must accept only an unforgeable/internal healthy context or perform the probe themselves.
  4. Add negative contract tests for contradictory state, expired proof, Valkey-only liveness, and caller-forged healthy.

KCR-002 — BLOCKER — Coordinator error shape can conflate denial, unknown outcome, and conflict

Location

  • contracts/mechanical-coordinator.v1.ts:184-216 — one CoordinatorFailureV1 allows every code to pair with arbitrary retryable and either requestOutcome value.
  • contracts/health-state.v1.ts:51-106 — the Gateway health contract correctly distinguishes deliberate denial, transport uncertainty, and version conflict.
  • SHARED-CONTRACT.md:177-216 — frozen client semantics require those cases not to be conflated.

Violation

Charter E and REQ-SOT-002. The current Coordinator result can legally encode WRITE_HEALTH_UNPROVEN as retryable: true, requestOutcome: 'unknown', or VERSION_CONFLICT as retryable. That permits blind retry or a false “unknown” outcome after an authoritative fail-closed denial.

Minimal fix

Replace CoordinatorFailureV1 with a discriminated union keyed by code/kind:

  • deliberate health denial: not_applied, retryable:false;
  • version conflict: not_applied, retryable:false, current version;
  • stale fence/session/eligibility/approval failures: exact non-retry semantics;
  • transport failure: a separate retryable_transport_error, unknown, same idempotency key.

Reuse or map explicitly to KanbanMutationFailureV1, and add exhaustive client tests proving 503 authoritative bodies, 502/504/timeouts, and 409 cannot cross-map.

KCR-003 — BLOCKER — Approval proof is forgeable and is not linked to the persisted proposal

Location

  • contracts/mechanical-coordinator.v1.ts:107-137 — proposal and approval DTOs.
  • contracts/mechanical-coordinator.v1.ts:265-270acquireApprovedLease accepts the entire ApprovalProofV1 by value.
  • contracts/kanban-schema.v1.ts:650-688task_assignments has no proposal expiry, task version, session binding, or proposal/approval FK.
  • contracts/kanban-schema.v1.ts:823-863approval_decisions can target only a task or mission and has no proposal/assignment relation.
  • SHARED-CONTRACT.md:128-137 — lease acquisition requires authoritative approval under the exact policy revision.

Violation

Fixed invariant 5 and REQ-COORD-002/003. A caller can construct an ApprovalProofV1; the schema cannot prove that it belongs to the proposal, workspace, task version, agent/session, unexpired policy revision, or still-current approval. The DTO state vocabulary (awaiting_approval | policy_pre_authorized) also does not map directly to the persisted assignment states (proposed | approved | ...).

Minimal fix

Persist one authoritative proposal/assignment identity with task version, target agent/session, expiry, state, and policy revision. Add a workspace-aware approval relation to that identity. Change lease acquisition to accept IDs, then reload and lock proposal + approval + task inside PostgreSQL and verify workspace, current version, target session, state, expiry, and policy revision before creating the lease. Freeze one state vocabulary across schema and DTOs.

KCR-004 — BLOCKER — Fencing is unique but not monotonically increasing; relational binding is incomplete

Location

  • contracts/kanban-schema.v1.ts:694-743task_leases has positive/unique fencing tokens but no monotonic per-task counter.
  • contracts/kanban-schema.v1.ts:748-784 — checkpoints independently carry task, lease, and fencing token.
  • contracts/kanban-schema.v1.ts:905-910 — token equality is deferred to prose; same-task lease binding is not stated.
  • contracts/mechanical-coordinator.v1.ts:140-175 — worker commands depend on fencing safety.

Violation

Fixed invariant 12 / REQ-COORD-003. Uniqueness permits token 10 followed by token 9. A lease can reference assignment A while naming task B in the same workspace, and a checkpoint can reference lease A while naming task B. bigint(..., { mode: 'number' }) also eventually loses integer precision in JavaScript.

Minimal fix

Add a durable per-task fencing counter (or equivalent PostgreSQL sequence row) incremented atomically under task lock and use the returned value for every new lease. Add workspace-aware composite constraints tying lease to its exact task+assignment and checkpoint to exact task+lease+fence. Use bigint-safe representation (bigint/serialized decimal), and test monotonicity, concurrent claims, stale lower tokens, and mismatched same-workspace IDs.

KCR-005 — BLOCKER — Hard tenant boundary is not frozen for several polymorphic relationships

Location

  • contracts/kanban-schema.v1.ts:315-318 and 475-478 — project/task accountable owners are unvalidated (kind, text id) pairs.
  • contracts/kanban-schema.v1.ts:659-663 — assignment principals are unvalidated (kind, text id) pairs.
  • contracts/kanban-schema.v1.ts:758 and 841 — checkpoint/evidence artifact relationships are JSON arrays without workspace-aware FKs.
  • SHARED-CONTRACT.md:89-96 — only selected polymorphic checks are delegated to domain transactions; owner/principal/evidence checks are not included.
  • REQUIREMENTS.md:101-108 — every relationship must reject cross-workspace IDs.

Violation

Fixed invariant 7 / REQ-TEN-001 and REQ-ID-001. The frozen schema can name a team or agent from another workspace as owner/assignee, and can embed foreign-workspace artifact IDs in checkpoint or approval evidence arrays. A global user ID is also insufficient without active workspace membership validation.

Minimal fix

Use workspace-aware owner/assignment join tables or separate nullable user/team/agent columns with exactly-one checks and composite FKs where possible. Model checkpoint/evidence artifact links as workspace-scoped join rows, or freeze explicit transaction checks for every ID. Require active workspace membership for user principals and workspace-agent/session consistency for agent principals. Add DB/repository/API/Coordinator cross-workspace negative tests without existence oracles.

KCR-006 — BLOCKER — Post-recovery outage proposals have no canonical persistence or command surface

Location

  • REQUIREMENTS.md:93-99 — proposal submission and authorized accept/reject are required.
  • SHARED-CONTRACT.md:26-29 — outage notes may return only as authenticated proposals.
  • SHARED-CONTRACT.md:243-267 — the thin command/query contract contains no proposal submit/get/accept/reject operations.
  • contracts/kanban-schema.v1.ts:1-916 — no proposal table captures proposed command, target/version, attribution, lifecycle, or decision.
  • TASKS.md:99-108 — KBN-110 does not own an outage-proposal command path.

Violation

Fixed invariant 4 / REQ-SOT-004. An implementation lane would have to invent storage or misuse artifacts/approval gates. Either path risks silently applying an outage note or creating shadow state.

Minimal fix

Add a workspace-scoped change_proposals/outage_proposals contract with authenticated proposer, source note digest, target aggregate, expected version, proposed typed command/payload, pending/accepted/rejected state, decision actor/reason/time, idempotency key, and audit linkage. Add explicit submit/query/accept/reject Gateway commands. Acceptance must execute the normal command in a healthy transaction; a proposal itself can never claim, order, satisfy a gate, or mutate the target.

KCR-007 — BLOCKER — Parallel slice ordering is not freeze-safe and contains a direct lane-order contradiction

Location

  • TASKS.md:43-60 — dependency graph makes KBN-010 and KBN-100 siblings.
  • TASKS.md:88-97 — KBN-100 nevertheless depends on KBN-010 threat findings that alter constraints.
  • SHARED-CONTRACT.md:243 and INDEX.md:44-50 — exact route names/DTO placement remain unresolved.
  • TASKS.md:110-130 — KBN-120/130 depend on a frozen endpoint/DTO contract, while mocks may begin before KBN-110 lands.
  • TASKS.md:145-153 — KBN-200 says lane-serial after KBN-120.
  • TASKS.md:248-254 — wave table runs KBN-200 before KBN-120.

Violation

Charter C and the mandatory freeze-before-parallelize gate. Schema can begin before tenant/threat findings are complete; web/CLI consumers have only semantic operations, not exact DTO/endpoint contracts; coder4 has two opposite legal orders. This does not create same-file edits immediately, but it guarantees contract invention or rework across active lanes.

Minimal fix

  1. Make KBN-010 (or an explicit constraint-impact gate from it) a completed prerequisite of KBN-100.
  2. Add a small serialized KBN-105 endpoint/DTO/endpoint-registry freeze, with exact request/response/error DTOs, before KBN-120 and KBN-130 implementation.
  3. Choose one coder4 lane order and use it consistently in slice text, graph, and wave table.
  4. Name the exact MCP-owned files or assign their Gateway changes to coder3 before coder4 starts.

KCR-008 — BLOCKER — Claimed P0 migration map is absent; concrete N-1 hazards remain unresolved

Location

  • MISSION-MANIFEST.md:153-157 — P0 says to publish a migration map and states the build hold is lifted at line 3.
  • SHARED-CONTRACT.md:101-121 — only generic expand/backfill/contract rules are supplied.
  • contracts/kanban-schema.v1.ts:1-916 — target-state declarations reuse live table names and make target fields required.
  • Current foundation evidence: origin/main:packages/db/src/schema.ts:120-301 has no workspace keys, nullable project/mission links, legacy text status vocabularies, tasks.tags, tasks.assignee, tasks.due_date, mission JSON milestones/config, mission_tasks.status, and legacy agent fields.

Violation

Charter D / REQ-MIG-001 and the P0 exit claim. The generic rule is correct, but coder2 lacks the required field-by-field transition map. A direct Drizzle reconciliation could attempt type narrowing/status conversion, add required workspace/project/owner columns too early, or drop legacy columns before N-1 readers and writers are retired.

Minimal fix

Publish a concrete current-main delta map before lifting the hold. For each existing table/column, specify expand, backfill, compatibility read/write, switch, and contract release. At minimum cover:

  • nullable-first workspace_id, required project/owner fields, and workspace backfill;
  • legacy task/project/mission status aliases or shadow columns before v1 emission;
  • mission_tasks.status read retirement and write-source prohibition;
  • mapping/retention for tags, assignee, due date, mission description/config/milestones, and agent fields;
  • current milestone circular FK ordering;
  • empty, production-shape, partial-resume, and rollback/downgrade tests already named in §4.

Explicitly require legacy columns to remain in the unified Drizzle declaration during the expand/N-1 window.

KCR-009 — MAJOR — Dependency uniqueness permits parallel duplicate edges

Location

  • contracts/kanban-schema.v1.ts:561-567 — unique key includes dependencyType.
  • SHARED-CONTRACT.md:47-49 — calls for a unique directed edge.
  • REQUIREMENTS.md:142-149 — duplicate edge attempts must fail.

Violation

REQ-DEP-001. The same predecessor/successor pair can be inserted three times, once per dependency type. That is not a unique directed edge and complicates readiness semantics.

Minimal fix

Make (workspace_id, predecessor_task_id, successor_task_id) unique independent of type, or explicitly redefine the requirement as one edge per type and freeze deterministic multi-edge completion semantics. The source plan says unique directed edge, so the former is the minimal faithful fix.

KCR-010 — MAJOR — Same-workspace planning relationships can contradict the project hierarchy

Location

  • contracts/kanban-schema.v1.ts:325projects.currentMilestoneId has no FK in the declaration.
  • contracts/kanban-schema.v1.ts:427-448 — mission/milestone association checks workspace but not common project.
  • contracts/kanban-schema.v1.ts:490-516 — a tasks project, mission, milestone, and parent only need share a workspace, not a project.
  • contracts/kanban-schema.v1.ts:905-908 — only current milestone is mentioned as a deferred invariant.

Violation

REQ-PLAN-001 and schema correctness. A task in project A can point to a mission/milestone/parent task from project B in the same workspace. A mission can associate a milestone from another project despite having one required project.

Minimal fix

Add project-congruent composite keys/FKs (or freeze mandatory transaction checks) for task→mission, task→milestone, task→parent, mission→milestone, and project→current milestone. Add same-workspace/same-project negative tests.

KCR-011 — MAJOR — Immutable/append-only records can be erased by parent cascades

Location

  • contracts/kanban-schema.v1.ts:798-818task_events is described as append-only but remains under a workspace cascade.
  • contracts/kanban-schema.v1.ts:911 — only application-role UPDATE/DELETE privilege removal is stated.
  • Numerous canonical relationships use onDelete('cascade'), including workspace roots and artifact/checkpoint/event owners.
  • REQUIREMENTS.md:41-43 and 154-170 — audit must be append-only, attributable, and reconstructable.

Violation

REQ-AUD-001. Revoking direct DELETE on task_events does not prevent a parent delete from cascading into the audit log. Checkpoints and immutable artifacts also lack explicit append-only privilege/retention semantics.

Minimal fix

Use lifecycle/archive states and RESTRICT for canonical parent deletion during normal operation. Freeze a separate, audited retention/break-glass purge procedure. Apply INSERT/SELECT-only or equivalent immutability controls to task events, checkpoints, and immutable artifacts, and test that parent deletion cannot silently erase them.

KCR-012 — MAJOR — Coordinator persistence lacks durable quarantine/retry state and DTO/schema alignment

Location

  • contracts/mechanical-coordinator.v1.ts:239-244 — expiry returns quarantined IDs.
  • contracts/kanban-schema.v1.ts:457-490 — task has only untyped retryPolicy metadata and no quarantine/execution disposition.
  • contracts/mechanical-coordinator.v1.ts:173evidenceIds has no corresponding evidence table/type; schema has artifacts.
  • contracts/kanban-schema.v1.ts:479, 663, and agent role JSON — specialist roles are free text despite the frozen role vocabulary in mechanical-coordinator.v1.ts:19-29.

Violation

REQ-COORD-004 and internal consistency. PostgreSQL cannot deterministically reconstruct why/when a task was quarantined, its bounded retry state, or which typed evidence was submitted. Free-text roles allow the schema and engine to disagree.

Minimal fix

Freeze a durable execution/retry/quarantine record (attempt count, next eligibility, terminal reason, actor/policy, timestamps, version) or typed task columns with events. Align evidenceIds to artifact IDs or add a real evidence entity. Use one specialist-role enum/check across tasks, assignments, agents/sessions, DTOs, and Coordinator.

KCR-013 — MAJOR — Thin MVP promises task archive and tag filtering without target-state semantics

Location

  • REQUIREMENTS.md:182-193 — users must archive tasks and filter by tags.
  • SHARED-CONTRACT.md:252-267 — mutations include cancel but not archive task.
  • contracts/kanban-schema.v1.ts:457-490 — no task archive field and no typed tags field/table.
  • Current origin/main already has tasks.tags, making omission from the target declaration a migration-loss hazard.

Violation

REQ-UI-001/002 and internal acceptance consistency. “Archive” cannot be implemented without inventing whether it means cancelled, hidden, or soft-deleted; tag filtering has no frozen storage/query contract.

Minimal fix

Either remove task archive/tag acceptance from P1, or add explicit non-lifecycle archival semantics (archived_at/by/reason) and a workspace-safe tags model/query contract. Preserve/migrate the current tags column until the selected model is live.

KCR-014 — MAJOR — Recovery contract states critical rules only in comments and has no owning implementation slice

Location

  • contracts/recovery-posture.v1.ts:97-147 — exported JSON Schema validates only local field shapes.
  • contracts/recovery-posture.v1.ts:150-156 — PITR/WAL, effective RPO, off-cluster, high-assurance minima, and audit rules are comments only.
  • REQUIREMENTS.md:270-277 — parser rejection of impossible combinations is acceptance-critical.
  • TASKS.md:75-244 — no bounded slice owns recovery config parsing, override audit, backup/WAL setup, or restore/break-glass evidence.

Violation

REQ-REC-001. A consumer using the advertised JSON Schema can accept weakened high-assurance values, PITR without WAL, or an impossible RPO. The task plan has no lane accountable for closing that acceptance criterion.

Minimal fix

Export a normative validateRecoveryPostureV1/schema refinement with machine-testable cross-field checks and add a bounded Infra/recovery slice (serialized if it touches shared config) owning config parsing, override audit, mechanism verification, restore test, and break-glass evidence. Recovery config must continue to expose no authority/gate knobs.

KCR-015 — MAJOR — Pure Coordinator slice cannot implement two frozen methods without persistence access

Location

  • contracts/mechanical-coordinator.v1.ts:259-263explainEligibility receives only taskId, not a structured snapshot.
  • contracts/mechanical-coordinator.v1.ts:289-293recoverFromPostgres explicitly reads PostgreSQL.
  • TASKS.md:145-153 — KBN-200 is a pure engine with no SQL, Drizzle, Gateway, or Valkey.
  • TASKS.md:157-164 — persistence belongs to coder3/KBN-210.

Violation

Charter C and internal consistency. coder4 cannot implement the frozen port in a pure package without crossing coder3s persistence boundary. If coder3 implements the port instead, KBN-200s acceptance and ownership are misassigned.

Minimal fix

Split the contract into a pure decision engine that receives complete immutable snapshots and a persistence/orchestration service port implemented by KBN-210. Move recoverFromPostgres and ID-based loading to the adapter/service; make pure explanation accept a snapshot.

KCR-016 — MINOR — Health denial code/state pairs are not correlated by type

Location

  • contracts/health-state.v1.ts:35-61 — either denial code can pair with either degraded state.
  • SHARED-CONTRACT.md:188-190 — prose defines KANBAN_WRITE_UNAVAILABLE specifically for write-unavailable.

Violation

Health contract precision. A client can receive a semantically inconsistent authoritative body even after KCR-001s broader state fix.

Minimal fix

Make deliberate denial a two-variant union with exact code/state pairing.


Clean checks / invariants that do hold

The review did not find a gap in these areas:

  • The canon consistently selects current mosaicstack/stack + Drizzle/PostgreSQL and rejects greenfield/Prisma revival.
  • Every artifact states PostgreSQL is the sole writable SOT and Valkey/files are non-authoritative.
  • Generated TASKS.md, mission.json, and exports are consistently declared read-only and never import sources. KBN-300s importer is scoped to immutable legacy JSON/Vikunja snapshots, not generated projections.
  • Recovery config exposes recovery fields only; it contains no direct fail-open, SOT, Coordinator-authority, or gate-waiver knob.
  • The Coordinator interface contains no createTask, acceptance-edit, gate-waive, certify, merge, release, or provider-close method. submitForReview is type-limited to in_review, not done or certified.
  • Certifier is consistently final independent gate with no merge authority.
  • The seven canonical task status values match across requirements, shared prose, schema, and Coordinators ready/in-review surfaces.
  • One-active-lease partial uniqueness, no-self-edge, outbox aggregate-revision/event-type uniqueness, optimistic task/project/mission/milestone versions, and N-1 test categories are explicitly present.
  • The file-tree partition is mostly well separated once the ordering/freeze defects in KCR-007 are corrected.

Required re-review scope

After remediation, re-review at minimum:

  1. health/coordinator discriminated unions and mutation-time health proof;
  2. proposal/approval/assignment/lease relational model;
  3. monotonic fencing and composite bindings;
  4. tenant-safe polymorphic relationships;
  5. outage-proposal persistence and commands;
  6. concrete current-main migration map;
  7. corrected dependency graph and exact API/DTO freeze;
  8. recovery validator/owner slice;
  9. all schema and DTO vocabulary alignment.

Overall verdict

NO-GO — 8 BLOCKERs must be resolved before the v1 contract is frozen or parallel implementation begins.