U12: resolve the move-path compatibility flag — trait hooks unconditional, legacy branch deleted (#2655)
U12's headline goal. The raw `experimentalFeatures.workflowColumns` flag gated **every task move**; its six seams are now unconditional and the flag, its last two readers, and the 124-line inline legacy branch are deleted. ## Deleted, not converted The flag-OFF branch goes with the gate. Converting a branch we intended to delete would have left a second definition of every column side effect alive to drift — the defect this program has spent its length removing. Both readers flip in **one commit** because they are not separable: the preflight in `workflow-task-create-ops.ts` computes the `movePolicyPreflight` that `moves.ts` consumes and validates. Un-gating either alone either evaluates workflow move policies — with their plugin-gate side effects — whose result is ignored, or validates against a preflight that was never computed. ## Evidence, not assertion **Equivalence (precondition 1).** `moves-flag-equivalence.test.ts` (commit 1) ran the same journey under both flag states against live PostgreSQL and diffed the persisted row: **identical across 128 fields** plus an equal timing shape, over `todo → in-progress → in-review → todo → in-progress`. Mutation-verified both ways — stamping the flag-ON branch, and diverging the reopen hook, each fail it. **And there was stronger evidence already on main that isn't mine.** U2b's `move-path-equivalence.pg.test.ts` ran *every* scenario once per path and has been green across ~10 of them: `preserveStatus`, `preservePause`, timing accounting, `preserveProgress`, `preserveWorktree`, engine-source rehome, `in-progress → todo`. Two independently built harnesses agreeing is the best evidence this question has had. **The flag was read by nothing in production.** `experimentalFeatures` is global-only and no module writes it, so this path had never run for any project without a stale persisted value. That is also why a green suite was never evidence on its own — both paths were individually valid and only one was live. ## Two claims of mine this PR corrects **1. Seam 2 does not introduce new rejections.** I said in #2639 and in the census that with the flag off there is *no* target validation, so flipping would add refusals. Reproduced the opposite: a move to an undeclared column already rejects on the legacy path with `Invalid transition: … Valid targets: …`. I found it because the discriminator I wrote to prove "the flag is the cause" failed. **2. My first equivalence test proved nothing.** It used `updateSettings`; `experimentalFeatures` is **global-only**, so `getSettingsFast()` filtered the write out and `useWorkflow` was false in *both* runs. Caught by stamping the flag-ON branch and watching the test stay green. It now writes via `updateGlobalSettings` and **asserts the flag took effect** before the journey. U2b's harness carries the same warning independently — `MUST be updateGlobalSettings, NOT updateSettings`. ## The user-visible change Move rejections now report **workflow-resolved** targets instead of the hardcoded legacy adjacency table. Concretely: `Valid targets: in-progress, triage, archived` becomes `Valid targets: archived, in-progress`. That is the fix, not a regression — the legacy table still advertised `triage`, a column the default lineage stopped declaring at #2515, so an operator following the old message was told to move somewhere impossible. Likewise a move *into* `triage` is now refused rather than stranding the card in a column with no trait flags, invisible to every trait-driven sweep until reconciliation re-homes it. `live-move-path-undeclared-target.test.ts` characterised exactly that defect and carried `it.todo("should REFUSE a move into a column the task's workflow does not declare (U2b)")` — **this fulfils it.** ## Test migration | file | change | |---|---| | `move-path-equivalence.pg.test.ts` | deleted — every scenario ran once per path; purpose fully discharged | | `workflow-capacity-invariant.pg.test.ts` | `setPath("inline"\|"hooks")` → `assertMovePathLive()`; the probe is **kept** so capacity cannot pass because moves were broken for an unrelated reason | | `store-movement.pg.test.ts` | asserts the refusal **and** that the legitimate backward move still works, so it reads as a narrowing | | `raw-workflow-columns-flag-census.test.ts` | deleted per its own instructions — it was built to fail in both directions and fired exactly as designed: `expected [] to deeply equal [3 readers]` | | `moves-workflow-flag-seams.test.ts` | deleted — it pinned the six seams this removes | ## Verification Full core suite: **33 failed / 10 files — byte-identical to main's baseline**, with **zero** files failing exclusively on this branch. I measured the baseline by checking out `origin/main` and running the same command, because the first comparison I made was by count alone and would have blamed the flip for 8 files that were already red. `pnpm lint` clean. `pnpm test:gate` green (10 / 132 / 482 / 71). `tsc -p packages/core/tsconfig.json` clean. Core builds. ## Left in place deliberately The `workflowColumns` settings key stays schema-tolerated and is already in `HIDDEN_EXPERIMENTAL_FEATURE_KEYS`, so an upgraded project carrying a stale value renders nothing and loads cleanly. Removing it from the schema would risk rejecting those projects for no benefit now that nothing reads it. --- ## Rebased onto current main — and `triage` reaches ZERO | metric | before | after | |---|---:|---:| | `moves.ts` column guards | 39 | **15** | | repo column total | 745 | **741** | | **`triage` column guards** | 1 | **ABSENT (0)** | `triage` is now absent from `byColumnId` entirely: no unconverted `triage` guard remains anywhere in production source. Combined with #2664 (the last one, in `TaskContextMenu`) this closes bar item 1. The census behaved exactly as designed on the rebase: the flip *deletes* guards, so `--strict` reported `moves.ts: allows 39, tree has 15` rather than leaving a stale allowance, and the re-record lands in this PR's diff. ## Verification on the rebased tree - Full core suite: **33 failed / 10 files — identical to main's baseline**, zero files failing exclusively on this branch (measured by checking out `origin/main` and diffing the failing-file sets, not by comparing counts). - `pnpm test:gate` green (10 / 158 / 487 / 71). - `pnpm check:lifecycle-columns` exits 0. - `pnpm lint` clean, `@fusion/core` builds. ## Two review fixes carried in this PR **P1 — optionless engine moves lost their bypass.** `resolveWorkflowBypassGuardsImpl` did `void moveSource;` — it discarded the resolved parameter and re-read `options?.moveSource`, so `moveTask(id, target)` resolved to `"engine"` at the call site and computed `bypassGuards === false`. Latent while the flag gated validation; with the gate gone, an internal executor/merger/recovery move made without an options object would be judged as a user move. **P2 — the absence signal.** Emitting `workflowId` unconditionally would have stamped `builtin:coding` onto every task with no explicit selection, reporting a fallback as authoritative. Now emits the selection directly, so absent still means "not resolved here". <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Task moves are now validated against the task’s declared workflow. - Invalid destinations are rejected with a clear error, and tasks remain in their original column. - Valid backward moves continue to work as expected. - Move behavior and lifecycle updates are now handled consistently across workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,27 +1,26 @@
|
||||
{
|
||||
"generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline",
|
||||
"totals": {
|
||||
"column": 746,
|
||||
"column": 722,
|
||||
"role": 5,
|
||||
"status": 186,
|
||||
"deliberate": 17
|
||||
},
|
||||
"byColumnId": {
|
||||
"done": 199,
|
||||
"in-progress": 144,
|
||||
"in-review": 205,
|
||||
"done": 195,
|
||||
"in-progress": 138,
|
||||
"in-review": 200,
|
||||
"archived": 147,
|
||||
"todo": 47,
|
||||
"triage": 4
|
||||
"todo": 42
|
||||
},
|
||||
"byFile": {
|
||||
"packages/engine/src/self-healing.ts": 110,
|
||||
"packages/engine/src/executor.ts": 85,
|
||||
"packages/dashboard/app/components/TaskCard.tsx": 42,
|
||||
"packages/core/src/task-store/moves.ts": 39,
|
||||
"packages/dashboard/app/components/TaskDetailModal.tsx": 30,
|
||||
"packages/engine/src/scheduler.ts": 28,
|
||||
"packages/dashboard/src/routes/register-task-workflow-routes.ts": 20,
|
||||
"packages/core/src/task-store/moves.ts": 15,
|
||||
"packages/core/src/store.ts": 12,
|
||||
"packages/engine/src/project-engine.ts": 12,
|
||||
"packages/engine/src/mission-execution-loop.ts": 10,
|
||||
|
||||
Reference in New Issue
Block a user