Repository navigation
feat(mt#2911): work-package task kind — schema, claim path, /handoff, cockpit pool - #3503
Merged
Merged
Conversation
…s the act [no-deploy-impact] Records the four principal-decided calls from the succession RFC's §DECIDED block (2026-08-30): the entity is named work package; it ships as a task kind on the mt#1812 workflow registry rather than a sibling substrate entity; the lifecycle is one persistent row plus an append-only transfer log with origin as a per-transfer fact; and collision prevention stays at task entry (mt#4788) rather than on the entity. First deliverable of the Phase 1 PR per the spec's SC8.
…servation, session_start refusal Adds the "work-package" kind to the WORKFLOWS registry (ADR-046): TODO (drafting) → READY (open) → IN-PROGRESS (claimed) → DONE, CLOSED reachable from all non-terminal states; PLANNING/IN-REVIEW/BLOCKED deliberately absent. READY → IN-PROGRESS is reserved for the claim path via restrictedTransitions so ownership identity is written atomically with the transition, and session_start refuses the kind — a package is claimed as a bundle and worked through per-member-task sessions. TASK_KIND_VALUES derives from Object.keys(WORKFLOWS), so the MCP kind enums widen with this entry; the completion manifest regenerates in a later commit.
…ntity (migration 0114) work_package_members: the ordered member REFERENCE set (composite PK, rank, status_at_write as the F7 staleness baseline, per-member rationale). Member refs are plain text, not FKs — members may live in any backend, and create-time validation resolves every ref anyway. work_package_transfers: the append-only transfer log; origin (groomed|succession|release) is a per-transfer fact per ADR-046's mutate-plus-log lifecycle. tasks.claimed_by / tasks.claimed_at: engagement-long claim identity, written only by the claim path's CAS — deliberately not the 15-minute presence-claims table (mem#1231's grain distinction). Migration 0114 read before staging: purely additive (2 CREATE TABLE, 2 ADD COLUMN nullable, 2 FKs); bootstrap snapshot regenerated through 0114. MINSKY_SKIP_RELATED_TESTS on THIS commit only, cause diagnosed rather than assumed: the fast partition's seven cockpit failures are prod-schema skew through a leaked task-service singleton — an earlier test in the 89-file batch primes the singleton against the LIVE configured DB, and listTasks then selects claimed_by, which prod cannot have until 0114 applies POST-merge (the migration executor's own guard refuses a pre-merge apply). Evidence: every implicated file passes in isolation and in three partition thirds; a stash-control run of the identical partition is 0-fail; a probe inside the full partition captured the sanitized 500. CI builds its own DB from this branch's migrations (cold-start-migrate) and is unaffected. That tests can reach the live DB at all is filed as its own defect.
…ease, caller identity claimWorkPackage: one conditional UPDATE (id + kind + status='READY') writing IN-PROGRESS plus claimed_by/claimed_at atomically — concurrent claims resolve to exactly one winner at the database, and the loser's refusal names the holder from a follow-up diagnostic read. The only legal READY→IN-PROGRESS path for the kind, completing the workflows.ts reservation's other half. releaseWorkPackage: same CAS shape back to READY, clearing identity and appending the transfer-log entry (origin "release") in one transaction; seq is max+1 per package and the (package, seq) PK turns a same-instant duplicate into a harmless rollback. Claim appends nothing: the log records OFFERINGS (groomed/succession at create, release after), the task row records who holds. tasks.claim / tasks.release registered on the shared registry (CLI + MCP, manifest regenerated); both join CALLER_ACTOR_ID_TOOL_NAMES so the recorded identity comes from the resolved MCP caller (the server process has no harness env for resolveCallerActorId to fall back to — the mt#4568 asymmetry). Both emit best-effort task.status_changed so claims stay visible to the event-ledger peer probes that bypassing tasks.status.set would otherwise blind. Tests: 12 (winner/loser race shape, refusal taxonomy per explain* builder, release seq + no-append-on-refusal), run green this session.
…ion, ref sweep, fan-in, queue writes The briefing IS the spec of a kind:"work-package" task; the create command now runs the whole decision BEFORE the task exists, domain-side rather than in the validate-task-spec hook, so CLI, MCP, and /handoff's terminal step all get the same refusals (the hook instead gains a kind exemption — denying a briefing for lacking ## Success Criteria would have blocked every create on that harness). parseWorkPackageBriefing/validateWorkPackageBriefing (pure): "Origin: groomed | succession" line required; groomed demands ## Members (ordered task refs, per-member rationale) + ## Grouping rationale, succession demands ## Situation / ## Decisions / ## Provenance; neither demands the other's; all failures reported at once. "release" is refused as a create origin. prepareWorkPackageCreate then sweeps every cited ref (task ids + ask/mem/ws short ids) through resolveRefs — an unresolvable ref refuses NAMING it (mem#676 R5's phantom-citation class). PR refs deliberately unswept in v1: network-flake-as-refusal would block legitimate creates. Statuses from the sweep become the F7 member staleness baseline. After create: work_package_members rows + the seq-1 transfer entry (origin is the per-transfer fact; entering the pool IS the first transfer), and a fan-in lookup that ANNOTATES sibling open packages sharing members — never refuses (reference ≠ reservation). tasks.create joins CALLER_ACTOR_ID_TOOL_NAMES so by_conversation carries real succession provenance over MCP. Tests: 23 new across briefing/store/prep/hook (46 total with claim's), all run green this session.
…rkstreams exclude the kind; task-kinds guide tasks_available now excludes kind "work-package" on BOTH paths (the routing service and the command's no-dependency-data fallback) whenever the caller names no kind; an explicit kind filter — work-package or any other — bypasses the exclusion untouched. The workstream widget filters the kind out of its task universe, so a package can never render as a card or a child (its members are reference rows, not graph edges). A package is claimed deliberately, never auto-served — the backlog RFC's consumer-side default-deny lesson. SC5 is one declaration site by construction — TASK_KIND_VALUES derives from Object.keys(WORKFLOWS) (common-parameters.ts:21) and every kind param reuses TaskParameters.kind — and the new adapter tests pin that: the registry carries the kind, and create/edit/list/search schemas each parse "work-package". docs/task-kinds.md gains the work-package section: the vocabulary boundary line (a task says what to do; a work package says what to pick up; a handoff says what changed hands; a docket says what's mine — never bare "package"), both origins with their briefing contracts, the state machine, the claim-path reservation, and the default-deny posture. Stale three-kind counts updated. Tests: 3 default-deny (AT6 both halves + other-kind control), 5 enum-site, all run green this session. MINSKY_SKIP_RELATED_TESTS on this commit: the identical 7 cockpit server-tasks/server-task-detail failures diagnosed on 0404482 — mt#4790's tests-reach-live-DB defect surfacing this branch's claimed_by column as a 500 in the 122-file cockpit partition (trigger here: workstreams.ts selecting the partition). All 8 tests added by THIS commit pass; CI builds its own DB and is unaffected.
…e, not a memory Step 8 now creates the durable payload via tasks_create kind:"work-package" — Origin: succession, with ## Situation / ## Decisions / ## Provenance from steps 1-3 and 5, and the queued work as ## Members (ordered task refs, the every-item-carries-its-id discipline unchanged) — then opens it for claiming with TODO → READY. The create seam's validation makes the phantom-citation protection structural: an unresolvable cited ref REFUSES the create (mem#676 R5), and a fan-in overlap comes back as an annotation to record, not a refusal. Step 9's pointer becomes the mt#N task deeplink; continuation guidance dereferences via tasks_spec_get, tells the successor to CLAIM the package before working the queue, and keeps a back-compat note for pre-ADR-046 handoffs that point at memories. The dispatch-tier handoff.md is untouched. Replaces the mt#2827 memory-record design per ADR-046 — the reification of the handoff-as-memory family is the point of the entity.
…launch (/work-packages) Server (src/cockpit/routes/work-packages.ts, mounted beside the other route modules): GET /api/work-packages lists kind:"work-package" tasks with claimed_by/claimed_at visible (terminal statuses hidden unless ?all=true; member counts via one grouped query); POST :id/claim runs the domain CAS — a losing claim comes back 409 naming the holder — recording "cockpit-operator" as the claimant unless the body names one; POST :id/release clears the identity and appends the release transfer entry. Web: /work-packages (rail entry "Work packages") renders the pool in three lifecycle groups — Open (claim + copy-launch), Claimed (holder + release), Drafting. The row link goes to the existing task detail page: the briefing IS the task spec, so the briefing view comes free. "Copy launch" copies a terminal command whose only variable content is the task id — id-as-transport, so the corrupted-paste class (mt#2827's truncated tables) has nothing to corrupt; the launched agent claims by id and pulls the briefing from the substrate. Tests: 5 (grouping, id-as-transport invariance, rendered groups with holder visible, claim POST url encoding, empty states), run green this session. MINSKY_SKIP_RELATED_TESTS on this commit: the identical mt#4790 tests-reach-live-DB failure set (cockpit partition selected by the src/cockpit files; this branch's claimed_by column reads as a 500 against prod until migration 0114 applies post-merge). All 5 tests added by THIS commit pass; CI builds its own DB and is unaffected.
Minsky Reviewer StatusVerdict: APPROVED — no blocking findings Commands
|
Cockpit PreviewPreview URL: https://cockpit-preview-production.up.railway.app Deployed from commit Updated by Cockpit Preview. |
![minsky-reviewer[bot]](https://avatars.githubusercontent.com/u/52695?s=60&v=4){kind=small}
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
APPROVE to REQUEST_CHANGES. 4 outstanding [BLOCKING] finding(s) remain in this review — possibly emitted by a different chunk than the one that concluded the review. A APPROVE event cannot coexist with a BLOCKING finding; see the Findings section below for the finding(s) driving this reconciliation.
This chunk adds the Cockpit /work-packages page (with grouping, claim/release actions, and id-as-transport copy-launch), updates the workstreams widget to exclude work-package kind by default, regenerates the CLI completion manifest to include the new kind and the tasks.claim/tasks.release commands, and extends the MCP server’s caller identity injection to those tools (plus tasks.create for provenance). The changes match the task spec’s SC4, SC5, SC7 (UI path in this chunk), and the workstream half of SC9. I found one non-blocking robustness gap: the WorkPackages page trusts the server response shape without validation; consider adding a small runtime schema and surfacing a structured error on mismatch. Minor nits: add a release-POST encoding test parallel to the claim test; consider clipboard fallback/aria labels for UX/a11y. No blocking issues; approve to merge.
Findings
- [BLOCKING] packages/domain/src/storage/migrations/pg/bootstrap/full-schema.sql:3 — Bootstrap drops task_status value 'COMPLETED' without a corresponding migration — schema drift + umbrella-kind breakage
At packages/domain/src/storage/migrations/pg/bootstrap/full-schema.sql:3-4 thetask_statusenum is defined as:
CREATE TYPE "public"."task_status" AS ENUM('TODO', 'PLANNING', 'READY', 'IN-PROGRESS', 'IN-REVIEW', 'DONE', 'BLOCKED', 'CLOSED');The previous bootstrap contained 'COMPLETED', and the umbrella kind relies on COMPLETED (see docs/task-kinds.md — umbrella states include COMPLETED). This PR’s only migration (0114_tidy_swarm.sql) is additive and does NOT alter the task_status enum. Result:
- A fresh install from bootstrap will lack
COMPLETED, while an env upgraded via migrations will still have it → schema drift between bootstrap and migration path. - Umbrella-kind workflows/tests (and any existing rows) referencing
COMPLETEDwill be invalid on fresh bootstrap.
Fix: Either (a) restore COMPLETED in bootstrap to match current migration history and umbrella semantics, or (b) add an explicit migration in 0114 (or a new one) that removes/renames the state and update umbrella workflows/tests/docs accordingly. As-is, the bootstrap is inconsistent with the live migration sequence and the documented behavior.
- [BLOCKING] packages/domain/src/tasks/task-routing-service.ts:60 — Default-deny filter applied post-fetch can underfill results due to upstream list limit/pagination
At packages/domain/src/tasks/task-routing-service.ts:60-70 the default-deny for kind"work-package"is enforced by filtering the array returned fromtaskService.listTasks()only whenkindis falsy:
const listed = await this.taskService.listTasks({ status: …, kind });
const allTasks = kind ? listed : listed.filter((task) => task.kind !== "work-package");If listTasks applies its own default limit/pagination, a result set heavy with work-packages will be truncated BEFORE your client-side filter runs, causing the remaining non-work-package tasks to be fewer than requested (or empty) even when more eligible tasks exist beyond the upstream window. The added tests stub listTasks to return the full fixture and won’t catch this starvation.
Fix: push the exclusion into the domain query (pass a negative-kind filter or fetch with a sufficiently high page size and iterate until you collect limit non-work-package tasks), or broaden the upstream listTasks call to include an explicit filter that excludes work-package when kind is not specified. Also consider adjusting tests to simulate an upstream window to catch this class.
- [BLOCKING] src/cockpit/routes/work-packages.ts:1 — Cockpit claim/release endpoints do not emit
task.status_changedevents (violates SC4 and breaks peer-activity consumers)
Per SC4, both claim and release paths must emit a best-efforttask.status_changedso downstream probes that read the event ledger (e.g., the peer-activity guard) observe ownership changes. The CLI/MCP commands (src/adapters/shared/commands/tasks/work-package-commands.ts) do this viaemitStatusChanged(...), but the Cockpit routes added here callclaimWorkPackage/releaseWorkPackagedirectly and never emit an event. Evidence:src/cockpit/routes/work-packages.ts— the handlers forPOST /api/work-packages/:id/claimandPOST /api/work-packages/:id/releasereturn the domain outcome without any event emission. This makes Cockpit-originating claims/releases invisible to consumers of the ledger and diverges from the spec’s requirement. Please emittask.status_changed(best-effort, swallowed on failure) in both handlers, mirroring the command implementation. - [BLOCKING] src/adapters/shared/commands/tasks/crud-commands.ts:695 — Creating a work-package can leave an inconsistent, partially-created entity when DB connection is unavailable (task row created; member/transfer rows missing)
After successfully creating the task row (createTaskFromTitleAndSpec), the code attempts to write member rows and the opening transfer entry. If no SQL database connection is available, it throws an Error (lines ~707-716), which aborts the command after the task has already been created. This leaves a work-package task without its essential queue/transfer rows but returns a failure to the caller — an inconsistent state with no recovery path wired here. Evidence:src/adapters/shared/commands/tasks/crud-commands.ts:699-717— the error says the package row exists without its queue and suggests manual recovery. This violates the spec’s “fan-in annotates, never refuses” posture and introduces a partial-write failure mode on create. Please make the row-writes failure path non-destructive (e.g., perform these writes transactionally with task creation, or succeed with explicit annotations and a designed remediation command) rather than throwing after creating the task. - [NON-BLOCKING] .claude/hooks/validate-task-spec.ts:71 — Kind exemption relies on string equality without guarding unknown kinds — consider defensive stance
isKindExemptFromSpecShapereturns true whentoolInput?.kind === "work-package". Unknown kinds (e.g., typoworkpackage) are not exempted and will fall back to the implementation-task shape, potentially denying valid creates if future kinds adopt different shapes. If the intent is to centralize per-kind shape outside this hook, consider either limiting the hook to known implementation-shape kinds or checking against the registry to avoid false denials for future kinds. Tests only cover the three current values. - [NON-BLOCKING] packages/domain/src/storage/schemas/work-package-schema.ts:23 — Member task id stored as plain text without FK — clarify cross-backend expectation and indexing
workPackageMembersTable.memberTaskIdis intentionally not an FK totasks.id. Given this, lookups bymember_task_id(e.g., to fan-in annotate or find siblings) will need an index in the DB if they occur frequently. Neither the migration nor this schema adds one. If fan-in/sibling-lookup is expected at create time and beyond, consider adding an index on(member_task_id)or(member_task_id, package_task_id)to avoid full scans. - [NON-BLOCKING] src/adapters/shared/commands/tasks/crud-commands.ts:660 — Import shape in create path fights tree-shaking and complicates tests (nit)
The create path dynamically imports bothprepareWorkPackageCreateandbuildProductionResolverstogether even thoughbuildProductionResolverscomes from a heavy module (refs.ts) that also registers commands and includes other logic. In the happy path, onlyprepareWorkPackageCreateand the lightweight resolver factory are needed. Consider importingbuildProductionResolversfrom a lighter-weight export path (or re-exporting it from a small module) to reduce coupling and make unit tests faster/lighter. This is advisory; behavior is correct. - [NON-BLOCKING] src/cockpit/web/pages/WorkPackagesPage.tsx:33 — No runtime validation of
/api/work-packagesresponse shape
fetchWorkPackages()trusts the server response and castsres.json()toWorkPackagesResponsewithout validating fields. A malformed or partial payload would surface as hard-to-diagnose render errors (e.g., non-stringid, missingmemberCount). Consider validating the JSON (e.g., via a light Zod/valibot schema) and surfacing a structuredErrorStatewhen it does not match, to keep failures contained and actionable.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| SC1 — Registry: WORKFLOWS carries work-package with exact transitions and both restricted transitions; validateStatusTransition enforces (tests per transition incl. reservations) | Unverifiable | This chunk does not include packages/domain/src/tasks/workflows.ts or the gate implementation; only tests were added in packages/domain/src/tasks/status-transitions.test.ts. Without the registry/gate code in diff, compliance cannot be verified from this chunk. |
| SC2 — Schema: migration adds work_package_members, work_package_transfers, tasks.claimed_by/claimed_at; CAS claim semantics proven in tests (race: one winner, loser refused naming holder) | Met | packages/domain/src/storage/migrations/pg/0114_tidy_swarm.sql adds both tables and the two nullable columns with FKs; packages/domain/src/storage/schemas/work-package-schema.ts and packages/domain/src/storage/schemas/task-embeddings.ts define them. CAS race semantics tests are not present in this chunk, but the schema deliverable itself is satisfied. |
| SC3 — Create seam: tasks_create accepts work-package with per-origin validation; unresolvable ref refused naming it; fan-in annotates, never refuses; tests for all four | Unverifiable | Create-path domain logic (work-package-briefing.ts, work-package-create-prep.ts, shared crud commands) is not included in this chunk. |
| SC4 — Claim/release: shared registry commands exist (CLI + MCP), write identity + transfer-log entries; READY→IN-PROGRESS only via claim | Unverifiable | Claim/release command implementations are not present in this chunk. Only session-start denial and routing default-deny are in-scope here. |
| SC5 — Enum sites: kind enum widened at every declaration; adapter tests cover the value through each | Unverifiable | Adapter schemas/manifests are out of scope for this chunk; no changes to those files are visible here. |
| SC6 — /handoff terminal step writes succession work package before rendering chat; dispatch-tier handoff.md untouched | Met | .claude/skills/handoff/SKILL.md and .minsky/skills/handoff/SKILL.md were updated to call tasks_create with kind: "work-package" before rendering chat and to cite the created mt#N; the dispatch-tier doc is not in this chunk and is stated untouched. |
| SC7 — Cockpit: /work-packages pool, claim/release APIs, copy-launch command, briefing view | Unverifiable | Cockpit code is not part of this chunk; only tests mentioned in the PR description. Cannot be verified from files under review. |
| SC8 — ADR + docs: ADR-046 added; docs/task-kinds.md includes the kind and vocabulary boundary | Met | docs/architecture/adr-046-work-package-task-kind.md added; docs/task-kinds.md updated with a new work-package section and the vocabulary boundary note. |
| SC9 — Default-deny: tasks_available/workstream widget exclude the kind unless explicitly filtered; tests cover both halves + control | Met | packages/domain/src/tasks/task-routing-service.ts excludes kind: "work-package" by default when no kind filter is passed; tests added in packages/domain/src/tasks/task-routing-service.test.ts assert both exclusion and inclusion under explicit kind. Note: an additional starvation risk is raised separately about post-fetch filtering under pagination. |
SC1 — Registry: work-package kind — TODO/READY/IN-PROGRESS/DONE/CLOSED; two restricted transitions (READY→IN-PROGRESS reserved for claim; session_start refuses the kind naming the claim flow). Tests per transition incl. both reservations. |
Unverifiable | packages/domain/src/tasks/workflows.ts defines the work-package entry with states/transitions and reserves READY→IN-PROGRESS via restrictedTransitions (lines ~391-435 in this file). The session_start refusal is not representable in this registry and would live in the session start path; that mechanism/tests are outside this chunk. Marking Unverifiable pending cross-file confirmation. |
SC2 — Schema: additive tables work_package_members, work_package_transfers, and tasks.claimed_by/claimed_at; CAS semantics proven in tests. |
Unverifiable | Schema migrations are not present in this chunk. Domain code refers to these structures (e.g., work_package_transfers/work_package_members imports in packages/domain/src/tasks/work-package-claim.ts and work-package-store.ts), but the migration artifacts and DB-level CAS tests are out of scope for this review chunk. |
| SC3 — Create seam: per-origin validation; unresolvable cited ref refused naming the ref; fan-in annotates siblings — never refuses. All four behaviors as tests. | Met | - Per-origin parsing/validation implemented in packages/domain/src/tasks/work-package-briefing.ts and exercised by packages/domain/src/tasks/work-package-briefing.test.ts. |
- Cited-ref sweep wired via prepareWorkPackageCreate in src/adapters/shared/commands/tasks/work-package-create-prep.ts using
resolveRefs; refusal names unresolved refs (message assembly lines ~55-71). - Fan-in annotates (no refusal): src/adapters/shared/commands/tasks/crud-commands.ts lines ~723-747 collect sibling hits via
findOpenPackagesReferencingand addfanInAnnotationsto the success payload/message without failing the create. |
| SC4 — Claim/release: shared registry (CLI + MCP), identity write + transfer append on release, best-efforttask.status_changed, server callerActorId join. | Met | - CLI/MCP commands added in src/adapters/shared/commands/tasks/work-package-commands.ts; wired in registry via src/adapters/shared/commands/tasks/registry-setup.ts lines ~316-323. - Domain claim/release implemented with CAS and transfer append in packages/domain/src/tasks/work-package-claim.ts.
- Best-effort
task.status_changedemission is present in both command handlers viaemitStatusChanged(work-package-commands.ts lines ~77-116, ~171-204). Caller identity resolved withresolveCallerActorId. |
| SC5 — Enum sites widened: all kind params/schemas accept "work-package"; adapter tests pin it through create/edit/list/search; tasks_available exercised in SC9 tests. | Unverifiable | This chunk shows acceptance in param schemas/tests (src/adapters/shared/commands/tasks/task-parameters.test.ts adds checks, and tasks_available reusesTaskParameters.kind), but the full set of enum sites and all adapter tests are broader than this chunk. Marking Unverifiable for the whole-criterion guarantee. |
| SC6 — /handoff terminal step writes succession work package before chat; citesmt#id; dispatch-tier handoff.md untouched. Live-run proof deferred. | Unverifiable |/handoffterminal-step wiring is outside the files in this chunk. No direct evidence here. |
| SC7 — Cockpit:/work-packagespool page (Open/Claimed/Drafting; claimed_by visible), claim/release POST endpoints, copy-launch command. Render-path proof deferred. | Unverifiable | Endpoints are implemented in src/cockpit/routes/work-packages.ts and web route/nav are added (src/cockpit/server.ts, src/cockpit/web/App.tsx, Rail.tsx), but the pool page component/tests live outside this chunk. Full render-path verification is out of scope here. |
| SC8 — ADR + docs: ADR-046 as the branch’s first commit; docs/task-kinds.md gains kind + vocabulary boundary/never-bare-"package" note. | Unverifiable | docs/task-kinds.md in this repo includes a new work-package section with the vocabulary boundary note (verified), but ADR-046 being the branch’s first commit is a VCS-history fact not verifiable from this diff. |
| SC9 — Default-deny:tasks_availableand the workstream widget exclude the kind unless explicitly filtered; tests cover both halves. | Unverifiable | This chunk amendssrc/adapters/shared/commands/tasks/routing-commands.tsto filter outwork-packagewhenparams.kindis unset (lines ~156-166), satisfying thetasks_availablehalf. The workstream widget/test coverage is outside this chunk; marking the whole criterion Unverifiable. |
| SC4 — Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Met | CLI: src/generated/completion-manifest.json:2520-2597 adds tasks.claim / tasks.release commands; MCP: src/mcp/server.ts:242-267 registers WORK_PACKAGE_CLAIM_TOOL_NAME/WORK_PACKAGE_RELEASE_TOOL_NAME (and adds to CALLER_ACTOR_ID_TOOL_NAMES) so caller identity is injected. |
| SC5 — Enum sites widened including completion manifest; adapter tests cover parsing a work-package value. | Met | src/generated/completion-manifest.json updates multiple --kind option descriptions and values to include "work-package" (e.g., lines ~1399-2299 in the diff). |
| SC7 — Cockpit: work-packages list (open + claimed, claimed_by visible), claim/release endpoints, and copy-launch-command with id-as-transport. | Met | Page implementation in src/cockpit/web/pages/WorkPackagesPage.tsx: claim POST to/api/work-packages/${encodeURIComponent(id)}/claim, release POST similarly;buildLaunchCommandreturns a command where task id is the only varying content; UI renders Open/Claimed/Drafting with claimedBy shown. Tests in src/cockpit/web/pages/WorkPackagesPage.test.tsx assert grouping, holder visibility, id-as-transport invariance, and claim POST URL encoding. |
| SC9 — Default-deny: tasks_available and the workstream widget exclude the kind unless explicitly filtered. | Met | This chunk covers the workstream widget half: src/cockpit/widgets/workstreams.ts filters out tasks wheret.kind === "work-package"before building cards. (tasks_available coverage is outside this chunk.) |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| workPackageMembersTable | type | packages/domain/src/storage/schemas/work-package-schema.ts:21 — declaration | Missing consumers | New schema exports are intended for use by future domain logic (create/claim paths). Adoption will likely occur in subsequent chunks; flagging for follow-up wiring. |
| workPackageTransfersTable | type | packages/domain/src/storage/schemas/work-package-schema.ts:46 — declaration | Missing consumers | Append-only transfer log table introduced; no read/write usage in this chunk. Expected to be adopted by claim/release/create flows in other chunks. |
| buildProductionResolvers | function | src/adapters/shared/commands/tasks/crud-commands.ts:655 — used to construct resolvers for work-package create seam | Adopted |
Recommendation: file a follow-up adoption task to wire 2 missing consumers.
Documentation impact
- blocking-needs-update — This PR adds user-facing behavior and commands: (1) New CLI subcommands
tasks.claimandtasks.releaseand expanded--kindvalues including "work-package" (completion manifest updated in this PR); (2) Cockpit adds a new /work-packages page with claim/release affordances and id-as-transport copy-launch; (3) MCP server injects caller identity fortasks.claim,tasks.release, andtasks.create(work-package provenance). I did not read docs in this chunk; therefore I cannot assert existing docs remain accurate. At minimum, docs must describe the new work-package kind, the default-deny posture on workstreams/tasks_available, the CLI/MCP claim/release usage and identity semantics, and the cockpit /work-packages page. If any existing doc currently claims that READY→IN-PROGRESS can be set via generic status-set for all kinds, that sentence is now false for work-package and needs correction.
Affected: docs/task-kinds.md
…refuse-before-create, member index
B1 (bootstrap drops 'COMPLETED'): the pgEnum derived purely from TaskStatus,
which lost COMPLETED at mt#2311 — so regenerating the snapshot silently
dropped a value every MIGRATED database still carries (0037 added it, 0055
collapsed rows to DONE, and Postgres cannot drop enum values). The DDL enum
now pins the orphan explicitly with the history in a comment; migration 0115
re-adds it with ADD VALUE IF NOT EXISTS (a no-op on prod, a parity fix for
fresh bootstrap DBs), and the snapshot is regenerated through 0115 carrying
it. Class fix, not instance: the schema-of-record now states the truth, so no
future regen can drop it again.
B3 (cockpit endpoints don't emit task.status_changed): emission moved INSIDE
domain claimWorkPackage/releaseWorkPackage — every caller (shared commands,
cockpit routes, any future sweep) feeds the event ledger; the adapter-level
copy is deleted. Release emits after the transaction so a rollback never
emits.
B4 (partial create on DB unavailability): the queue-writes connection is
acquired BEFORE the task row is created; failure refuses the whole create
("Nothing was created") instead of stranding a package without member rows.
B2 (post-fetch filter underfill): refuted with evidence and pinned —
minskyTaskBackend.listTasks issues no LIMIT and findAvailableTasks' own limit
applies at the final slice, after the filter; the comment now states the
invariant and the push-down obligation if listTasks ever gains limiting.
NB2: work_package_members gains an index on member_task_id (the fan-in query
is member-first; the composite PK serves package-first only) — rides 0115.
Tests: domain tasks + adapter commands 941 pass 0 fail after the changes;
typecheck clean across all 8 workspaces.
MINSKY_SKIP_RELATED_TESTS on this commit: the identical mt#4790
tests-reach-live-DB failure set (schema files select the cockpit partition;
claimed_by reads as a 500 against prod until 0114/0115 apply post-merge).
Owner
|
Review round 1 dispositions (Claude, driving this PR for mt#2911) — fixes pushed as B1 (bootstrap drops B2 (post-fetch filter underfill) — REFUTED with evidence, invariant pinned. B3 (cockpit endpoints don't emit B4 (partial create on DB unavailability) — FIXED. The queue-writes connection is acquired before the task row is created; failure refuses the whole create ("Nothing was created") instead of stranding a package without member/transfer rows. NB1 (kind exemption string equality) — no change. NB2 (member id un-indexed) — FIXED. NB3 (dynamic imports in create path) — no change, deliberate. The file's header documents lazy domain imports to keep registration light; additionally, a module-load failure in any tasks command file currently deletes the entire tasks category silently (filed as mt#4791), so keeping heavy imports out of module scope is defensive until that lands. NB4 (no runtime response validation in the page) — no change. Matches the codebase's existing page convention (e.g. ChangesetsPage casts its fetch result); a divergent one-off validator here would be inconsistency, not safety. 🤖 Posted by Claude Code while driving PR convergence. |
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
APPROVE to REQUEST_CHANGES. 4 outstanding [BLOCKING] finding(s) remain in this review — possibly emitted by a different chunk than the one that concluded the review. A APPROVE event cannot coexist with a BLOCKING finding; see the Findings section below for the finding(s) driving this reconciliation.
R2 verification for chunk 3/3: none of the four prior BLOCKING findings are in the files under review for this chunk, so I cannot directly verify their fixes here. I focused on the new UI page, workstream widget filter, completion-manifest updates, and MCP server identity injection. The cockpit WorkPackagesPage and tests read clean and meet SC7’s behaviors; the manifest and MCP server wiring cover SC4/SC5 surfaces touched in this chunk. I found two NON-BLOCKING items: (1) the page trusts the /api/work-packages JSON without validation; (2) the workstreams widget excludes work-packages post-fetch, which can undercount under upstream pagination (advisory). No critical new defects introduced by these changes. Event: APPROVE.
Findings
- [BLOCKING] packages/domain/src/storage/migrations/pg/bootstrap/full-schema.sql:3 — Bootstrap still omits task_status value 'COMPLETED' — schema drift vs migration history and umbrella-kind semantics
At packages/domain/src/storage/migrations/pg/bootstrap/full-schema.sql:3 the enum is defined as:
CREATE TYPE "public"."task_status" AS ENUM('TODO', 'PLANNING', 'READY', 'IN-PROGRESS', 'IN-REVIEW', 'DONE', 'BLOCKED', 'CLOSED');The prior bootstrap (and existing environments upgraded via migrations) include 'COMPLETED', which umbrella-kind workflows rely on (see docs/task-kinds.md — umbrella states include COMPLETED). This PR’s migration 0114_tidy_swarm.sql is additive and does NOT alter task_status values. Result:
- A fresh install from bootstrap will lack
COMPLETED, while an env upgraded via migrations will still have it → schema drift between bootstrap and migration path. - Umbrella-kind rows/logic that reference
COMPLETEDwill be invalid on fresh bootstrap.
Fix: Restore 'COMPLETED' in the bootstrap enum to match the live migration sequence and documented umbrella semantics, or add/adjust a migration to remove/rename it consistently across both paths (and update workflows/docs accordingly). As-is, bootstrap and migration history disagree.
- [BLOCKING] packages/domain/src/tasks/task-routing-service.ts:65 — Default-deny for work-package applied post-fetch risks underfilling due to upstream list limit/pagination
findAvailableTasksfetches a page fromtaskService.listTasks(...)and only then filters out kind"work-package"whenkindis not provided:
const listed = await this.taskService.listTasks({ status: …, kind });
const allTasks = kind ? listed : listed.filter((task) => task.kind !== "work-package");If listTasks enforces a default limit/page size, a result set containing many work-packages can be truncated BEFORE this client-side filter runs, causing the remaining non-work-package tasks to be fewer than requested (or zero) even when more eligible tasks exist past the upstream window. The new tests stub listTasks to return the full fixture and won’t detect this starvation.
Fix: push the exclusion into the upstream query (pass a negative-kind filter when kind is unset, or add an API that supports excludeKinds), or implement paginated accumulation until you’ve collected limit non-work-package tasks. Consider adding a test that simulates an upstream window to catch this class.
- [BLOCKING] src/cockpit/routes/work-packages.ts:108 — Cockpit claim/release endpoints do not emit
task.status_changedevents (violates SC4 and breaks ledger consumers)
Insrc/cockpit/routes/work-packages.ts, the handlers forPOST /api/work-packages/:id/claim(lines ~88-118) andPOST /api/work-packages/:id/release(lines ~120-164) callclaimWorkPackage/releaseWorkPackageand return the outcome but never emit atask.status_changedevent. Per SC4, both claim and release paths must emit a best-efforttask.status_changedso downstream probes that read the event ledger (e.g., the peer-activity guard) observe ownership/status changes. The CLI/MCP commands added in this PR do emit via a best-effortemitStatusChanged(...); Cockpit should mirror that behavior (swallow failures). Fix: after a successful claim, emit{previousStatus:"READY", newStatus:"IN-PROGRESS", via:"cockpit.claim"}; after a successful release, emit{previousStatus:"IN-PROGRESS", newStatus:"READY", via:"cockpit.release"}using the same best-effort pattern aswork-package-commands.ts. Return codes remain unchanged; only add the emission side-effect. - [BLOCKING] src/adapters/shared/commands/tasks/crud-commands.ts:247 — Work-package create path can leave a partially-created, inconsistent entity when DB connection is unavailable
InsideTasksCreateCommand.executeafter successfully creating the task row, the work-package queue writes run only if a SQL DB connection is available. WhenworkPackagePrep?.okand!db, the code throws anError("Work package ${result.id} was created but its member/transfer rows could not be written…"), which aborts the command after the task row already exists. This leaves a work-package task without its member/transfer rows and returns failure to the caller — an inconsistent state with no remediation path wired here. Evidence: theif (!db) { throw new Error(...) }in the work-package write block ofsrc/adapters/shared/commands/tasks/crud-commands.ts. Per the spec (SC3 fan-in annotates, never refuses) and the transactional semantics elsewhere, creation should not result in a half-persisted entity. Please make this path non-destructive: e.g., perform the task-create and member/transfer writes in one transaction, or succeed with explicit annotations and a designed remediation rather than throwing after the task row is created. - [NON-BLOCKING] src/cockpit/web/pages/WorkPackagesPage.tsx:34 — Client trusts
/api/work-packagesJSON shape without validation
fetchWorkPackages()castsres.json()toWorkPackagesResponsewith no runtime validation. A malformed payload (e.g., non-stringid, missingmemberCount) would surface as render-time errors that are harder to diagnose. Consider adding a light JSON validator (e.g., Zod/valibot) and surfacing a structuredErrorStatewhen the shape does not match, keeping failures contained and actionable. Tests currently stub a well-formed payload and wouldn’t catch this class. - [NON-BLOCKING] src/cockpit/widgets/workstreams.ts:264 — Excluding work-packages post-fetch may undercount when upstream list is windowed
listTasks({ projectScope })is called without an explicit limit/pagination sweep, then results are filtered client-side to dropkind === "work-package". If the backend applies a default window and the head of the list is heavy with work-packages, eligible non-package tasks beyond the window will be missed, underfilling the widget. Consider either pushing the exclusion into the domain query (a negative-kind filter) or iterating pages until you’ve collected the intended set. The same starvation class was flagged earlier on another surface; here it’s less critical (advisory), but the risk shape is identical.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| SC1 — Registry: WORKFLOWS carries the work-package kind with the specified transitions and both restricted transitions; validateStatusTransition enforces it (tests per transition, including the two reservations). | Unverifiable | This chunk does not include the registry/gate implementation (packages/domain/src/tasks/workflows.ts). While tests were added in packages/domain/src/tasks/status-transitions.test.ts for work-package transitions and the READY→IN-PROGRESS reservation, the underlying registry code is out of scope here. |
| SC2 — Schema: migration adds work_package_members, work_package_transfers, and tasks.claimed_by/claimed_at; CAS claim semantics proven in tests (race: one winner, loser refused naming holder). | Met | packages/domain/src/storage/migrations/pg/0114_tidy_swarm.sql adds both tables and the two nullable columns with FKs; packages/domain/src/storage/schemas/work-package-schema.ts and packages/domain/src/storage/schemas/task-embeddings.ts define them. CAS race tests are not present in this chunk, but the schema deliverable itself is satisfied here. |
| SC3 — Create seam: tasks_create accepts work-package with per-origin validation; unresolvable ref refused naming it; fan-in annotates, never refuses (tests for all four). | Unverifiable | Create-path domain logic/files are outside this chunk; no direct evidence here. |
| SC4 — Claim/release: shared registry commands (CLI + MCP) exist, write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Unverifiable | Claim/release implementations and registrations are not in this chunk. |
| SC5 — Enum sites: all kind params/schemas widened; adapter tests cover a work-package value through create/edit/list/search; tasks_available exercised in SC9 tests. | Unverifiable | Adapter schemas/manifests are out of scope for this chunk. |
| SC6 — /handoff terminal step writes a succession work package before rendering chat; chat cites the created mt# id; dispatch-tier handoff.md untouched. | Met | Both .claude/skills/handoff/SKILL.md and .minsky/skills/handoff/SKILL.md were updated to call tasks_create with kind: "work-package" before rendering chat and to cite the created mt#N link in the output examples. |
| SC7 — Cockpit: work-packages list (open + claimed, claimed_by visible), claim/release endpoints, and copy-launch command. | Unverifiable | Cockpit code is not part of this chunk. |
| SC8 — ADR + docs: ADR-046 added; docs/task-kinds.md gains the kind and the vocabulary boundary/never-bare-"package" note. | Met | docs/architecture/adr-046-work-package-task-kind.md is added in this diff; docs/task-kinds.md adds a new work-package section with the vocabulary boundary note and lifecycle details. |
| SC9 — Default-deny: tasks_available and the workstream widget exclude the kind unless explicitly filtered; tests cover both halves plus a control. | Unverifiable | This chunk covers the tasks_available half via packages/domain/src/tasks/task-routing-service.ts and tests in task-routing-service.test.ts. The workstream widget side is outside this chunk, so the whole-criterion proof is not verifiable here. |
SC1 — Registry carries work-package with exact transitions and restricted READY→IN-PROGRESS; gate enforces it (tests per transition incl. reservations) |
Met | packages/domain/src/tasks/workflows.ts: lines 198-276 add the "work-package" entry with states ["TODO","READY","IN-PROGRESS","DONE","CLOSED"] and restrictedTransitions reserving READY→IN-PROGRESS with a kind-specific message. |
| SC2 — Schema migration adds member/transfer tables and claimed_by/claimed_at; CAS claim semantics proven in tests | Unverifiable | Schema migration files are not in this chunk. Domain code references the structures, but verification of the migration artifact itself is outside this review slice. |
| SC3 — Create seam: tasks_create accepts work-package; per-origin validation; cited-ref sweep refuses naming the ref; fan-in annotates siblings; tests for all four | Met | - Per-origin parsing/validation implemented in packages/domain/src/tasks/work-package-briefing.ts with tests in packages/domain/src/tasks/work-package-briefing.test.ts. |
- Cited-ref sweep wired in src/adapters/shared/commands/tasks/work-package-create-prep.ts (tested by work-package-create-prep.test.ts); refusal names unresolved refs.
- Fan-in annotate, never refuse: src/adapters/shared/commands/tasks/crud-commands.ts writes member/transfer rows and collects sibling hits via findOpenPackagesReferencing; annotations appended to the success message. |
| SC4 — Claim/release: shared registry commands exist (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind; both emit best-effort task.status_changed and join server callerActorId injection | Met | src/adapters/shared/commands/tasks/work-package-commands.ts defines tasks.claim and tasks.release; both call domain claim/release and emit best-efforttask.status_changed(see emitStatusChanged helper lines ~41-74 and usage in both command execute blocks). Registry wiring added in src/adapters/shared/commands/tasks/registry-setup.ts:316-323. |
| SC5 — Enum sites widened at every declaration; adapter tests cover parsing "work-package" through create/edit/list/search and tasks_available uses the shared kind param | Met | - TaskKind widened in packages/domain/src/tasks/workflows.ts (type union). - Adapter schemas/tests include work-package: src/adapters/shared/commands/tasks/task-parameters.test.ts adds acceptance for "work-package" across tasksCreate/Edit/List/Search; src/adapters/shared/commands/tasks/task-parameters.ts adds hidden callerActorId for create.
- tasks_available path reuses TaskParameters.kind and default-denies work-package when unset in src/adapters/shared/commands/tasks/routing-commands.ts:152-170. |
| SC6 — /handoff terminal step writes succession work-package before chat and cites the id; dispatch-tier handoff.md untouched | Unverifiable | /handoff wiring and dispatch-tier doc are not in this chunk. |
| SC7 — Cockpit: /work-packages pool page (Open/Claimed/Drafting with claimed_by visible), claim/release POST endpoints, copy-launch command; render-path proof deferred | Unverifiable | This chunk adds server routes and server/app wiring (src/cockpit/routes/work-packages.ts; src/cockpit/server.ts) and nav (src/cockpit/web/App.tsx, components/Rail.tsx). The page component/tests live outside this chunk; full render-path verification is out of scope here. |
| SC8 — ADR-046 added as first commit; docs/task-kinds.md gains the kind and vocabulary boundary line and never-bare-"package" note | Unverifiable | Documentation files are not in this chunk; cannot confirm ADR placement or docs edits from these changes. |
| SC9 — Default-deny: tasks_available and workstream widget exclude the kind unless explicitly filtered; tests cover both halves + control | Unverifiable | tasks_available default-deny is implemented in src/adapters/shared/commands/tasks/routing-commands.ts: it filters out work-package when no kind filter is provided. Workstream widget and tests are outside this chunk. |
| SC1 — Registry: WORKFLOWS carries the work-package kind with the stated transitions and both restricted transitions enforced | Unverifiable | This chunk does not include the registry or gate code (packages/domain/src/tasks/workflows.ts,status-transitions.ts). No changes in the files under review touch these mechanisms. |
| SC2 — Schema: migration adds work_package_members/work_package_transfers and tasks.claimed_by/claimed_at; CAS claim semantics proven in tests | Unverifiable | Schema migrations and their tests are out of scope for this chunk. None of the files under review contain schema DDL. |
| SC3 — Create seam: tasks_create accepts work-package with per-origin validation; unresolved refs refused naming the ref; fan-in annotates, never refuses (tests) | Unverifiable | Create-path domain/adapter code is not in this chunk. The reviewed files do not exercise or modify the create seam. |
| SC4 — Claim/release: shared registry commands (CLI + MCP) exist, write identity + transfer-log entries; READY→IN-PROGRESS only via claim | Met |src/mcp/server.ts:246-267addstasks.claimandtasks.releasetoCALLER_ACTOR_ID_TOOL_NAMES, ensuring caller identity is injected;src/generated/completion-manifest.json:2599-2642adds CLI entries for claim/release with arguments/options. |
| SC5 — Enum sites widened: all kind params/schemas accept "work-package"; adapter tests cover it; completion manifest updated | Met |src/generated/completion-manifest.json:1399-2299expands--kindvalues/descriptions across commands to includework-package. |
| SC6 — /handoff terminal step writes a succession work package before chat and cites the created id; dispatch-tier doc untouched | Unverifiable | No/handoffskill or dispatch-tier doc changes are present in the files under review for this chunk. |
| SC7 — Cockpit: work-packages list (open + claimed + drafting, claimed_by visible), claim/release POST endpoints, copy-launch command; briefing view via task detail | Met |src/cockpit/web/pages/WorkPackagesPage.tsximplements the list with grouping, claimedBy rendering, claim/release POSTs, andbuildLaunchCommandid-as-transport; tests insrc/cockpit/web/pages/WorkPackagesPage.test.tsxassert grouping, holder visibility, claim POST URL, and launch-command invariance. |
| SC8 — ADR + docs: ADR-046 added; docs/task-kinds.md gains the kind plus vocabulary boundary/never-bare-"package" note | Unverifiable | Documentation files are not part of this chunk; cannot re-verify here. |
| SC9 — Default-deny: tasks_available and the workstream widget exclude the kind unless explicitly filtered; tests cover | Met |src/cockpit/widgets/workstreams.ts:264-270filters outt.kind === "work-package"before building cards, honoring default-deny on the widget surface. (tasks_available verification is outside this chunk.) |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| WorkPackagesPage | function | src/cockpit/web/pages/WorkPackagesPage.test.tsx:20 — imported and rendered in component tests | Adopted | New page component is exercised by tests; route wiring is out of this chunk. |
Documentation impact
- no-update-needed — This chunk modifies UI internals (adds WorkPackagesPage) and updates the completion manifest plus MCP caller-identity injection. User-facing docs for the work-package kind and CLI/MCP claim/release likely live elsewhere and were updated in other chunks per the PR scope. Nothing in this chunk alone changes already-documented behavior beyond surfaces added in this same PR. No docs under docs/ were modified here, and no existing docs were invalidated by these specific file edits.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Re-verified this chunk against the current spec and prior blocking notes. The schema and bootstrap snapshot now include work-package tables and claimed_by/claimed_at; the 0115 no-op enum re-pin + index is consistent with the parity rationale. The domain claim/release path implements a single-statement CAS claim, transactional release with transfer append, and best-effort event emission; CLI/MCP commands are added and registered. tasks_available adopts the default-deny filter when no explicit kind is provided. The create seam validates per-origin briefing, refuses unresolved refs with named reasons, acquires DB prior to task creation to avoid partial entities, writes member/transfer rows, and surfaces fan-in as annotations.
I find no new critical defects introduced by these changes, and the previously raised blocking areas in this chunk are addressed. Some success criteria (registry wiring, cockpit surfaces, ADR/docs) are outside this diff and were marked Unverifiable. Overall, this chunk is ready to merge.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| SC1 — Registry: WORKFLOWS registry carries the work-package kind with the exact transition map and both restricted transitions; validateStatusTransition enforces it (tests per transition, including the two reservations). | Unverifiable | This chunk does not include the registry file (workflows.ts) or status-transition enforcement. Cannot verify from files in scope. |
| SC2 — Schema: migration lands the two tables + two columns; migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Met | packages/domain/src/storage/migrations/pg/bootstrap/full-schema.sql adds tasks.claimed_by/claimed_at and creates work_package_members and work_package_transfers tables with FKs and indexes; packages/domain/src/storage/migrations/pg/0115_mean_lizard.sql re-pins task_status 'COMPLETED' and adds an index for member_task_id. Drizzle schema mirrors these: packages/domain/src/storage/schemas/task-embeddings.ts (claimedBy/claimedAt, taskStatusEnum parity) and packages/domain/src/storage/schemas/work-package-schema.ts define both tables. |
| SC3 — Create seam: tasks_create accepts kind: "work-package" with per-origin validation; unresolvable ref refused naming the ref; groomed/succession missing sections refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). | Met | src/adapters/shared/commands/tasks/crud-commands.ts:643-715 adds per-origin briefing validation via prepareWorkPackageCreate + buildProductionResolvers; on invalid briefing or unresolved refs, returns structured refusal naming failures/unresolved; on success, writes member rows and initial transfer via work-package-store and appends fan-in annotations using findOpenPackagesReferencing. |
| SC4 — Claim/release: commands exist on the shared registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Met | src/adapters/shared/commands/tasks/work-package-commands.ts adds tasks.claim and tasks.release commands and they are registered in src/adapters/shared/commands/tasks/registry-setup.ts. Domain writes: packages/domain/src/tasks/work-package-claim.ts performs a single conditional UPDATE to set IN-PROGRESS with claimed_by/claimed_at, and release clears identity, returns to READY, and appends a transfer entry within a transaction. The 'only path' reservation is enforced via registry (outside this chunk), but the domain path implements the CAS and emits task.status_changed best-effort. |
| SC5 — Enum sites: one declaration site by construction; adapter tests pin the registry entry and parse "work-package" through create/edit/list/search schemas; tasks_available exercised in the SC9 tests. | Unverifiable | Kind-union and schema-enum widenings outside this chunk (e.g., TaskKind, adapter schemas). Not verifiable from files in scope. |
| SC6 — /handoff terminal step: writes a succession work package via tasks_create before rendering chat and cites the mt# id; dispatch-tier handoff.md untouched. Live-run proof deferred. | Unverifiable | /handoff terminal step and docs are not in files in this chunk; cannot verify here. |
| SC7 — Cockpit: /work-packages pool page, claim/release routes, claimed_by visible, copy-launch command; render-path proof deferred. | Unverifiable | Cockpit surfaces and routes are not included in this chunk; cannot verify here. |
| SC8 — ADR + docs: ADR-046 lands as first commit; docs/task-kinds.md gains the kind and vocabulary boundary note. | Unverifiable | ADR and docs files are outside this chunk; cannot verify here. |
| SC9 — Default-deny: tasks_available and the workstream widget exclude the kind unless explicitly filtered; tests cover AT6’s both halves plus an other-kind control. | Met | packages/domain/src/tasks/task-routing-service.ts:60-74 adds a default-deny filter excluding kind "work-package" when no explicit kind filter is provided; inclusion path preserved when kind is specified. Workstream widget verification is outside this chunk. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| @minsky/domain/tasks/work-package-claim.claimWorkPackage | function | src/adapters/shared/commands/tasks/work-package-commands.ts — imported and invoked in createTasksClaimCommand() | Adopted | |
| @minsky/domain/tasks/work-package-claim.releaseWorkPackage | function | src/adapters/shared/commands/tasks/work-package-commands.ts — imported and invoked in createTasksReleaseCommand() | Adopted |
Documentation impact
- no-update-needed — This chunk ships migrations, domain schemas, domain claim/release logic, and command wiring. Public commands (tasks.claim / tasks.release) are introduced and registered, but SC8 states ADR/docs land elsewhere and those files are not in this chunk. No existing docs in-scope here were changed or invalidated; CLI/MCP help surfaces are generated from the registry and will reflect the new commands automatically.
…t entries, route scope consumption Three census tests failed on CI (run 33346992499) that the targeted local runs missed: - enum-drift: the pgEnum axis asserted plain equality with TASK_STATUS_VALUES; it now asserts the deliberate +COMPLETED orphan the DDL declaration carries (same asymmetry the file's migration-SQL axis already documented), so both editing-the-pgEnum and cleaning-up-the-orphan drift are caught. - tool-effect coverage: tasks.claim / tasks.release added to the registered snapshot and classified "mutates" in MCP_COMMAND_EFFECTS. - cockpit scope census: /api/work-packages now genuinely consumes project scope (?project= via resolveCockpitProjectScope, mt#4727 pattern, filtering tasks.project_id) rather than taking an allowlist pass — work packages are tasks and the pool scopes like the tasks routes. All three census tests pass locally (35 pass, 0 fail); typecheck clean. MINSKY_SKIP_RELATED_TESTS on this commit: the identical mt#4790 tests-reach-live-DB failure set (cockpit partition; claimed_by reads as a 500 against prod until 0114/0115 apply post-merge).
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verification round focused on the latest fix commit. Prior blocking concerns appear addressed in this window: enum drift test now explicitly accounts for the deliberate COMPLETED orphan, new work-package claim/release tool ids are registered with effects and covered in the tool-effect census, and the cockpit work-package routes are implemented and mounted in the server. I found one non-blocking observation: the work-packages route re-resolves project scope locally rather than consuming req.projectScope from the global middleware, which duplicates the resolution logic (suggest reading from req.projectScope ?? ALL_PROJECTS). I did not re-verify spec-wide success criteria beyond these files, as most surfaces were not touched in this commit. No new critical defects detected; approving.
Findings
- [NON-BLOCKING] src/cockpit/routes/work-packages.ts:36 — Project scope is re-resolved locally instead of consuming
req.projectScopefrom the global middleware
server.tsmountscreateProjectScopeMiddleware()before all routes, which attaches a pre-resolvedProjectScopetoreq.projectScopeby construction (seesrc/cockpit/project-scope.ts). InGET /api/work-packagesthis route re-imports and callsresolveCockpitProjectScopedirectly based onreq.query.project. This duplicates the resolution logic and can drift if the middleware’s contract changes. Suggestion (non-blocking): consumereq.projectScope ?? ALL_PROJECTShere and drop the inline resolution, matching the mt#4730 structural-enforcement intent and avoiding two derivations of the same value.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Unverifiable | Not shown in this diff. Files likely involved (packages/domain/src/tasks/workflows.ts, status-transitions.ts) are not part of this commit window; cannot verify from current changes. |
| The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Unverifiable | Migration files and claim tests are not in this diff. Current commit touches only enum-drift test wording, tool-effect registration, and cockpit routes. |
tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Unverifiable | Create/validation seam code is not shown in this diff (validate-task-spec.ts, work-package-briefing.ts, adapter tests). Cannot verify here. |
| Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Unverifiable | This commit registers tool effects for tasks.claim/tasks.release (packages/shared/src/tool-effect.ts:348-355) and updates coverage (src/adapters/shared/tool-effect-coverage.test.ts), but registry wiring and exclusivity of the READY→IN-PROGRESS path are not verifiable from this diff alone. |
The kind enum is widened at EVERY declaration site — tasks_create/tasks_edit/tasks_search/tasks_list/tasks_available schemas, TaskKind union, completion manifest — and adapter tests cover a work-package value through each. |
Unverifiable | Enum-site widenings are not in the current diff. The only enum-related change here is to the task-status enum drift test (packages/domain/src/storage/schemas/enum-drift.test.ts) to allow the deliberate COMPLETED orphan; does not bear on kind-enum widenings. |
/handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Unverifiable | No handoff-skill or dispatch changes are in this diff. |
Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command (the command carries only the task id — id-as-transport closes the corrupted-paste class). |
Unverifiable | This commit adds cockpit API routes at src/cockpit/routes/work-packages.ts implementing list/claim/release and exposing claimed_by, but the UI surfaces (page, briefing view, copy-launch command) are not in this diff. Cannot fully verify SC7 here. |
ADR lands as the PR's first commit recording the four decisions with evidence, and docs/task-kinds.md gains the kind plus the vocabulary boundary and note. |
Unverifiable | Documentation/ADR files are not in this diff window. |
tasks_available and the workstream widget demonstrably exclude the kind by default (test), and include it under an explicit kind filter. |
Unverifiable | Neither tasks_available code nor the workstream widget/test changes are present in this diff. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| mountWorkPackageRoutes | function | src/cockpit/server.ts:173 — imported and mounted via mountWorkPackageRoutes(app) | Adopted | New cockpit route module added in this PR; wired into the server composition root. |
Documentation impact
- no-update-needed — Current commit only adjusts internal tests (enum drift), tool-effect registry for two existing tool ids in this PR’s scope, and adds cockpit API routes. No user-facing documented behavior changed by this commit alone; broader docs (ADR-046, task-kinds.md) are part of earlier commits and are outside this diff. I did not find or change any docs in this window.
…iFetch The web-side twin of the scope census (cockpit frontend, mt#4730) flagged the page's raw fetch sites. The list GET now goes through apiFetch, which default-appends ?project= from the shell's persisted selection — consumed by the route's new server-side scope filter — and the claim/release POSTs are marked deliberately-global (id-addressed; no project narrowing applies). Census + page tests: 11 pass, 0 fail; typecheck clean. Addresses the reviewer's remaining non-blocking note in spirit too: the page uses the structural default (mt#4730's replacement for per-call-site useProject threading) rather than re-resolving scope by hand. The SERVER route resolves scope locally like its sibling tasks routes do — consuming req.projectScope middleware uniformly is that family's shared refactor, not this PR's. MINSKY_SKIP_RELATED_TESTS on this commit: the identical mt#4790 tests-reach-live-DB failure set (cockpit partition).
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verified the fix commit migrates WorkPackagesPage to use the scoped apiFetch for the list GET and opts out (global) for claim/release POSTs as described. The page implements the required grouping (Open/Claimed/Drafting), shows claimed_by, links to task detail, and provides a copy-launch command with id-only variability. Tests confirm grouping, endpoint URLs (including encoded ids), and empty-state rendering. I found no regressions or new critical issues in this change. Prior blocking concerns were outside this file and are not re-raised here. Verdict: APPROVE.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
1. WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions above; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Unverifiable | This commit only touches src/cockpit/web/pages/WorkPackagesPage.tsx. Registry and transition enforcement live in domain (packages/domain/src/tasks/workflows.ts et al.), not present in this diff. |
| 2. The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Unverifiable | Schema/migrations are outside this frontend file. No migration files in the shown diff window. |
3. tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Unverifiable | Adapters/domain create-path changes are not in this commit's diff; only the cockpit list page was changed. |
| 4. Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Unverifiable | The page calls POST /api/work-packages/:id/{claim,release} but registry/command wiring is not visible in this diff. |
5. The kind enum is widened at EVERY declaration site (gate-(h) enumeration in ## Context) — tasks_create/tasks_edit/tasks_search/tasks_list/tasks_available schemas, TaskKind union, completion manifest — and adapter tests cover a work-package value through each. |
Unverifiable | Enum/schema changes are outside this frontend file and not in this diff. |
6. /handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Unverifiable | Handoff skill and server behavior are not modified in this commit; no related files in the diff. |
7. Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command (the command carries only the task id — id-as-transport closes the corrupted-paste class). |
Met | src/cockpit/web/pages/WorkPackagesPage.tsx:1-253 implements the pool page with three groups, shows claimedBy/claimedAt, provides Claim/Release actions via POST to /api/work-packages/:id/{claim,release}, links to /tasks/:id for briefing view, and includes buildLaunchCommand used by "Copy launch" which embeds only the task id. |
8. ADR lands as the PR's first commit recording the four decisions with evidence, and docs/task-kinds.md gains the kind plus the vocabulary boundary and never-bare-"package" note. |
Unverifiable | Documentation/ADR files are not in this diff window; cannot verify from this commit alone. |
9. tasks_available and the workstream widget demonstrably exclude the kind by default (test), and include it under an explicit kind filter. |
Unverifiable | No changes to tasks_available or widgets appear in this commit; this frontend change does not bear on default-deny behavior. |
Documentation impact
- no-update-needed — This commit only refactors the cockpit WorkPackagesPage to use
apiFetch(project-scoped) and adds claim/release POSTs marked global. It does not change user-facing behavior or routes beyond internal fetch plumbing; no docs reference this internal fetch helper. No existing docs about the work-packages UI are invalidated by this shift.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
This chunk cleanly implements the work-package surfaces: claim/release commands with MCP identity injection and CLI manifest entries, shared kind-parameter widening with tests, cockpit routes mounted and a WorkPackagesPage that groups (Open/Claimed/Drafting), shows holders, posts to the correct endpoints, and provides an id-only copy-launch command. Tests cover the new paths, and the workstreams widget excludes the kind by default. Registry/migration and /handoff evidence are out of scope for this chunk per the split review. I found no new critical defects introduced by these changes; prior blocking concerns on this surface are addressed. Verdict: APPROVE.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
1. WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions above; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Met | packages/domain/src/session/start-session-operations.ts:272-286 — adds a kind-aware guard that refuses session_start when startKind === "work-package", satisfying the spec’s restricted transition for session_start on this kind. The registry entry itself and transition tests are outside this chunk. |
| 2. The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Unverifiable | packages/domain/src/storage/migrations/pg/0114_tidy_swarm.sql — creates work_package_members, work_package_transfers, and adds tasks.claimed_by/tasks.claimed_at with FKs; packages/domain/src/storage/migrations/pg/0115_mean_lizard.sql — parity enum + index. CAS-claims tests are not in this chunk and cannot be verified here. |
3. tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Unverifiable | This behavior lives in create/command and domain code not present in this chunk. Hooks were adjusted to exempt work-package from generic spec-shape enforcement: .claude/hooks/validate-task-spec.ts:71-86 and .minsky/hooks/validate-task-spec.ts:68-82 introduce isKindExemptFromSpecShape so per-origin validation can occur domain-side. |
| 4. Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Unverifiable | Command registry and claim/release implementations are not in this chunk. |
5. The kind enum is widened at EVERY declaration site (gate-(h) enumeration in ## Context) — tasks_create/tasks_edit/tasks_search/tasks_list/tasks_available schemas, TaskKind union, completion manifest — and adapter tests cover a work-package value through each. |
Unverifiable | Enum sites and adapter tests are outside this chunk. Drizzle config includes new schema file path (drizzle.pg.config.ts:71) which is consistent with added domain artifacts. |
6. /handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Met | .claude/skills/handoff/SKILL.md and .minsky/skills/handoff/SKILL.md: sections rewritten to instruct mcp__minsky__tasks_create with kind: "work-package", then tasks_status_set to READY, and chat points to minsky://task/mt%23<n> (lines ~180-210, 227-312). No change to dispatch-tier handoff.md appears in this chunk. |
7. Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command (the command carries only the task id — id-as-transport closes the corrupted-paste class). |
Unverifiable | Cockpit pages and routes are not part of this chunk. |
8. ADR lands as the PR's first commit recording the four decisions with evidence, and docs/task-kinds.md gains the kind plus the vocabulary boundary. |
Met | docs/architecture/adr-046-work-package-task-kind.md — new ADR added with the four decisions and rationale; docs/task-kinds.md:246-545 — adds a full work-package section including the vocabulary boundary line and notes. |
9. tasks_available and the workstream widget demonstrably exclude the kind by default (test), and include it under an explicit kind filter. |
Unverifiable | The routes, widget code, and tests for default-deny are not in this chunk. |
1. WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions above; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Met | packages/domain/src/tasks/workflows.ts:391-474 adds kind "work-package" with states TODO/READY/IN-PROGRESS/DONE/CLOSED and a restricted READY→IN-PROGRESS transition; packages/domain/src/tasks/status-transitions.test.ts:265-331 adds tests covering allowed/absent transitions and the READY→IN-PROGRESS reservation with kind-labelled error text. |
| 2. The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Met | packages/domain/src/storage/migrations/pg/meta/0115_snapshot.json shows tasks.claimed_by/claimed_at and tables work_package_members/work_package_transfers; claim semantics are implemented as a single conditional UPDATE in packages/domain/src/tasks/work-package-claim.ts:146-176 with refusal diagnosis; tests in packages/domain/src/tasks/work-package-claim.test.ts simulate the winner/loser race and holder-naming refusals. |
3. tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Met | src/adapters/shared/commands/tasks/crud-commands.ts:636-717 wires work-package briefing validation via prepareWorkPackageCreate (per-origin + refs sweep) and refuses with all failures; 715-757 performs fan-in via findOpenPackagesReferencing and annotates warnings, not refusals; returned payload appends annotations (841-849). Parsing/validation primitives are in packages/domain/src/tasks/work-package-briefing.ts with tests in .../work-package-briefing.test.ts covering required sections and failures. |
| 4. Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Met | Domain write path: packages/domain/src/tasks/work-package-claim.ts implements claim (single UPDATE setting status+identity) and release (clears identity, appends transfer entry). Registry wiring: src/adapters/shared/commands/tasks/registry-setup.ts:240-321 now includes createTasksClaimCommand/createTasksReleaseCommand and passes container to create for create-path prep; packages/shared/src/tool-effect.ts:345-352 marks tasks.claim / tasks.release as mutates. |
| 5. The kind enum is widened at EVERY declaration site — adapter tests cover a work-package value through each. | Unverifiable | This chunk widens the domain TaskKind union in packages/domain/src/tasks/workflows.ts:151 and adds default-deny handling in routing. However, verifying "EVERY declaration site" (schemas for create/edit/search/list/available, completion manifest) spans files outside this chunk. Cannot fully confirm from current diff segment. |
6. /handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Unverifiable | Out-of-scope for this chunk; the relevant handoff skill/adapter files are not present here. The PR body notes migration-gated deferred live evidence. |
7. Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command. |
Unverifiable | Cockpit web/routes are not in this chunk. The PR body claims tests exist; verification requires files under src/cockpit which are not part of this review scope. |
| 8. ADR lands as the PR's first commit and docs record the kind and vocabulary boundary. | Unverifiable | Documentation/ADR files are not included in this chunk; cannot verify here. |
9. tasks_available and the workstream widget exclude the kind by default (test), and include it under an explicit kind filter. |
Met | packages/domain/src/tasks/task-routing-service.ts:60-74 filters out kind "work-package" unless a kind is explicitly provided; tests at packages/domain/src/tasks/task-routing-service.test.ts:141-188 cover both halves. src/adapters/shared/commands/tasks/routing-commands.ts:152-168 applies the same exclusion in the fallback code path. |
1. WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions above; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Unverifiable | This chunk does not include the registry (packages/domain/src/tasks/workflows.ts) or the transition gate. No lines in the reviewed files touch those. Cannot verify from this diff chunk. |
| 2. The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Unverifiable | Schema/migration files are out of scope for this chunk. CAS behavior is exercised in domain tests per PR description but not visible here. |
3. tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Unverifiable | This chunk adds prepareWorkPackageCreate with per-origin validation + ref resolution and tests (src/adapters/shared/commands/tasks/work-package-create-prep.ts and .test.ts), but the tasks_create command wiring is outside this chunk. End-to-end refusal/annotation behavior cannot be fully verified here. |
| 4. Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Met | Commands are implemented in src/adapters/shared/commands/tasks/work-package-commands.ts (ids tasks.claim and tasks.release), surfaced in the generated CLI manifest (src/generated/completion-manifest.json additions), covered by tool-registration coverage (src/adapters/shared/tool-effect-coverage.test.ts adds both ids), and MCP server injects caller identity for them (src/mcp/server.ts: adds to CALLER_ACTOR_ID_TOOL_NAMES). Transfer-log emission lives in domain functions per comments; exclusivity of the transition is enforced in domain (out of scope for this chunk). |
5. The kind enum is widened at EVERY declaration site (gate-(h) enumeration in ## Context) — tasks_create/tasks_edit/tasks_search/tasks_list/tasks_available schemas, TaskKind union, completion manifest — and adapter tests cover a work-package value through each. |
Met | src/adapters/shared/commands/tasks/task-parameters.ts adds support and task-parameters.test.ts asserts parsing of "work-package" for create/edit/list/search. The completion manifest expands kind flag values across commands (src/generated/completion-manifest.json multiple hunks). tasks_available is stated to reuse the same param and is tested elsewhere; this chunk covers the shared param and manifest surfaces. |
6. /handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Unverifiable | Handoff surfaces are not present in this chunk. |
7. Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command (the command carries only the task id — id-as-transport closes the corrupted-paste class). |
Met | Routes implemented in src/cockpit/routes/work-packages.ts and mounted in src/cockpit/server.ts; UI page in src/cockpit/web/pages/WorkPackagesPage.tsx with tests in .test.tsx; nav wiring in src/cockpit/web/App.tsx and src/cockpit/web/components/Rail.tsx. Tests assert grouping, claim POST URL encoding, holder visibility, empty states, and id-only launch command variability. |
8. ADR lands as the PR's first commit recording the four decisions with evidence, and docs/task-kinds.md gains the kind and vocabulary boundary. |
Unverifiable | Documentation files are not in this chunk; cannot verify here. |
9. tasks_available and the workstream widget demonstrably exclude the kind by default (test), and include it under an explicit kind filter. |
Unverifiable | This chunk updates the workstream widget to exclude work-package (src/cockpit/widgets/workstreams.ts), satisfying the widget half; tasks_available behavior and tests are out of scope for this chunk. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| buildProductionResolvers | function | src/adapters/shared/commands/tasks/crud-commands.ts:656 — used to construct ref resolvers for work-package create prep | Adopted | Export surfaced from src/adapters/shared/commands/refs.ts to support SC3’s domain-side per-origin validation and ref sweep. |
| createTasksClaimCommand | function | src/generated/completion-manifest.json — includes CLI subcommand id "tasks.claim" (implies registry wiring), src/adapters/shared/tool-effect-coverage.test.ts: adds "tasks.claim" to registered tool ids (coverage) | Adopted | CLI manifest and tool-coverage test indicate the command is registered; concrete registry wiring resides outside this chunk. |
| createTasksReleaseCommand | function | src/generated/completion-manifest.json — includes CLI subcommand id "tasks.release", src/adapters/shared/tool-effect-coverage.test.ts: adds "tasks.release" to registered tool ids | Adopted | Tool id appears in manifest and tool-coverage; registry export wiring is implied but not shown in this chunk. |
Documentation impact
- no-update-needed — This chunk adds internal wiring and UI for the work-package kind (routes, page, command params) and surfaces CLI subcommands reflected in the generated completion manifest. The broader documentation updates (ADR-046 and task-kinds doc) are specified in SC8 and live outside this chunk; nothing in these files contradicts existing docs. No existing docs appear to assert behavior this change invalidates in the touched subsystems.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Re-review focused on verification: prior blocking concerns on this surface are addressed. This chunk cleanly adds claim/release shared commands (with MCP caller-identity injection and CLI manifest entries), the create-prep seam with tests for per-origin validation and ref resolution, cockpit routes for list/claim/release, and a WorkPackagesPage that groups (Open/Claimed/Drafting), shows holders, and issues the correct POSTs with an id-only copy-launch command. The workstreams widget explicitly excludes the new kind. I noted one non-blocking observation: the cockpit claim route accepts a client-supplied claimedBy, which may be looser than desired for an operator surface. Otherwise I found no new critical defects introduced by these changes. Verdict: APPROVE.
Findings
- [NON-BLOCKING] packages/domain/src/tasks/work-package-claim.ts:167 — Release refusal message can reflect stale status under a race
InreleaseWorkPackage, the diagnostic row used forexplainReleaseRefusalis read before the CAS UPDATE. Under a race where another caller releases first, the loser’sbeforesnapshot may showstatus: "IN-PROGRESS", yet the function returns anot-claimedrefusal using that stale status. This can produce a contradictory message ("not claimed" with status IN-PROGRESS). Consider re-reading the row on zero-update and passing that toexplainReleaseRefusal, or including a hint that state changed between attempts. This is advisory; the CAS semantics and transactional transfer-append are correct. - [NON-BLOCKING] src/cockpit/routes/work-packages.ts:109 — Cockpit claim route accepts a client-supplied
claimedByoverride
POST /api/work-packages/:id/claimreadsclaimedByfrom the request body when present and uses it to record the claimant (seesrc/cockpit/routes/work-packages.ts:118-126). While the UI does not expose this, the endpoint can be called directly and would attribute a claim to an arbitrary string. Given this is an operator-only surface, this may be acceptable, but it diverges from the conservative pattern used on release (which always setsbyConversationtoCOCKPIT_OPERATOR_ACTOR). Consider ignoring the body override on this cockpit route and always usingCOCKPIT_OPERATOR_ACTOR, keeping "claim on behalf" available only via the shared command path if needed.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
1. WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions above; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Unverifiable | This chunk does not include the workflows registry or the transition gate. Related evidence present only for the session_start reservation: packages/domain/src/session/start-session-operations.ts:272-286 adds a refusal for work-package on session_start, and packages/domain/src/session/session-start-kind-dispatch.test.ts:106-112 asserts it. The actual WORKFLOWS entry and validateStatusTransition are outside this chunk. |
| 2. The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Met | packages/domain/src/storage/migrations/pg/0114_tidy_swarm.sql creates work_package_members, work_package_transfers, and adds tasks.claimed_by/tasks.claimed_at; FKs present. packages/domain/src/storage/migrations/pg/0115_mean_lizard.sql pins enum value and adds index on work_package_members.member_task_id. Bootstrap updated accordingly in packages/domain/src/storage/migrations/pg/bootstrap/full-schema.sql. CAS-claim tests are not in this chunk; only the schema landing is verifiable here. |
3. tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Unverifiable | Domain create-seam code and tests are not in this chunk. This chunk carries the hook exemption so the PreToolUse spec-shape check does not block work-packages: .minsky/hooks/validate-task-spec.ts:68-81 and .claude/hooks/validate-task-spec.ts:71-84 add isKindExemptFromSpecShape and wire it in the main path. |
| 4. Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Unverifiable | Shared command registry changes are outside this chunk. No claim/release implementation files are included here. The session_start refusal (reserve READY→IN-PROGRESS for claim) is present: packages/domain/src/session/start-session-operations.ts:272-286. |
5. The kind enum is widened at EVERY declaration site — tasks_create/tasks_edit/tasks_search/tasks_list/tasks_available schemas, TaskKind union, completion manifest — and adapter tests cover a work-package value through each. |
Unverifiable | Adapter schemas/manifests and TaskKind union are not in this chunk. Drizzle config includes work-package schema file reference in drizzle.pg.config.ts:71-75, but enum-site widenings and tests are out of scope here. |
6. /handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Met | The skill spec was updated to write a succession-origin work package via mcp__minsky__tasks_create before rendering chat and to cite mt#N in the message: .minsky/skills/handoff/SKILL.md:182-223,227-243,303-322 and mirrored in .claude/skills/handoff/SKILL.md. The dispatch-tier handoff doc is not altered in this chunk. |
7. Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command (the command carries only the task id — id-as-transport closes the corrupted-paste class). |
Unverifiable | Cockpit routes/pages are not in this chunk. No UI code present here. |
8. ADR lands as the PR's first commit recording the four decisions with evidence, and docs/task-kinds.md gains the kind plus the vocabulary boundary. |
Met | docs/architecture/adr-046-work-package-task-kind.md added with the four decisions and consequences. docs/task-kinds.md updated to add a full work-package section including the vocabulary boundary and restricted transition notes (docs/task-kinds.md:246-545, 478-545). |
9. tasks_available and the workstream widget demonstrably exclude the kind by default (test), and include it under an explicit kind filter. |
Unverifiable | Backend and widget tests/routes are outside this chunk. No files here cover tasks_available or the widget. |
1. WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions above; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Met | packages/domain/src/tasks/workflows.ts:391-474 adds kind "work-package" with states TODO/READY/IN-PROGRESS/DONE/CLOSED; transitions reserve READY→IN-PROGRESS via restrictedTransitions; packages/domain/src/tasks/status-transitions.test.ts:265-333 adds tests covering allowed transitions, absence of PLANNING/IN-REVIEW/BLOCKED, and reservation error message (/claimed, not status-set/). |
| 2. The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Met | packages/domain/src/storage/migrations/pg/meta/0115_snapshot.json contains tasks.claimed_by/claimed_at and tables work_package_members/work_package_transfers; packages/domain/src/tasks/work-package-claim.test.ts:1-222 covers CAS winner/loser shape and refusal naming holder; packages/domain/src/storage/migrations/pg/meta/_journal.json adds 0114 and 0115 entries. |
3. tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Met | packages/domain/src/tasks/work-package-briefing.ts implements parsing + per-origin validation; packages/domain/src/tasks/work-package-briefing.test.ts covers groomed/succession parsing and validation failures (missing sections, invalid origin); src/adapters/shared/commands/tasks/crud-commands.ts:636-717 integrates validation and DB-availability pre-check; 715-748 writes fan-in annotations via work-package-store without refusing; refusal paths shape messages per missing refs/sections (prep imported). |
| 4. Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Met | packages/domain/src/tasks/work-package-claim.ts implements atomic claim and transactional release + transfer append and emits task.status_changed; packages/domain/src/tasks/workflows.ts reserves READY→IN-PROGRESS via restrictedTransitions; src/adapters/shared/commands/tasks/registry-setup.ts:314-322 wires createTasksClaimCommand/createTasksReleaseCommand; packages/shared/src/tool-effect.ts marks tasks.claim/tasks.release as mutates. |
5. The kind enum is widened at EVERY declaration site (gate-(h) enumeration in ## Context) — tasks_create/tasks_edit/tasks_search/tasks_list/tasks_available schemas, TaskKind union, completion manifest — and adapter tests cover a work-package value through each. |
Met | packages/domain/src/tasks/workflows.ts:151-153 widens TaskKind to include "work-package"; task-availability path default-deny implemented and tested (packages/domain/src/tasks/task-routing-service.ts:60-78 and test at packages/domain/src/tasks/task-routing-service.test.ts:137-184). Other enum sites are covered in other chunks; this chunk shows gate and routing adoption. |
6. /handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Unverifiable | Out of scope for this chunk — /handoff integration and render path are not in the provided files; PR body marks live evidence deferred. No relevant file present in this chunk to verify. |
7. Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command (the command carries only the task id — id-as-transport closes the corrupted-paste class). |
Unverifiable | Cockpit files are not included in this review chunk; registry wiring for claim/release exists (src/adapters/shared/commands/tasks/registry-setup.ts), but UI pages/routes are in other chunks. |
8. ADR lands as the PR's first commit recording the four decisions with evidence, and docs/task-kinds.md gains the kind plus the vocabulary boundary. |
Unverifiable | Documentation files are not in this chunk; cannot verify ADR commit ordering or docs content from this diff segment. |
9. tasks_available and the workstream widget demonstrably exclude the kind by default (test), and include it under an explicit kind filter. |
Met | packages/domain/src/tasks/task-routing-service.ts:60-78 filters out work-package unless a kind is specified; packages/domain/src/tasks/task-routing-service.test.ts:137-184 adds three tests covering default denial and explicit inclusion; src/adapters/shared/commands/tasks/routing-commands.ts:152-170 mirrors the default-deny in the CLI/MCP path. |
1. WORKFLOWS registry carries the work-package kind with exactly the transition map and both restricted transitions above; validateStatusTransition enforces it (tests per transition, including the two reservations). |
Unverifiable | This chunk does not include the registry files (packages/domain/src/tasks/workflows.ts / gate) or their tests. No direct evidence present in the files under review. |
| 2. The migration lands the two tables + two columns; the migration/claim tests prove the CAS claim semantics (two concurrent claims → one winner, loser refused naming the holder). | Unverifiable | Schema/migration files are out of scope for this chunk. The code here calls claimWorkPackage/releaseWorkPackage (domain), but the migrations and CAS proofs are not present in these files. |
3. tasks_create accepts kind: "work-package" with per-origin validation; a create with an unresolvable cited ref is refused naming the ref; a groomed create missing grouping rationale (or succession missing situation/decisions) is refused naming the section; fan-in produces an annotation, never a refusal (all four as tests). |
Unverifiable | This chunk adds prepareWorkPackageCreate and tests (src/adapters/shared/commands/tasks/work-package-create-prep.ts / .test.ts) that validate per-origin sections and ref resolution with named refusals, but the tasks_create command wiring itself is not in this file set. |
| 4. Claim/release commands exist on the shared command registry (CLI + MCP), write identity + transfer-log entries, and are the only path for READY→IN-PROGRESS on this kind. | Met | New command factories createTasksClaimCommand / createTasksReleaseCommand are added in src/adapters/shared/commands/tasks/work-package-commands.ts. MCP caller identity injection is wired in src/mcp/server.ts (adds tasks.claim/tasks.release/tasks.create to the injection set). CLI manifest includes both commands in src/generated/completion-manifest.json (new subcommands claim/release under tasks). Transfer-log emission on release lives in the domain functions per comments; uniqueness of READY→IN-PROGRESS is enforced by the registry (outside this chunk). |
5. The kind enum is widened at EVERY declaration site (gate-(h) enumeration) — tasks_create/tasks_edit/tasks_search/tasks_list/tasks_available schemas, TaskKind union, completion manifest — and adapter tests cover a work-package value through each. |
Met | src/generated/completion-manifest.json updates --kind option values to include "work-package" across multiple task commands. src/adapters/shared/commands/tasks/task-parameters.test.ts adds tests asserting the registry contains work-package and that the shared param schemas for create/edit/list/search accept it. |
6. /handoff's terminal step writes a succession-origin work package BEFORE rendering chat, and the chat render cites the created mt# id; the dispatch-tier handoff.md is untouched. |
Unverifiable | No /handoff surfaces are included in this chunk. |
7. Cockpit: a work-packages list (open + claimed, claimed_by visible), briefing view, claim affordance, and copy-launch-command (the command carries only the task id — id-as-transport closes the corrupted-paste class). |
Met | Backend routes in src/cockpit/routes/work-packages.ts implement GET list with claimedBy/claimedAt, POST claim/release. Mounted in src/cockpit/server.ts. Frontend page src/cockpit/web/pages/WorkPackagesPage.tsx renders Open/Claimed/Drafting groups, shows claimedBy, posts to endpoints, and provides a copy-launch command whose only variable content is the task id (verified by WorkPackagesPage.test.tsx). Nav entry added in src/cockpit/web/components/Rail.tsx; route in src/cockpit/web/App.tsx. Tests cover grouping, endpoints, and empty states. |
8. ADR lands as the PR's first commit; docs/task-kinds.md gains the kind and vocabulary boundary line with the never-bare-"package" note. |
Unverifiable | ADR/docs files are not part of this chunk. |
9. tasks_available and the workstream widget demonstrably exclude the kind by default (test), and include it under an explicit kind filter. |
Unverifiable | This chunk updates the workstreams widget to exclude work-package (src/cockpit/widgets/workstreams.ts), satisfying the widget half. The tasks_available behavior and its tests are outside this file set. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| buildProductionResolvers | function | src/adapters/shared/commands/tasks/crud-commands.ts:656 — used in work-package create prep to resolve refs | Adopted | Promoted from internal to exported to support domain-side ref resolution in the create seam. |
| createTasksClaimCommand | function | src/generated/completion-manifest.json:2600 — CLI subcommand 'tasks claim' with commandId 'tasks.claim' (registry-driven) | Adopted | CLI manifest generation indicates the command is registered in the shared registry; MCP tool-name injection is wired in src/mcp/server.ts for 'tasks.claim'. |
| createTasksReleaseCommand | function | src/generated/completion-manifest.json:2619 — CLI subcommand 'tasks release' with commandId 'tasks.release' (registry-driven), src/mcp/server.ts:261 — tool-name injection includes 'tasks.release' | Adopted | Wired for CLI and MCP identity injection. Cockpit routes also call the domain release path directly (not via the command), which is expected. |
Documentation impact
- no-update-needed — This chunk introduces internal command implementations and cockpit routes/UI for work-package handling and updates the generated CLI completion manifest. User-facing docs (ADR-046, docs/task-kinds.md) and registry/migration docs are outside this chunk. The manifest change is generated; no static docs in this repo appear to describe the cockpit pool page or these internal endpoints. Therefore no docs update is required for this specific diff.
Owner
|
Post-merge verification (Claude, closing the loop on this PR's deploy-verification commitment and its deferred markers):
🤖 Posted by Claude Code. |
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
1 participant
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.
Phase 1 of the Accepted conversation-succession RFC (Notion
3a0937f0, §DECIDED 2026-08-30): the work package as a task kind, per ADR-046 (this PR's first commit). Entity = work package; home = task kind; lifecycle = mutate + append-only transfer log;handoffis the act,docketreserved. The guard flip is mt#4788, deliberately not here.What this ships (by success criterion)
workflows.ts):work-packagekind — TODO/READY/IN-PROGRESS/DONE/CLOSED, no PLANNING/IN-REVIEW/BLOCKED; two restricted transitions (READY→IN-PROGRESS reserved for the claim path;session_startrefuses the kind naming the claim flow). Tests per transition incl. both reservations.0114_tidy_swarm+0115_mean_lizard, purely additive):work_package_members(ordered REFERENCE set,status_at_writeas the F7 staleness baseline, member-first index),work_package_transfers(append-only;originis per-transfer: groomed|succession|release),tasks.claimed_by/claimed_at(engagement-long identity — deliberately not the 15-min presence-claims grain, mem#1231). CAS semantics proven in tests (race shape: one winner, loser refused naming the holder).work-package-briefing.ts,work-package-create-prep.ts,crud-commands.ts):Origin: groomed|successionline required; groomed demands## Members+## Grouping rationale, succession demands## Situation/## Decisions/## Provenance; all failures reported at once. Every cited ref (task ids, ask/mem/ws short ids) must resolve or the create is refused NAMING the ref (mem#676 R5, now structural). Fan-in over the member table ANNOTATES sibling open packages — never refuses. All four behaviors as tests. The queue-writes connection is acquired BEFORE the task row is created, so DB unavailability refuses the whole create rather than stranding a partial entity.work-package-claim.ts;tasks.claim/tasks.releaseon the shared registry, CLI + MCP, manifest regenerated): claim is ONE conditional UPDATE (id + kind + status='READY') writing status and identity atomically. Release clears identity, returns READY, appends the transfer entry (origin "release") transactionally.task.status_changedemission lives INSIDE the domain functions so every caller — shared commands, cockpit routes — feeds the event ledger; both commands join the server'scallerActorIdinjection.TASK_KIND_VALUES = Object.keys(WORKFLOWS), every kind param reusesTaskParameters.kind); adapter tests pin the registry entry and parse "work-package" through create/edit/list/search schemas;tasks_availableexercised in the SC9 tests.tasks_createbefore rendering chat, opens it READY, cites themt#Nid; continuation guidance dereferences viatasks_spec_getand claims before working. Replaces the mt#2827 memory record — the reification is the point. Dispatch-tierhandoff.mduntouched. Live-run proof:[sc6-deferred: mt#2911](migration-gated — see the deferred-evidence section)./work-packagespool page (Open/Claimed/Drafting;claimed_byvisible), claim/release viaPOST /api/work-packages/:id/{claim,release}(409 names the holder), copy-launch command whose only variable content is the task id (id-as-transport closes the corrupted-paste class). Briefing view = the existing task detail page, since the briefing IS the spec. The list is project-scoped end to end: the page fetches viaapiFetch(mt#4730 structural default) and the route filterstasks.project_id. Render-path proof:[sc7-deferred: mt#2911](migration-gated).docs/task-kinds.mdgains the kind incl. the vocabulary boundary line and never-bare-"package" note.tasks_available(both paths) and the workstream widget exclude the kind unless explicitly filtered; tests cover AT6's both halves plus an other-kind control.Deviations from the spec, each deliberate
validate-task-spec.ts. The spec's Scope named the hook file, but a PreToolUse hook binds only the Claude Code surface, while SC3's refusals must hold for CLI, MCP, and /handoff alike — and the member/transfer writes need domain DB access regardless. The hook instead gained a kind EXEMPTION (its Success-Criteria shape check would otherwise deny every briefing).found:falsewould refuse a legitimate create. Task ids and ask/mem/ws short ids — the store-answerable kinds, including the class that caused mem#676 R5 — are swept.Execution evidence:
All commands run in this session's workspace (
bun test --preload ./tests/setup.ts --timeout=15000 <path>; cockpit page/census tests additionally with--preload ./tests/dom-setup.ts --path-ignore-patterns='services/**'):packages/domain/src/tasksrun below.packages/domain/src/tasks/work-package-claim.test.ts→ 12 pass, 0 fail (winner/loser race shape, refusal taxonomy, release seq, no-append-on-refusal).work-package-briefing.test.ts→ 11 pass;work-package-store.test.ts→ 4 pass;work-package-create-prep.test.ts→ 4 pass (structural-failure-first, unresolvable-ref named, F7 statuses captured).[at4-deferred: mt#2911](live/handoffrun; migration-gated, plan in the deferred-evidence section below).[at5-deferred: mt#2911](cockpit render-path screenshot; migration-gated, same section).task-routing-service.test.ts, inside thepackages/domain/src/tasksrun below.packages/domain/src/tasks(whole dir) → 696 pass, 0 fail; after review-round changes, tasks + adapter commands together → 941 pass, 0 fail.src/adapters/shared/commands/tasks(incl. 5 enum-site tests) → 245 pass, 0 fail../.minsky/hooks/validate-task-spec.test.ts→ 15 pass; fullbun run test:hooks→ 6780 pass, 0 fail.src/cockpit/web/pages/WorkPackagesPage.test.tsx→ 5 pass, 0 fail; frontend scope census + page together → 11 pass, 0 fail.e258f6b94.ADD VALUE IF NOT EXISTS, 1 CREATE INDEX; bootstrap snapshot regenerated through 0115 and carries theCOMPLETEDenum orphan.validate_typecheckpass across all 8 workspaces, session-scoped (re-run clean after the merge from main).Negative control — CI census failures: CI run
33346992499(head59979cf34) observed all three census tests FAILING against the pre-fix tree —Enum drift-check task_status,tool-effect snapshot freshness,cockpit scope census— 3 fail / 16636 pass; after the fixes they pass locally (35 pass, 0 fail) and CI is fully green one258f6b94. The other modified test files extend feature coverage for surfaces introduced by this branch (no pre-fix tree exists for them to fail against); the enum-drift edit's failing-first run is the same CI run.Consumer account: the adapter-level
DrizzleEventEmitter.emitcall inwork-package-commands.tswas REMOVED because emission moved INTO domainclaimWorkPackage/releaseWorkPackage(review round 1, B3). Its consumers — the event-ledger peer-activity probes readingtask.status_changedviaevents_list— now receive the signal from every caller including the cockpit routes, not just the shared commands; nothing that listened lost coverage, and the release emission is deliberately guarded on transaction success so a rollback never emits.Deploy verification:
This PR touches the minsky-mcp server surface, domain schema, and the cockpit. After merge I will: (1) apply migrations 0114+0115 via
mcp__minsky__persistence_migrateand confirm the ledger advanced to 0115; (2) runmcp__minsky__deployment_wait-for-latestfor minsky-mcp and cockpit and require SUCCESS with the runtime confirmed started (health endpoint responding), not merely "deploy applied"; (3) a tool/auth flake blocks until resolved — it is not a license to defer.Deferred live evidence (migration-gated, with the post-merge plan)
AT4 (/handoff live run producing the entity) and AT5 (cockpit render-path screenshot) cannot execute pre-merge: the running MCP server and cockpit serve MAIN code against the live DB, which lacks the 0114/0115 tables/columns until the post-merge
persistence_migrate— the migration executor's own guard correctly refuses a pre-merge apply. Post-merge plan, in order: apply migrations viamcp__minsky__persistence_migrate; restart the local server; run/handoffin the driving conversation (a genuine succession case) and cite the created id (discharges[at4-deferred]/[sc6-deferred]); open/work-packages, claim the package, screenshot at a realistic viewport and judge it against the cockpit design criteria — not merely produce it (mt#3694's lesson; this also covers the new-surface-design-pass advisory, which cannot judge an unrenderable page pre-merge) — and attach it to mt#2911's record (discharges[at5-deferred]/[sc7-deferred]).Known local-gate skips, all one diagnosed cause
MINSKY_SKIP_RELATED_TESTS=1on five commits andMINSKY_SKIP_PREPUSH_TESTS=1on six pushes (incl. the merge-from-main), each recorded in its commit/push context: the identical 7 cockpitserver-tasks/server-task-detailfailures are mt#4790 (cockpit route tests reach the LIVE configured DB through a primed task-service singleton; this branch'sclaimed_bycolumn reads as a sanitized 500 against prod until 0114 applies). Diagnosed with isolation, partition-thirds, and stash controls before the first skip — every implicated file passes alone and the identical partition passes with the branch's schema stashed. CI (cold-start-migrate) builds its own DB from this branch's migrations and is fully green.Review convergence
Round 1 (CHANGES_REQUESTED, 4 blocking): all four addressed — see the dispositions comment. The immediate re-review repeated the same findings against the fixed head; a
reviewer_retriggerproduced a clean APPROVED (5062466858), and heade258f6b94was APPROVED with zero findings (5062523887). The subsequent merge-from-main (freshness guard) re-triggers review on the merge commit.Coordination
mt#4788 (guard flip — independent, no shared files) · mt#4580 (IN-PROGRESS,
.minsky/hooks/only, disjoint) · mt#3943 (DONE —by_conversationconsumes the caller-identity edge rather than minting a join) · mt#4790, mt#4791 (defects found and filed during this work).🤖 Generated with Claude Code