Commit Graph

5 Commits

Author SHA1 Message Date
gsxdsm
89aaf341d0 the unwired-seam audit: 9 defects the census cannot see, incl. a reviewed card that cannot merge (#2820)
**Nine operator-visible defects in a class the census cannot see, plus
the audit method that found them.**

The census scans for lifecycle-column **comparisons**. This PR is about
guards that have no literal to find: a helper takes an optional
*resolved* lane set, its own test passes it, the census entry is gone —
and the callers pass nothing. **A resolved seam nobody wired is
indistinguishable from no seam at all.**

## What was broken

| defect | operator sees |
| --- | --- |
| `getTaskMergeBlocker` unwired in `mergeTaskImpl` | `Cannot merge FN-1:
task is in 'checking', must be in 'in-review'` — **a reviewed card
cannot merge** |
| …and in the completion move | `Cannot move FN-1 to done: …` — **and
cannot complete** |
| `isParkedTaskColumn` unwired ×2 (`agent-heartbeat`) | a durable agent
keeps claiming a parked card; **Health Check renders it RUNNING** |
| `resolveLinkSyncColumnRoles` first-per-role | link hygiene skips a
**second hold lane** entirely |
| `executor` active-task predicate first-per-role | a card in a **second
wip lane reads as INACTIVE**; its prompt file becomes reclaimable |
| `isPlanningContinuationTaskDispatchable` partially threaded | a board
declaring `done` as *non-terminal* stalls its cards — **stalled by a
lane name** |
| `default-workflow-hooks:72`, `executor:2404` | resolved gate admits
the move, unresolved blocker refuses it |

## The recurring shape, which is sharper than "a caller forgot an
argument"

Four sites resolve the lane and then re-ask with the literal, **a few
lines apart in the same function**:

- `task-artifacts-ops` resolves `completeColumn`, then asks the blocker
with the literal.
- `default-workflow-hooks:72` gates on `lifecycleColumns?.review`, then
the literal.
- `executor:2404` compares `resolveResumeLanes(…).review`, then the
literal.
- `resolvePlanningContinuationCandidate` applies the caller's terminal
set, then delegates without it.

**Grep for the helper, not the literal.** The literal is one function
away, correctly annotated as a fallback — which is exactly why the
census is blind to all of it.

## The arity trap, named and measured (six occurrences, one caught by
review here)

`resolveLifecycleColumns` answers *"which column is **the** hold
lane?"*. A `.includes()`/`.has()` test asks *"is this **any** hold
lane?"*. Nothing distinguishes them — same types, no literal.

**A default-vs-renamed differential cannot catch it**, because the
default board declares one column per role and therefore cannot express
the failing shape. It needs a *structurally* different fixture. That is
a sharper rule than "test both vocabularies", and it would have caught
all six.

Scanned: 12 candidate sites. **4 fixed · 3 blocked (2 on the inert sync
IR reader; `triage:833` also query-shaped) · 1 needs a hook-contract
change · 3 not defects (a returned tuple; an ordering-sensitive
precedence list) · 1 false positive of my own scan.**

A sweep over all twelve would have broken the ordering-sensitive pair,
delivered nothing at the sync-blocked ones, and "fixed" a site that was
already correct.

## Two traps in fixing this class — I hit both here

1. **The legacy id is a FALLBACK, not a member.** Pre-seeding
`"in-review"` admits a board that *declares* `in-review` as its WIP
column — a card mid-implementation merges prematurely. A real resolved
answer must **replace** the default. (Caught by review; it is the same
unscoped-legacy-acceptance the glasses plugin's review caught earlier,
which I had read and reintroduced.)
2. **Two guards, one assertion.** `toContain("must be in")` passed with
`mergeTaskImpl` reverted, because the *completion* guard caught the card
instead. The assertion now names the site (`Cannot merge` vs `Cannot
move … to done`) so the two fail independently.

## Corrections I made to my own work, recorded rather than quietly fixed

- My first PG test was **vacuous three ways**:
`saveWorkflowDefinition?.()`/`setTaskWorkflowSelection?.()` do not exist
(the `?.` swallowed both, so the task kept the builtin workflow),
`updateTask({column})` does not move a card, and a two-node IR made
every setup move illegal. Premise is now **asserted**, not assumed.
- My doc claimed the audit was complete. It enumerated **helpers**, not
every **caller** — `getTaskMergeBlocker` alone has 13 call sites.
Corrected in place, with the still-unwired ones listed by file and line
and a note to distrust any "audit complete" claim including mine.
- A severity correction to another worker's E2E:
`selectActionablePlanningContinuations` has **no production caller**, so
its stated consequence is latent, not live.

## Verification

- `pnpm test:gate` — 161 + 487 + 13 + 71
- `tsc` on core and engine; `pnpm lint`; `check:changesets`; census
`--strict` — all clean, each run explicitly
- Every fix revert-measured; each has a non-vacuous companion. The
two-hold-lane and repurposed-`in-review` cases exist because the default
board cannot express those shapes.

## Deliberately not done, with reasons in
`resolved-seams-nobody-wired.md`

`isTaskReadyForMerge` (dead in production — wiring it would be the
anti-pattern itself); `getTaskHardMergeBlocker` (3 of 4 callers are
query-gated sweeps); `getInReviewStallReason` (needs a **batch
prefetch**, not a per-task resolve — its callers decorate every task on
every list read; the in-review stall badge is wrong on renamed boards
until then); `default-workflow-hooks` planning/live-work sets (needs
`DefaultWorkflowMoveContext` to carry the IR — a shared contract
change).
2026-07-30 15:08:01 -07:00
gsxdsm
2e39763930 test(engine): prove agent-link hygiene on a renamed board — a leaked agent slot, not a stale link (#2514)
Stacked on #2510. Test-only. Closes the `task-agent-sync` ledger entry.

## The defect this reproduces, in the code's own words

`task-agent-sync.ts`'s conversion note:

> a move into a renamed terminal column matched nothing and this handler
returned early — so the agent kept a `taskId` pointing at a finished
card and stayed `running`, **with no error and no failing test**.

"No error and no failing test" is the whole problem — and the cost is
not a stale link. **The scheduler counts `running` agents against its
cap**, so on a renamed board every completed task permanently consumes
an agent slot until a human notices. A board would just get slower and
slower.

## Everything in the path is real

Real PostgreSQL `TaskStore`, real `AgentStore`, the real
`attachAgentLinkSync` subscribed to the store's real `task:moved` event
(the same call `in-process-runtime` makes), and a real `moveTask` to
trigger it. Assertions read the **agent row** back out of the store —
never "the handler was called".

## Mutation-verified

Forcing the legacy literal sets (the pre-conversion behavior) fails
**exactly the two renamed cases**, leaving the default-vocabulary floor
and both negatives green. So this reproduces the original defect rather
than merely covering the file.

## Negative half

An ordinary mid-lifecycle move (`wip → review`) must **not** release the
agent — given the same time to run as the positive case. "Clear the link
whenever the card moves" would drop the binding the moment work started,
a louder failure than the leak it fixes.

## Two anti-flake, anti-vacuity details

- **Async delivery.** `task:moved` is a plain EventEmitter and the
handler is async, so the assertions **poll the persisted row** to a
bounded deadline and fail with the row's actual contents. A fixed sleep
would flake in both directions.
- **The fixture asserts itself.** The link and `running` state are
verified *before* the move, so an agent that was never linked cannot
make this pass for the wrong reason — the failure mode I hit twice
already in this program.

## Verification

- four live-E2E suites green together: **39/39**
- engine `tsc --noEmit` clean
- `pnpm test:gate` green (307 + 10 + 71)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved agent-link cleanup when tasks reach workflow completion.
* Ensured completed task links are released even when workflow columns
have been renamed.
* Preserved active agent links when tasks move through non-terminal
workflow stages.
* Improved reporting of link cleanup outcomes and handling of
synchronization errors.

* **Tests**
* Added live PostgreSQL end-to-end coverage for completion, in-progress
moves, and renamed-column scenarios.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 18:52:59 -07:00
gsxdsm
02b0f4f860 Phase B slice B2: U5 small movers — 12 literal sites converted, plus a negative result on hold-release (#2471)
Stacked on #2470 (Phase B slice B1). Base is
`feature/workflow-vocabulary-conversion` — do not merge before it.

## What this is

Phase B slice B2 — the U5 small movers. **12 literal sites converted,
plus one negative result.**

The plan estimated 36 sites. A survey found 12 genuinely-convertible
ones, and separately found that the plan's headline hold-release
scenario **was already fixed**. Both are reported below rather than
padded into a bigger-looking diff.

## The negative result (commit 1)

The plan named hold-release.ts as a target on the scenario *"release
readiness must hold and release identically for a RENAMED hold column."*
I wrote that test first, to prove it broken.

**It is not broken.** All five assertions passed against unmodified
`hold-release.ts`. U6/KTD-5 had already converted the module —
`isHeldTask`, `resolveReleaseTarget`, and `dependencySatisfied` each
resolve the task's IR. **`hold-release.ts` has no production change in
this PR.**

The tests are kept as a regression floor: the invariant rests on three
independent trait resolutions any of which could be "simplified" back to
a literal, and nothing else covered a renamed vocabulary end-to-end
through the sweep.

**I verified the tests can actually fail.** Mutating `isHeldTask` back
to `task.column === "todo"` kills all five. Without that check, a green
run against unmodified code is indistinguishable from a test asserting
something trivially true.

Two drafting notes kept in the file: the renamed ids deliberately avoid
colliding with any legacy literal, and the first draft's two dependency
tests used a `capacity` hold — which never consults dependencies at all,
so one passed **vacuously**. Both now use a `dependency` hold.

## The 12 conversions

Each was red-green: the renamed-workflow test written first and
**observed failing**, then made to pass.

| Site | Was | Now |
|---|---|---|
| `task-agent-sync` CLEAR_COLUMNS | `{done,archived,todo,triage}` |
resolved complete+archived+hold+intake |
| `task-agent-sync` isParkedTaskColumn | `{todo,triage}` |
`parkedColumns` param (hold+intake) |
| `task-agent-sync` handler branch | `to === "todo" \|\| "triage"` |
resolved parked set |
| `mesh-lease` parked guard | `task.column !== "todo"` | resolved
rebound column |
| `mesh-lease` rebound move | `moveTask(id,"todo")` | resolved rebound
column |
| `mesh-lease` audit decisionPath | `=== "todo" ? … : …` | same resolved
column |
| `mesh-lease` audit newColumn | `… : "todo"` | same resolved column |
| `merger-ai` already-finalized | `=== "done" \|\| "archived"` |
resolved complete+archived |
| `merger-ai` ×4 rebounds | `moveTask(id,"todo")` | shared
`resolveFinalizeReboundColumn` |

Rebound targets all use KTD-10 `resolveReboundTarget` (hold → intake →
first column), the helper `self-healing.ts:714` already uses — reused,
not invented.

## Three findings worth reading

**1. The mesh-lease bug was in the AUDIT, not the move.** The guard and
the audit were *independent* `=== "todo"` comparisons, so `newColumn`
asserted the card landed in `todo` regardless of what the move did. For
a workflow with no `todo` column that produced a lease-recovery trail
naming a nonexistent column — and run-audit is the only post-hoc record
of a lease recovery. Now resolved once and threaded to both, so they are
structurally incapable of disagreeing.

**2. The merger-ai failure mode was not what I predicted.** I expected
the already-finalized guard to fail open and re-merge a finished card.
The red run showed it actually throws `Cannot merge FN-1: task is in
'shipped', must be in 'in-review'` — a hard error blaming the column, on
a task whose real state is "already done". The thing preventing the
re-merge is *itself* a literal in core's `getTaskMergeBlocker`, outside
this slice. Two bugs coinciding, not a design.

**3. A fourth site had to move that wasn't on the list.**
`evaluateParkedAgentTaskLink` calls `isParkedTaskColumn` internally.
Converting only the handler would have left the preservation branch on
legacy ids after the caller resolved a renamed workflow — trading a
stale-link bug for a **worse** dropped-link bug (a live agent's link
cleared mid-run).

## Deliberately NOT converted

Both keep their literals with the reason recorded at the site under a
greppable `DELIBERATE-LITERAL` tag:

- **`hold-release.ts:326` `legacyDependencySatisfied`** — the FN-5719
dual-accept half. Converting makes both halves compute the same answer,
deleting the compatibility signal *and* its divergence detector while
looking like a cleanup.
- **`replan-target.ts` final fallback** — its value is precisely that it
is *not* trait-resolved; resolving it against the workflow is the
stranded-card bug it was written to fix.

⚠️ **The U12 literal ratchet does not exist in the tree yet.** The brief
assumed an allowlist to add entries to; there is none. `grep -rn
DELIBERATE-LITERAL packages/*/src` enumerates the sites it must admit.

## What I could NOT verify

- **One of the four merger-ai rebound sites is untested.** The
`landWorkspaceTask` rebound is verified by inspection and the shared
resolver's unit tests only — `landWorkspaceTask` is only ever *mocked*
(project-engine.test.ts), never executed. Covering it needs a multi-repo
git fixture and a full land run. The **other three are genuinely
exercised** by pre-existing merger-ai.test.ts (lines 676/716/777/895
assert `moveTask("FN-1","todo",…)` through a real git repo) and pass
unchanged — real wiring proof for those.
- **3 of the 9 task-agent-sync tests passed before the conversion too**,
vacuously — the literal handler early-returned and cleared nothing. They
assert nothing about the old code; they are guardrails against the
conversion over-clearing.
- **No renamed workflow was run against a live engine.** All evidence is
unit-level.

## Call sites outside this slice — NOT converted, byte-identical

They keep the legacy defaults: `scheduler.ts:1273`,
`agent-heartbeat.ts:1169/3642`, `self-healing.ts:11600/11665` (all
`evaluateParkedAgentTaskLink`), and `merger.ts:6585` (the sibling
terminal guard). Each is its own Phase C/D surface.

## Behavior changes (not a pure refactor)

For a **renamed** workflow: agent links now actually get cleared on
terminal moves (they never were); lease rebounds land in the resolved
hold column; finalize-blocked rebounds land in the resolved hold column
and their operator-facing task-log lines name the real column;
already-finalized cards short-circuit cleanly instead of throwing.

For **builtin:coding** and any unresolvable workflow: byte-identical.
Every new parameter defaults to the legacy set, and both merger-ai
resolvers fail *soft* to legacy ids in opposite directions — the
terminal guard keeps `done`/`archived` (losing it sends a finished card
into the merge path), the rebound keeps `todo` (abandoning it strands
the card in the merge lane with no owner).

## Verification

- Merge gate **green**: 299 + 10 + 71 tests
- Slice suites **green**: 100 tests across 8 files (new + all
pre-existing neighbours)
- Existing merger suites **green**: 82 tests across 5 files, unchanged
- `tsc --noEmit` clean, `pnpm lint` clean

No changeset: `@fusion/engine` is private.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
2026-07-27 14:28:53 -07:00
gsxdsm
fe536b2af8 FN-6954: reconcile stale parked task assignments
Reconcile agent/task drift when durable agents remain linked to queued tasks without live execution proof.

- clear stale Agent.taskId links for parked todo/triage tasks while preserving task leases and queue state
- report stale parked assignments as active/no-live-run in Reports Health Check before reconciliation completes
- add scheduler and self-healing coverage for queued lease drift, overlap starvation, and audit events
- document the reconciliation behavior and add a published package patch changeset

Files changed:
 .changeset/fn-6954-agent-task-state-drift.md       |   5 +
 docs/architecture.md                               |   2 +
 .../src/__tests__/heartbeat-executor.test.ts       |  68 ++++++++++++
 .../__tests__/scheduler-overlap-starvation.test.ts |  68 +++++++++++-
 .../self-healing-agent-link-drift.test.ts          |  97 ++++++++++++++++-
 .../engine/src/__tests__/task-agent-sync.test.ts   |  33 +++++-
 packages/engine/src/agent-heartbeat.ts             | 121 +++++++++++++++++++--
 packages/engine/src/run-audit.ts                   |   5 +
 packages/engine/src/runtimes/in-process-runtime.ts |   1 +
 packages/engine/src/scheduler.ts                   |  24 +++-
 packages/engine/src/self-healing.ts                | 102 ++++++++++++++---
 packages/engine/src/task-agent-sync.ts             |  65 ++++++++++-
 12 files changed, 555 insertions(+), 36 deletions(-)

Fusion-Task-Id: FN-6954
Fusion-Task-Lineage: 24b8a2eb-5a33-4539-ab64-ae2bbfc2d195
2026-06-23 10:30:50 -07:00
Fusion
d3fa2e1a8d feat(FN-4296): complete Step 1 — add task moved agent link sync module
Fusion-Task-Id: FN-4296
Fusion-Task-Lineage: 9d061861-547a-4905-8fdd-4b66dd7bff97
2026-05-14 05:35:34 -07:00