docs(phase2): mark S4 merged in status table#9
Merged
screenleon merged 1 commit intomainfrom Apr 22, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Phase 2 planning workspace design doc’s Status tracking table to reflect that slice S4 (applied lineage) has been merged via PR #8, keeping the design doc’s progress tracking in sync with shipped work.
Changes:
- Mark S4 as merged in the status table
- Update the “Merged” column indicator for S4 to ✓
screenleon
added a commit
that referenced
this pull request
Apr 27, 2026
…talog enforcement (#27) * feat(phase6c-pr2): authoring lifecycle + actor_audit + multi-point catalog enforcement Phase 6c PR-2 (per docs/phase6c-plan.md v5.1 §3.2). Closes the catch-22 where role_dispatch was UI-disabled because no surface existed to set candidate.execution_role. The role choice now flows through the apply payload (and an inline candidate editor), is validated against the catalog at four entry points, and every mutation writes a structured audit row. ## Backend - Migration 030: new actor_audit table (generic discriminator on subject_kind so other fields can adopt the same model later). Append-only, no FK on subject_id (history-preserving by design). - New backend/internal/audit/ package: Record(tx, ...) inside caller's transaction; QueryLatest(ctx, db, ...) for reads. ActorKind enum: user / api_key / router / system / connector. Confidence is rejected for non-router actors so downstream router-only queries cannot be polluted (critic round 1 #1). - Role.Category field on backend/internal/roles/catalog.go (all 6 roles tagged "role"; the dispatcher meta-role lands in PR-3). TestCatalogMatchesPromptDir now cross-checks the frontmatter category column. - BacklogCandidateStore.Update(id, req, actor) and ApplyToTaskWithMode (id, mode, executionRole, actor) signatures changed to carry ActorInfo. Both validate execution_role against the catalog, write the column, and emit the audit row inside the same transaction. New typed errors: ErrBacklogCandidateMissingExecutionRole, ErrBacklogCandidateUnknownExecutionRole. - BacklogCandidateStore.EnrichWithAuthoring populates BacklogCandidate.ExecutionRoleAuthoring from the latest audit row so the GET / PATCH / apply responses surface the who/when/rationale trail without a second API round-trip (risk-reviewer H1 fix). Pre-Phase-6c rows have no audit history and surface null — backfill is intentionally not done. - TaskStore.ClaimNextDispatchTask drains stale-role tasks (role no longer in catalog) by transitioning queued -> failed with error_kind=role_not_found and an actor_kind=system audit row, then continues to find the next valid task. Drain is bounded by staleDrainCap (16 per call) so a poisoned queue cannot wedge the connector loop (critic round 1 #6). - New GET /api/roles handler (public, no auth — catalog is public source data). Filters category="role"; meta-roles introduced in PR-3 will be excluded from this surface. - New buildAuthoringActor(r, rationale) helper: routes session callers to ActorUser, API-key callers to ActorAPIKey (id="api-key:<key id>"), else logs an "unauthenticated" defense-in-depth row. Resolves critic round 1 #3 + risk-reviewer M1 (the apply/PATCH paths previously collapsed both auth channels into ActorUser with empty actor_id when api-key was active). ## Frontend - New types/roles.ts: KNOWN_ROLE_IDS hand-mirror of the Go catalog + isKnownRoleId narrow function. Drift-detected by roles.test.ts which fetches /api/roles via mock and asserts set equality. - New ExecutionRoleAuthoring type on BacklogCandidate; the candidate response now carries who/when/why for execution_role. - usePlanningWorkspaceData fetches /api/roles on mount; availableRoles is RoleInfo[] | null with the null sentinel meaning "still loading" so the panel can suppress the stale-role warning during the fetch (critic round 1 #5 + risk-reviewer L1). - CandidateReviewPanel: role_dispatch radio is always enabled (the Phase 5 disabled-on-no-role gating is removed). When selected, an inline role <select> shows each catalog role with version + estimated minutes. Apply button is disabled until a role is chosen. Stale-role warning fires only after /api/roles resolves with the candidate's role missing from it. - New CandidateRoleEditor: chip + inline edit popover so operators can pre-tag candidates with a role outside the apply flow. PATCH carries the actor through the same audit path as the apply endpoint. ## Tests - audit/audit_test.go: 9 tests covering record + query + rollback + concurrent insert + ordering + router confidence required + non-router confidence rejected. - store: replaced two Phase 5 contract tests ("unknown role accepted", "rune-truncation defense") with Phase 6c PR-2 catalog-enforcement counterparts. Existing Update callers updated to pass audit.ActorInfo{}. - handlers/connector_dispatch_test.go: three new tests covering TestClaimNextTask_StaleRoleTransitionsToFailed, TestClaimNextTask_DrainsStaleRoleThenClaimsNext, TestRolesEndpoint_ReturnsCatalog. - frontend: 7 CandidateRoleEditor tests, 5 new CandidateReviewPanel tests covering role select rendering + Apply gating + stale warning, 6 drift tests against /api/roles. ## Reviews completed - make pre-pr: green twice (SQLite + PostgreSQL + frontend build + lint + typecheck). - critic subagent (round 1): 10 findings, 4 Mandatory all addressed (#1 confidence rule, #3 actor disambiguation, #5 stale-warning race, #6 drain loop cap). 3 Should-fix addressed (#2 documented, #7 documented, #9 verified). - /security-review: 0 findings at confidence >= 8. - risk-reviewer: 3 HIGH, 4 MED, 6 LOW. HIGH/MED all addressed inline (H1 audit read path via EnrichWithAuthoring; H2 DECISIONS entry added; H3 documented as PR-4 surfacing path; M1 buildAuthoringActor; M2/M3 docs/api-surface.md + docs/data-model.md updated). ## Docs - docs/api-surface.md: PATCH + apply contracts updated, new GET /api/roles documented, breaking-change call-out for the apply payload. - docs/data-model.md: actor_audit table documented; execution_role description updated. - DECISIONS.md: new 2026-04-26 entry recording the implementation refinements that diverged from the 2026-04-25 entry's pre-implementation API names + cascade- delete description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(phase6c-pr2): address Copilot PR #27 review feedback Six line-level findings from copilot-pull-request-reviewer on PR #27, all addressed without simple/temporary workarounds: 1. CandidateReviewPanel pass-through: stop coercing the null loading sentinel to []; pass `availableRoles ?? null` so CandidateRoleEditor keeps its loading-state suppression. 2. CandidateRoleEditor staleness SoT: once /api/roles loads, base isStale on the runtime catalog (`availableRoles.find`), and only fall back to the static KNOWN_ROLE_IDS mirror while still loading. Survives staggered deploys where backend catalog leads frontend. 3. usePlanningWorkspaceData fetch failure: keep availableRoles=null on catch (never []), and add a sibling availableRolesError string state plumbed through PlanningTab → both panels. Editor + dropdown render an explicit "Failed to load roles: …" message so operators can distinguish transient failure from "catalog is empty". 4. task_store.parseRoleIDFromSource: split malformed-source from role-not-found. New error_kind=role_dispatch_malformed (with its own remediation hint) is emitted when the task source has no role suffix; role_not_found stays for catalog absence with a well-formed id. Helper now returns (roleID, hasSuffix). 5. Migration 030 comment: enumerate all five actor_kind values (added api_key) and remove an embedded `;` that re-tripped the SQLite migration-parser comment-split bug. 6. audit package: split ErrConfidenceNotAllowed (non-router supplied confidence) from ErrInvalidConfidence (router missing/out-of-range). Logs/debug now describe the actual failure mode. pre-pr (lint + SQLite + PostgreSQL + frontend typecheck + vitest + production build): all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(phase6c-pr2): critic round-4 follow-up — connector parity, tests, docs Critic round 4 raised two Mandatory and three Suggested findings on the prior commit (72e066d). All addressed: M1 — Connector parser parity. backend/internal/connector/service.go RunOnceTask was emitting ErrorKind: "unknown" for both the missing- suffix and unknown-role branches. Now mirrors the server: emits models.ErrorKindRoleDispatchMalformed for missing/empty role suffix and models.ErrorKindRoleNotFound for well-formed-but-absent ids, matching the diagnostic the server uses on the claim path. Updated godoc on parseRoleIDFromSource to reflect this lockstep. M2 — Backfill obligation documented. New 2026-04-27 DECISIONS entry constraint (d) explicitly waives backfill: Phase 6c is greenfield, no live deployment carries tasks rows with source="role_dispatch" (no colon), so the new error_kind has no historical backfill obligation. Future operators who observe this kind should investigate the candidate-applier code path. S1 — Malformed-source store-layer test. New TestClaimNextTask_MalformedSourceTransitionsToFailed in connector_dispatch_test.go covers all three sub-cases (no_colon, empty_suffix, whitespace_suffix) and asserts error_kind + error_message + audit row. Added seedQueuedTaskWithSource helper to seed arbitrary source strings. S2 — Frontend error-UI test coverage. Two new CandidateRoleEditor.test.tsx tests: - shows "Failed to load roles: …" alert when availableRolesError is set and the editor is opened - does NOT flag stale while availableRoles is loading (null) without an error (regression guard for the suppression logic). S4 — Server-push catalog assumption documented inline. New code comment in CandidateRoleEditor.tsx flags the assumption that availableRoles is fetched once on mount and never mutated mid- session, so a future Phase 6c PR-4/PR-5 SSE change to push catalog updates needs to revisit the staleness check. pre-pr (lint + SQLite + PostgreSQL + frontend typecheck + vitest + production build): all green. risk-reviewer: zero HIGH/MEDIUM, six LOW (all dispositioned). /security-review: zero findings at confidence >= 8. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trivial design-doc follow-up. Updates the §Status tracking table so S4 shows as merged with PR #8. No code change;
lint-governanceis the only gate.