dd930c8d7d652e8c71154288a77af06973033f4e
12725 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
flexi767
|
dd930c8d7d |
fix(cli): qualify cross-fork PR heads (#2377)
## Summary - resolve the repository receiving pushes through `git remote get-url --push origin` - qualify pull-request head branches with the fork owner when the push owner differs from upstream - preserve the existing unqualified head for same-repository workflows ## Root cause Fusion correctly resolved the PR target from origin's fetch URL, but assumed the pushed branch lived in that same repository. With an upstream fetch URL and a fork push URL, GitHub requires `fork-owner:branch`; the unqualified branch is rejected. ## Validation - CLI task lifecycle tests: 48 passed - `@fusion/core` typecheck - `@runfusion/fusion` typecheck - strict changeset validation <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Pull requests created from branches pushed to contributor forks now correctly qualify the PR head with the fork owner when the push remote differs from the upstream owner. * Improved PR head handling across both group/shared-branch and per-task pull request creation paths. * **Tests** * Updated and expanded lifecycle tests to cover “origin push to fork” scenarios using push URL–based repo resolution. * **Documentation** * Added a patch release note for the fork-aware PR head fix. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: v <v@v.speedport.ip> Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com> |
||
|
gsxdsm
|
4f929acc10 |
fix(dashboard): stop over-aggressive component unmounts (keep-alive for planning, terminals, popups) (#2420)
Implements
docs/plans/2026-07-22-001-fix-dashboard-remount-churn-plan.md: every
confirmed source of unnecessary unmount/remount churn in the dashboard,
plus a keep-alive layer for conversation- and terminal-bearing surfaces.
## What changed
**Keying / component identity (U1–U3)**
- Streaming chat segment key no longer embeds `entries.length` — an
expanded thinking block stays expanded while entries stream into it
(R1).
- Dock task list keys `TaskCard` rows by `task.id` (occurrence suffix
only for the duplicate-id anomaly) instead of `id-index` — no remount on
reorder/filter/status change (R2).
- `ProviderStatusBadge` / `GitHubStatusBadge` hoisted out of
ModelOnboardingModal's render body (R3); MCP server rows key by
`server.name` alone (R4).
**Keep-alive layer (U4–U6)**
- New shared `KeepAliveView` wrapper: visible = in-flow flex child;
hidden = out-of-flow `position:absolute; inset:0` with
`visibility:hidden; pointer-events:none` + `aria-hidden` (never
`display:none`, so xterm geometry never collapses).
- Planning Mode renders as a kept-alive sibling of the MainContent
switch after first open (per-project latch mirroring Quick Chat). While
hidden, the session-list SSE, recovery poll, and elapsed ticker suspend
via a new `active` prop; reveal re-subscribes and refreshes the sessions
list once. Payload-carrying entry points (initial-plan handoff, resume)
and project switches remount via a new
`modalManager.planningEntryGeneration` key, preserving pre-keep-alive
fresh-open semantics. `recordResumeEvent` instrumentation records
`remount` on first activation and `route-active` on reveal.
- Task-detail Terminal / Worktree-terminal / Planner-chat tabs stay
mounted-but-hidden after first open (per-task latches; task switch/close
still disposes fully). `SessionTerminal` gains `active`: reveal refits +
forces a font remeasure, and if the WS died while hidden it re-runs the
full attach lifecycle (dead-socket recovery).
- Popped-out task windows hide via FloatingWindow `hidden` instead of
leaving the render array; `TaskDetailContent` gains `active` so hidden
popups close their SSE/EventSource channels while the terminal WS stays
open. `visiblePoppedOutTaskEntries` remains the Escape-shortcut
consumer.
**Planning Mode internal-transition audit (U7)**
- Audit findings: session-list mode and mobile list/detail flips are
CSS-class transitions over one always-mounted detail pane (no
state-discarding unmounts); re-selecting the active session is an
early-return visibility restore; session switching intentionally reloads
from the session row (stream re-attach for generating sessions);
remaining index keys are on stateless lists. No product-code defects
found; regression tests now lock the always-mounted invariant on desktop
+ mobile.
**Cheap-view state (U8)**
- CommandCenter (active sub-tab + date range) and DevServerView
(selected script/task + typed-but-unsent command) persist per project
via `modalPersistence` and restore after their (intentional) unmount
round-trips. Also fixed the candidate auto-fill effect clobbering a
customized non-empty command.
## Symptom Verification
- **Original symptom:** streaming thinking blocks collapsed mid-stream;
terminals reconnected and lost scroll/input on tab flips; Planning Mode
lost in-flight interviews on navigation; popped-out windows vanished
off-view; dock cards remounted on reorder.
- **Exact reproduction:** (1) expand a thinking block during a stream;
(2) run a command in the Terminal tab, flip to Plan and back; (3) start
a planning interview, navigate Board and back; (4) pop out a task with
board/list-only scoping and switch views; (5) change a dock task's
status.
- **Assertion it is gone:** component-identity/instrumentation tests in
TaskChatTab, SessionTerminal, TaskDetailModal
(worktree/planner-chat/tabs), PlanningModeModal keep-alive +
internal-transitions, App keep-alive round-trip, and
App.taskPopupViewGating assert no remount and preserved state for each
repro, across desktop and mobile breakpoints.
## Verification
- File-scoped vitest: 23 files / 1091 tests green (all touched suites
plus FloatingWindow, TerminalModal, TaskPlannerChatTab,
lazy-loaded-views guard, App suites).
- `pnpm verify:fast`: PASS (13 steps — scoped typecheck/build, CLI
build, boot smoke).
- `pnpm check:changesets`: passes; changeset
`fix-dashboard-remount-churn` (`@runfusion/fusion` patch, labeled
format).
- Known pre-existing failures NOT caused by this branch (verified
failing at base
|
||
|
gsxdsm
|
189f237a07 |
fix(security): make the approval and permission gates actually enforce (#2457)
## Why
An AI agent autonomously deleted a live task (FN-8609) via
`fn_task_delete`. Investigating that led to two audits, which found the
gate machinery is well-built and its *enforcement* well-tested — but the
*decision boundary* was unguarded and the tools that matter never
reached the gate at all.
The headline defect: all ~76 tools in `packages/cli/src/extension.ts`
are delivered through `hostExtensionPaths` → `DefaultResourceLoader`,
which never passes through `wrapToolsWithActionGate`. `fn_task_delete`
*was* classified as gated in `gating-classifications.ts` — that
classification was dead code. Roughly 15 destructive tools were ungated
even for an operator who had locked their policy all the way down.
Second: `POST /api/approvals/:id/decision` took the approver identity
from the request body, validated for shape only, with no check that the
decider was human or differed from the requester. Combined with a bash
tool whose command string was never inspected (only `params.cwd`), an
agent could read the daemon token from `~/.fusion/settings.json`, find
its own pending request, and approve it.
## Scope
This does **not** tighten defaults. The `unrestricted` preset is
byte-for-byte unchanged — `git diff` on `agent-permission-policy.ts` is
empty — and regression tests assert that an out-of-the-box install
behaves exactly as before. The bug was never "the default is too
permissive"; it was "strict policy doesn't enforce." This makes turning
security up actually work.
The one deliberate exception: the containment that stops an agent
escalating its *own* privileges (reading the daemon token / credentials,
calling the approvals API to self-approve) applies at every preset
including `unrestricted`. That is a privilege-escalation boundary rather
than a permission preference — if it only engaged under strict policy it
would not have prevented the incident that prompted this.
## What changed
8 bisectable commits:
- **Approval lifecycle** — self-approval blocked via server-derived
deciders; same-verdict replay 409s; decide re-reads and re-validates
inside the transaction; expiry TTLs; `markCompleted` ownership check;
session identity registry in core.
- **Engine gates enforce for real** — unclassified tools resolve to a
policy-governed category instead of hardcoded `allow`; missing-policy
fail-open closed; bash containment floor + exact-command approval
binding.
- **Dashboard decision routes** — stop trusting client-supplied actors
(decision, bypass-review, worktrunk → 403 on forged actors).
- **`fn serve` authenticated by default** — auto-mints a token following
the existing `fn dashboard` precedent; `--no-auth` opts out.
- **Sibling entry points closed** — user-sourced hard-cancel moves, ACP
execute-once approvals, plugin task-store gating.
- **pi-extension principal resolution** — the extension resolves the
acting principal and can withhold or policy-gate the previously ungated
destructive tools.
- **Root-cause bonus fix** — `findLatestByDedupeKey` was broken in
PostgreSQL backend mode (already-parsed jsonb fed through a string-only
parser), so approved-grant redemption **never matched in production**,
minting duplicate requests. This explains the live DB state of 17
approved / 0 completed. *(Also cherry-picked to `main` as `a9b30013bb`,
since it is an active production defect on its own.)*
- **Review follow-ups** (`627f1b1fa8`) — operator-configured
provisioning privilege and a configurable grant TTL; see below.
## Review follow-ups
**Provisioning privilege is operator-configured, not role-derived.**
`isCallerPrivileged` had gone from `caller.reportsTo == null` (every
top-level agent privileged — permanent escalation by creating a
manager-less agent) to `caller.role === "ceo"`, which swapped an
implicit rule for a magic string: any agent config can claim that role,
while an operator who genuinely wants a privileged agent had no
supported way to say so. Privilege now derives solely from
`agentProvisioning.trustedAgentIds` / `trustedRoles` and fails closed
when settings are unresolvable.
It is also no longer forwarded to `resolveAgentProvisioningPolicy` as
`isPrivileged`, because that flag short-circuits ahead of
`alwaysApproveDelete` — a trusted caller was bypassing delete approval
entirely. The policy applies the same trusted rules itself, in the right
order. The function now governs only the org-chart escape hatch (acting
outside your own direct reports).
**Grant TTL defaults to 1 hour and is configurable.** Approval →
redemption is not instantaneous: an operator approving from their phone,
an engine restart, a queued lane, or a task waiting on a worktree all
routinely exceeded 15 minutes, after which the grant expired and the
agent silently re-requested. One hour remains far short of the
"redeemable forever" hazard the TTL exists to bound. Override via
`FUSION_APPROVAL_GRANT_TTL_MS` or `configureApprovalRequestTtls()`;
invalid overrides are ignored rather than widening the window to
infinity or collapsing it to zero.
## Behavior changes requiring operator review before rollout
1. `fn serve` requires a bearer token by default (`--no-auth` opts out);
unauthenticated clients get 401.
2. Agents can no longer run withheld destructive tools
(`fn_task_delete`, `fn_task_bypass_review`,
mission/milestone/slice/feature/workflow deletes, `experiment_finalize`,
`skills_install`). Operators keep them via CLI/dashboard. **This is the
incident fix.**
3. Agents get provisioning privilege only when the operator lists them
in `agentProvisioning.trustedAgentIds` / `trustedRoles`; the
provisioning gate is now live in production. Previously-implicit
privilege (top-level position, or a `ceo` role) no longer grants
anything on its own.
4. Decision replay 409s (was 200); pending approvals expire after 24h,
approved grants after 1h (configurable); bash approvals bind per exact
command.
5. Forged/body actors on decision, bypass-review, worktrunk routes →
403; `archive-all-done` requires `{confirm:true}` (external scripts
affected).
6. `fn_secret_get` approvals grant exactly one reveal (previously
granted nothing and looped forever); ACP approvals are execute-once
(previously infinite reuse).
7. Bash containment denies token/credential/approvals-API commands in
all agent sessions at every preset.
## Verification
Independently re-run against the branch, not just self-reported:
- 5 typechecks (core, engine, cli, dashboard `tsconfig.json` +
`tsconfig.app.json`) — clean
- `pnpm lint` — clean
- `pnpm test:gate` — 379 passed
- `pnpm build --force` — green (a plain `pnpm build` skips packages as
unchanged and does **not** compile the branch)
- `pnpm check:changesets` — clean
- ~650 file-scoped tests including new negative-path suites for the
decision boundary, which previously had **zero** test coverage
`packages/engine/src/__tests__/plugin-runner.test.ts` fails 56/80 —
**verified pre-existing**, reproducing identically at base commit
`93a403af67` on `main`. Not in the merge gate.
### A mutation check that failed to fail
Worth recording, because it nearly shipped an untested security fix. The
first mutation check on the provisioning change reintroduced the `ceo`
hardcode and **all 17 tests still passed** — the tests asserted through
the policy path, which can no longer observe `isCallerPrivileged` at
all, precisely because `isPrivileged` is no longer forwarded there.
Org-chart cases that do exercise the function were added; the hardcode
now fails exactly 1 of 19, and restoring is green. A green mutation run
is only meaningful if the test can actually see the code under test.
## Known limitations (stated, not papered over)
- The bash containment floor is string-matching: a cost-raiser, not a
sandbox. Quoting, encoding, `$HOME`, symlinks, or an interpreter
one-liner can evade it. The durable protection is the decision route
refusing agent-originated deciders — the filter is the belt, not the
braces.
- Approval expiry is lazy (evaluated at decide/complete/redeem), not
swept, so an expired pending row stays visible in lists until touched.
- The extension's require-approval path returns a pending message but
cannot suspend a pi session mid-turn; engine-side pause hooks cover
engine lanes only.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Security**
* Hardened approval and permission gating with server-side decider
attribution, self-approval blocking, ownership checks, replay/race
protection, and status/TTL enforcement.
* Added fail-closed behavior for sensitive/unclassified tools and
sandbox provisioning approvals.
* Blocked credential/approval access via bash containment; plugin
destructive task operations now require explicit permission.
* **New Features**
* `fn serve` now defaults to bearer-token auth, with `--no-auth` as the
explicit opt-out.
* **Bug Fixes**
* Improved task move-source attribution (`moveSource: "user"`) and
tightened dashboard archive/bypass confirmation and operator attribution
behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
gsxdsm
|
7712e0ada2 |
engine: merging was broken outright on a board with a renamed review lane (#2963)
**Not a degraded message — no task on such a board could be merged at all.** `getTaskMergeBlocker`'s column-identity check *returns a blocker* when the task's column is not a review lane. Both merge entry points called it without `reviewColumns`, so the check ran against the literal `in-review`: ``` Cannot merge FN-1: task is in 'signoff', must be in 'in-review' ``` `aiMergeTask` (`merger.ts`) and `runAiMerge` (`merger-ai.ts`) turn that into a thrown error. Every merge on a renamed board fails, with a message naming a column the board does not have. ## This exact defect was already found once The helper's own FNXC comment records it, in `moves.ts`: > *"so on a renamed board that move threw `Cannot move FN-1 to done: task is in 'signoff', must be in 'in-review'` even though the transition had just been validated as legal. A half-conversion, where the outer question is resolved and the inner one is not."* That fix added the `reviewColumns` option and wired `moves.ts`. **These two callers were missed** — same shape, one layer out. A fix that adds an optional parameter is only as good as the call-site sweep that follows it. ## How it was found By enumerating the call sites of every lane-taking helper, rather than trusting the `unwired-lane-parameter` guard. That guard is deliberately conservative — a mention of the parameter *anywhere* satisfies it — so **partial** wiring is invisible to it, and `reviewColumns` is mentioned plentifully elsewhere. This is the method #2956 used on a sibling defect, applied to every seam I have touched. ## Two sites deliberately unchanged - **`moves.ts`** passes `skipColumnIdentityCheck: true`. It has already proven lane identity from resolved IR traits, so supplying lanes *as well* would be contradictory rather than additive — the helper's comment is explicit that the two options answer different questions. - **`isTaskReadyForMerge`** has **zero** production callers. Adding a parameter there is precisely the unwired-parameter anti-pattern this program keeps removing. ## Revert result | | reverted → | | --- | --- | | `reviewColumns` at either call | reproduces the shipped string exactly | The middle test pins that string deliberately: it is the operator-visible failure, so if the wiring regresses the test says what the operator would have seen. A third case checks that supplying lanes does **not** switch the identity check off — a card in the wip lane is still blocked, and the message names the resolved lanes rather than a column the board lacks. The cases drive `getTaskMergeBlocker` directly: reaching it through the merge entry points needs a real repo, worktree and merge run, while the defect is entirely in *which columns the blocker is asked about*. The wiring itself is covered by tsc and the guard. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; `merger` + `merger-ai` + `self-healing` suites 461; `tsc` engine clean; lint, census `--strict`, FNXC gate, changesets all clean. |
||
|
ischindl
|
8d6acf1314 |
fix(RUFU-018): add noCommitsExpected dep-sync skip and corepack/pnpm env passthrough (#2501)
Manually land RUFU-018 fix bypassing the AI merge pipeline. ## Summary - Add `noCommitsExpected` flag to `LandRepoContext`; skip dependency sync when set - Forward `COREPACK_HOME`/`PNPM_HOME`/`npm_config_registry` in `installWorktreeDependencies` - Add comprehensive tests for both changes This unblocks all downstream RUFU audit tasks. ## Surface Enumeration - Providers/bridges: `installWorktreeDependencies` called from `landOneRepo` (AI merge) and legacy `merger.ts`; `landOneRepo` called from `runAiMerge` and `landWorkspaceTask` - Data states: `noCommitsExpected` can be `true`, `false`, or `undefined` — both callers use `=== true` strict check <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved support for tasks that do not produce commits by skipping unnecessary dependency installation during merges. - Preserved normal merge and review behavior when dependency installation is skipped. - **Bug Fixes** - Dependency installation now correctly preserves relevant package-manager and system environment settings. - Reduced installation failures caused by missing or unavailable package-manager configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Fusion <noreply@runfusion.ai> Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com> |
||
|
gsxdsm
|
01f081e8aa |
engine: restore the stall-signal lane wiring #2951 dropped (and the test that proved it) (#2961)
**My defect, shipped in #2951 — and the same family as the one #2956 just fixed.** Found by auditing my own seams after that, not by a failing check. ## What is on `main` right now `surfaceInReviewStalls` reads the project's review columns (converted in #2951), then calls `getInReviewStallReason` **without** `reviewColumns`. The classifier falls back to the literal `in-review`, returns no signal for a renamed-lane card, and the sweep surfaces nothing. That is the textbook **missed pair** this program has a ratchet for: a widened read handing every renamed-board card to a literal classifier. The resolve work happens and is then discarded. On a renamed board an operator sees no stall warnings at all. #2951's conflict resolution dropped two things together: - the per-card `stallLanes` map and the `reviewColumns` argument - **the test that proved the wiring** ## Why nothing caught it **A deleted test cannot fail.** I verified that rebase by comparing the 68 conflict *hunks* — stripping FNXC stamps, confirming 0 of 68 had real content differences — and then ran the gate. The gate passed precisely because the proving test had gone with the code it proved. I verified the conflicts. I did not verify the outcome. Those are different things, and the difference is invisible when the evidence disappears alongside the feature. The `unwired-lane-parameter` guard cannot catch this either, by design: it is deliberately conservative — a mention of the parameter *anywhere* satisfies it — so **partial** wiring is outside its reach. `reviewColumns` is mentioned plenty in `reads.ts`, so the guard is green while this call site goes unwired. ## How I found it The check #2956 used on the sibling defect, applied to every lane seam I have touched: enumerate each function's **call sites** and confirm each one carries the parameter. That enumeration also flags several other call sites without `reviewColumns`/lane arguments (`merger.ts`, `moves.ts`, `auto-merge-finalization.ts`, `merger-ai.ts`, `project-engine.ts`) — I have **not** touched those here; they need per-site judgement about whether the lane answer is even available, and that is a separate change rather than a sweep. ## Revert result | | reverted → | | --- | --- | | `reviewColumns` at the call (i.e. exactly what #2951 shipped) | fails the restored test | ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; blindness suite 71; `self-healing.test.ts` 412; `tsc` engine clean; lint, census `--strict`, FNXC gate, changesets all clean. |
||
|
gsxdsm
|
1f0d371228 |
fix(tests): three more portal query-root failures (pr-tab, worktree-terminal, milestone-slice) (#2959)
Three dashboard test files asserted against `render()`'s `container`, but the components under test mount through `createPortal` — so `container` is **empty** and every query returns nothing. Same root cause as the earlier portal batch; these are the three that were still held back. | File | Before | After | |---|---|---| | `TaskDetailModal.pr-tab` | failing | pass | | `TaskDetailModal.worktree-terminal` | failing | pass | | `MilestoneSliceInterviewModal` | failing | pass | **Measured: 39/39 passing**, rebased on current main (`3461ae7a92`). Lint clean, FNXC date gate exit 0. ### Why this stayed hidden The queries were a **mix** of `container.querySelector(...)` and `screen.*`. `screen` queries `document`, so they kept working — a portal-mounted modal makes only the `container` half go blind. The result is a file that looks half-alive rather than obviously broken, and the failures present as five different-looking symptoms (`null`, `undefined`, `+0`, `[]`, `-1`) that don't read as one bug. Grouping candidate files by **`container.querySelector` call count** rather than by symptom is what identified these correctly, and — the part that mattered — correctly *excluded* the neighbouring files that were failing for unrelated reasons. ### One thing to know if you repeat this A blanket `container` → `document` replace is wrong: it also rewrites `renderResult.container.querySelector` into `renderResult.document.querySelector`, which is not a thing. That broke two already-passing tests on my first attempt. This uses two separate passes with a lookbehind so only the bare receiver is rewritten. ### Scope Test-side only — **no product code changes**, so no changeset. This does not fix the *cause* (tests are still free to query the wrong root); a lint rule for that is worth considering separately, but it would need to distinguish portal-mounting components from ordinary ones, and I did not want to guess at that boundary inside a test-fix PR. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved modal and task detail accessibility test reliability by querying rendered elements from the document. * Updated coverage for keyboard navigation, Pull Request status indicators, tab ordering, and onboarding provider cards. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
fd795883c5 |
feat(missions): per-mission taskPrefix override for triaged task ids (#2347)
## Summary Maintainer re-land of [#2334](https://github.com/Runfusion/Fusion/pull/2334) (fork `flexi767:feat/per-mission-task-prefix`) after resolving merge conflicts with current `main`. Fork push was unavailable despite `maintainerCanModify`, so this branch carries the conflict resolution. ### Feature - Optional per-mission `taskPrefix` for triaged task ids (inherits project prefix when unset) - Dashboard MissionManager + routes + store/triage plumbing - Postgres migration for `project.missions.task_prefix` ### Conflict resolution - Main claimed migration **0026** (bigint counters) and **0027** (workflow IR pin) - Mission task-prefix migration renumbered **0026 → 0028** - Baseline `0000_initial.sql` includes `task_prefix` on missions - `legacy.ts` keeps code-org re-exports; `missions.ts` carries `taskPrefix` on create/update types ## Test plan - [ ] CI green (lint/typecheck/build/gate) - [ ] Create mission with custom prefix; triage feature → task ids use that prefix - [ ] Clear mission prefix via PATCH null; new tasks inherit project prefix Closes / supersedes #2334 once this lands (or re-point the fork PR). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Missions can now set an optional per-mission task ID prefix (overriding the project default). * Added task prefix support to mission create/edit UI and dashboard APIs, including normalized uppercase values and validation. * **Bug Fixes** * Improved commit hook generation for custom prefixes and special characters, with safer shell handling to prevent unsafe interpretation. * **Chores** * Added PostgreSQL migration and schema-applier support to persist and propagate mission task prefixes, including upgrade/backfill coverage. * **Tests** * Added backend and UI/API test coverage for task-prefix creation, clearing, and ID minting behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
gsxdsm
|
e6e70a2562 |
test(engine): delete two temp-cleanup mechanisms guarding a leak the harness already prevents (#2960)
`scheduler-paused-dispatch-refusal.test.ts` carried **two** tracking arrays and **two** `afterEach` hooks, both collecting the same `mkdtempSync` path and removing it twice. One was added per review round on #2779 — I wrote both, and neither round noticed the other. The obvious fix is to merge them into one. **I checked whether the leak was real first, and it isn't.** ### Measured `packages/core/src/__test-utils__/vitest-setup.ts` **redirects `os.tmpdir()`** to a per-worker sink and sweeps it by owning pid. So `tmpdir()` inside a test does not resolve to the real temp root at all. Probing the paths this file actually creates: ``` /var/folders/.../T/fusion-test-workers-8Tv8um/redir-5845/fusion-paused-dispatch-ZUhUVL ``` | run | fixtures created | left behind | |---|---|---| | cleanup as shipped | 4 | 0 | | **cleanup disabled** | 4 | **0** | The sink is reclaimed either way. Both mechanisms were appeasing a review comment about a problem that could not occur. ### Why deleted rather than merged A cleanup that cannot be observed to clean anything is not a cheap safety net — it is a claim the file cannot back, and it misreports which layer owns temp lifetime. Keeping one "just in case" would leave the next reader believing this file manages its own fixtures. If the redirect is ever removed, cleanup belongs in the shared setup for **every** test, not re-added file by file. An FNXC note records the measurement and says exactly that, so a third round doesn't re-add a third copy. ### A note on my own measurement My first check was `ls $TMPDIR/fusion-paused-dispatch-*` before and after — it reported zero leaked with cleanup **on**, which I nearly took as "cleanup works." It also reported zero with cleanup **off**. That contradiction is the only reason I looked further; the glob was measuring a directory the fixtures never reach. The before/after count would have "confirmed" a working cleanup just as readily as a redundant one. **Verified:** 4/4 pass, `tsc` 0 errors, lint clean, FNXC gate exit 0. Test-only, no product change, no changeset. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
3461ae7a92 |
docs(gate): record why the SQL-literal gate deliberately does not scan .sql (#2957)
Comment-only. No behavior change. ## Why this is worth a commit #2954 fixed the FNXC-date gate's walk: its extension filter listed the file types stamps were **expected** in rather than the ones they **occur** in, so it was blind to `.sql` and `.css`. That is a tempting pattern to generalize, and `check-sql-column-literals.mjs` is the obvious next candidate — a gate about *SQL* column literals that scans only `.tsx?`. Applying the same fix here would be wrong, and quietly so. ## The two gates are not the same kind of tool The FNXC gate is a plain-text regex scanner, so widening its extension list is trivially correct. This one is **AST-based**: `ts.createSourceFile(..., ScriptKind.TSX)` followed by a walk over string and template nodes. A `.sql` file is not TypeScript. Adding the extension would not widen coverage — it would feed DDL to the TS parser and traverse whatever lenient-mode nodes fell out. The gate would then **report coverage it does not have**, which is strictly worse than not looking, because the silence would read as "SQL is clean." ## Measured before deciding 38 tracked `.sql` files contain exactly one lifecycle-looking literal: ``` 0022_ideation.sql:19 CONSTRAINT ideation_sessions_status_check CHECK (status IN ('open','converged','archived')) ``` That is the **ideation-session** status enum — a different domain that happens to reuse the word — not a `tasks.column` comparison, and not something this gate would flag even if it could parse the file. **Zero real offenders.** So the honest scope is recorded as: raw SQL is **unwatched**, and the trigger that would make it worth watching is a data backfill (`UPDATE tasks SET column = ...`) landing in a migration. If that ever happens it needs a separate raw-text matcher against the exported `COMPARISON`, not an entry in the extension filter. ## Verification - `check-sql-column-literals` → exit 0 - `check-fnxc-future-dates` → exit 0, "478 known, none added" (the new stamp is dated today, local) - `scripts/__tests__/check-sql-column-literals.test.mjs` → **26 pass, 0 fail** - `scripts/__tests__/check-inert-flag-seams.test.mjs` → **12 pass, 0 fail** - `eslint` clean ## Why a comment rather than a doc Per AGENTS.md, decisions of this shape belong next to the code they constrain. The failure mode is specifically someone reading the walk, noticing `.sql` is missing, and "fixing" it — so the note has to be at the filter, where that person is looking, not in `docs/solutions/`. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
6c1f074773 |
fix(core): the in-review stall signal never got the board's review lanes — 0 of 4 call sites (#2956)
## Main is red, and the red is pointing at a real defect `unwired-lane-parameter-guard` fails on `origin/main` after #2951. This is not a stale allow-list — the parameter genuinely never reaches the function. #2951 added `reviewColumns?: ReadonlySet<string>` to three signal modules and wired two of them completely. **`getInReviewStallReason` was wired at none of its four call sites.** Measured by brace-matching each call's option literal: ``` getInReviewStallReason L227=NO L390=NO L599=NO L729=NO getInReviewStalledSignal all 4 wired getStalePausedReviewSignal both wired ``` ## The user-visible consequence `reads.ts` computes two adjacent signals for the same card. On a board declaring a **separate merge lane beside its human-review lane**, `inReviewStall` read the *first* review column only, while `inReviewStalled` — three lines below — read the *set*. **The same card is "in review" for one signal and not the other.** Two signals disagreeing is worse than both being legacy, and it is invisible on every builtin board because there the review set has exactly one element. At three of the four sites the resolve sat *below* the call, which is why the parameter could not be passed. Those are hoisted. ## I have to correct my own earlier report On #2951 I said *"3 of 4 call sites wired, `reads.ts:227` is the gap."* **That was wrong.** I had measured with a 12-line proximity grep, which bled into the adjacent `getInReviewStalledSignal` call and counted its `reviewColumns:` as the first call's. Brace-matching the literal shows 0 of 4. The defect was four times larger than I reported, and the cause was exactly the anti-pattern I have spent this session filing against other people's guards — a proximity window standing in for structure. ## Naming the context types The guard keys an interface member to its **owner symbol** and only counts a mention from a file that also names that owner, so passing the property inline reads as unwired even when every site supplies it. `satisfies InReviewStalledContext` / `satisfies StalePausedReviewContext` on the option literals is real type-checking, not a decorative import — lint rejected the decorative version, correctly. ## New test, because the existing guard cannot see this Measured: **deleting the `reviewColumns:` line from a fixed call site leaves `unwired-lane-parameter-guard` at 9/9 green**, because the file still names the type. So the wiring I just fixed had no coverage at all. The new ratchet brace-matches each call site's option literal: | mutation | result | |---|---| | remove lanes from one call site | **1 failed** — *"1 of 4 getInReviewStallReason call sites omit reviewColumns"* | It also asserts it **found** call sites before checking them — a parse that matched nothing would be vacuous, which is the failure mode this guard family keeps producing. (It caught me mid-change too: an earlier scripted edit left the file syntactically invalid and the source-text test still passed 3/3. It is a wiring ratchet, not a substitute for `tsc`.) ## Verification Core **4861 passed / 0 failed** · guard **9/9** with `KNOWN_UNWIRED` **unchanged** · `pnpm test:gate` **exit 0** · lint clean · core `tsc` **0 errors**. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
ed907e93dc |
chore: tighten the FNXC future-date baseline as the clock advances (#2953)
Retires `packages/core/src/task-store/reads.ts`'s allowance of 2. Its FNXC stamps are no longer in the future, so the headroom they held is removed rather than left sitting where a genuine regression could hide inside it. ## This is the drop path working, not a fix The gate watches a **clock-dependent population**: stamps age out of "future" on their own, with no code change. That is why the drop path auto-lowers the baseline and exits 0 instead of failing. The alternative — failing on drops, the way a code-measured ratchet should — would have turned this repo red on a day nobody touched it, and the noise would have trained everyone to re-baseline without looking. **A ratchet may only fail on drops when its measurement depends solely on code.** This one does not, so it tightens silently and a commit like this one records the new floor. ## What I verified - Second consecutive run exits 0 with no further diff — the tightening is idempotent, not an oscillation between two states. - Diff is a single removed line; the entry is deleted rather than set to `0`, so the file re-enters as a genuine addition if a future-dated stamp lands there again. - `467` known future-dated stamps remain across 237 files, unchanged. ## What this does not do It does not reduce the future-dated stamp count — those 467 are still there and still wrong. They came from agents (me included) stamping FNXC comments a day or two ahead. This only reclaims the allowance for one file that has aged out. **The count will keep falling on its own without anyone fixing anything**, so it should not be read as cleanup progress; the gate's value is blocking *new* future stamps, which it still does. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
255741e9ab |
fix(gate): scan every file type that carries an FNXC stamp (#2954)
The walk's extension filter was `/\.(tsx?|m?js|cjs|md)$/` — the file types stamps were **expected** in, rather than the ones they **occur** in. Wherever the convention spread on its own, the gate could not see it. ## How I found it Chasing four stamps dated `2026-10-19` — three months out, so unlike the rest of the population they would not age out on their own. All four were in `packages/core/dist/`, which the gate correctly skips as generated. The *source* they were compiled from is a `.sql` migration, which the gate skips for a different and much worse reason: it was never scanned at all. ## Why `.sql` is the expensive omission A migration's stamp records **when a schema change landed**. That is the case where a wrong date misleads most — it is the file you read to reconstruct the order schema changes happened in. 69 migration files carry stamps; 10 were future-dated and none were visible. `.css` had drifted furthest by volume: **1023 stamps across 123 files**, almost all from the dashboard CSS split. `.html`, `.ya?ml`, `.json`, `.sh` are included too; they add coverage but contribute no baseline entries. ## The 9 new baseline entries are newly VISIBLE, not new 5 `.css` + 4 `.sql`. Every one predates this change and would have been caught had the gate ever looked. Recording them is a **reclassification**, the same distinction the census draws for its DELIBERATE-LITERAL marker — a baseline that grows here is the gate's coverage improving, not the codebase regressing. Reading the rise as a regression would be exactly backwards. ## Verified by mutation, not by reading - A future-dated stamp appended to `ChatView.css` → gate **exit 1**. - A future-dated stamp appended to `0036_chat_session_tags.sql` → gate **exit 1**. - Both reverted → **exit 0**. Without this, both probes pass silently. ## Two notes on the diff - **Zero removals.** My first attempt rewrote the baseline with sorted keys, which turned unmoved lines into add/remove pairs and made it look like entries were being dropped. Rebuilt in walk order so the diff is additions only. - `reads.ts` is deliberately left at `2` here even though it now measures `0`. That drop belongs to #2953; duplicating it across two open PRs is how this queue got tangled before. The gate auto-tightens it at runtime and still exits 0. ## What this does not fix The **478** future-dated stamps still in the tree. They are agent-written (mine included) and most are one or two days out, so the count falls on its own as the clock advances — it should not be read as cleanup progress. This PR only makes the gate able to *see* the SQL and CSS ones, so no new stamp can land there unnoticed. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
b8e5d42b7a |
chore(gate): move the FNXC date ratchet beside its three siblings (#2952)
The half that #2948 and #2950 did not cover. Both of those fixed today's redness; **#2949** landed the un-redding first, so both are now conflicting and redundant. This is the placement, which is what made today's failure so expensive. ## Why it hurt `check-fnxc-future-dates` was wired into `pretest` **and** `test:gate`, with no `check:*` script and no `pr-checks.yml` step. So a baseline frozen below the tree it froze did not produce "one CI step is red" — it produced: - `pnpm test:gate` → exit 1, merge gate down for everyone - `pnpm test` → refuses to run before a single test executes ## The precedent All three sibling ratchets are dedicated `pr-checks.yml` steps. `lifecycle-column-census` always has been; `check-sql-column-literals` and `check-inert-flag-seams` moved there in #2941. The census's own header states the reason, and it is the one that matters here: > a permanently-red gate is a bigger hole than a stale allowance, because it gets ignored and then nothing is guarded at all ## The change ``` check:fnxc-future-dates script, beside check:inert-flag-seams "FNXC stamp dates" step in pr-checks.yml, after the other three removed from pretest / pretest:full / test:gate ``` **Enforcement where it matters is unchanged** — `pr-checks.yml` is the blocking gate, so a newly added future-dated stamp still cannot merge. What changes is that a baseline mismatch stops halting work unrelated to it. ## Deliberately not touching The drop behaviour. This gate **already** tightens on a drop rather than failing — the #2888 pattern, already correct here. I checked rather than assuming it needed the same fix its siblings did. ## Verification `pnpm check:fnxc-future-dates` exit 0 · `pnpm test:gate` green (now without this check in it) · lint 0 · step confirmed adjacent to the other three ratchets. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Added automated validation for FNXC stamp dates to lint checks. * Updated test and validation scripts to run the date check through a dedicated command. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
8e5e1147d2 |
core,engine: the last literal lifecycle query — and the three stall signals that disagreed (#2951)
**This is the last one.** `surfaceInReviewStalls` was the final literal
`listTasks({ column })` in production — I verified it by direct scan,
not by census arithmetic: **1 remaining before this, 0 after.**
It tells an operator that a card is stalled in review. On a renamed
board the stall was real and the board simply never said so.
## It came last on purpose
Converting the read alone would have been **worse than leaving it**.
`getInReviewStallReason` gated on the literal `in-review` itself, so a
widened read hands every renamed-board card to a classifier that drops
it — the missed-pair class, wearing the shape of a clean one-line
conversion.
## What was actually there
Three sibling signals decorate the same row, and they **disagreed about
which lane it is in**:
| signal | before |
| --- | --- |
| `getInReviewStalledSignal` | singular `reviewColumn` — resolved, but
**first-per-role** |
| `getStalePausedReviewSignal` | singular `reviewColumn` — same |
| `getInReviewStallReason` | **no seam at all** — literal |
So one row could be judged in-review by one signal and not by another.
And the singular ones are the **arity trap**:
`resolveLifecycleColumns().review` is the *first* column carrying a
review role, so a board with a separate merge lane beside its
human-review lane had a second review column matching none of them.
All three now take `reviewColumns` (membership), resolved **once per
row** through `resolveReviewColumns` — the union of the three review
roles — so they cannot disagree by construction. The singular/literal
paths remain as the no-metadata fallback, so a caller passing nothing is
byte-identical to today. Ten call sites in `reads.ts` wired from that
one answer; the singular resolver is deleted.
## Revert results
Each applied alone and re-run:
| conversion | reverted → |
| --- | --- |
| the resolved read | fails — the card is never listed |
| `reviewColumns` at the call | fails — the classifier drops the renamed
card the widened read just found |
That second row is the whole point: it proves the pair had to move
together, which is the thing I got wrong twice earlier in this series.
## Second commit: a red on `main`, not from this branch
`check-fnxc-future-dates` landed and **`main` fails it** — verified by
running the script on a clean `origin/main` checkout rather than
inferring. Nine files carry stamps dated after today, so every worker's
gate fails on a check none of their changes caused. Several are mine: I
had been stamping tomorrow's date across this whole series, which is
precisely the out-of-order record the check exists to prevent.
Scope held deliberately: a repo-wide sweep touched **266 files** across
docs, scripts and every package. I ran it, backed it out, and limited
this to the nine files the check actually flags — a mechanical rewrite
that size during a queue freeze would conflict with every in-flight
branch, which is worse than the red it fixes.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71 (green **only** with the stamp
commit); `@fusion/core` full suite **4810 passed**; engine self-healing
+ blindness + both ratchets **758 passed**; `tsc` clean on core and
engine; `pnpm lint`, `check:changesets`, `lifecycle-column-census
--strict`, `check-sql-column-literals` and `check-fnxc-future-dates` all
clean.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Review-stall detection now recognizes renamed and multiple review
columns while retaining support for the legacy review column.
- Paused tasks continue to be excluded from stall detection.
- Self-healing review-stall sweeps now search all configured review
lanes and avoid duplicate task results.
- **Tests**
- Added regression coverage for renamed and legacy review-lane queries.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
gsxdsm
|
f49e487d91 |
feat(core): untraited-project lane opt-in — and main was red on the FNXC gate (#2949)
Two things, and the second is why the first does not ship alone. ## The opt-in `resolveProjectColumnsForRoles` gains `untraitedProject: "declared-columns"`. When **no** workflow in the project expresses **any** lifecycle trait, every declared column id joins the answer. This is the three-state rule at **project** scope — the last item on the deferred list, recorded at three self-healing call sites (#2869, #2876). A board that renames its lanes and declares no traits contributes nothing today, so its cards are **absent from every role-keyed query**, and the correct per-card fallback downstream never runs for them. A fallback cannot rescue a card the query never returned. **Not "no workflow declares this role."** A project that expresses traits and has no review lane has *answered*; widening there would invent lanes it deliberately lacks. Mutation-verified both directions — widening unconditionally fails 1 of 12, making the option a no-op fails 1 of 12. **Opt-in, not default**, because the safe direction differs per caller — the finding in `project-union-versus-per-task-lanes.md`: | caller | over-inclusion costs | |---|---| | sweep | nothing — the per-card check discards the extra rows | | aggregator | an inflated number an operator reads (#2864, #2866) | | action site | a card routed or notified under a vocabulary that is not its own (#2852, #2891) | Making it the default moves all three at once, in the one direction two of them must not. Verified byte-identical without the option, so this lands with **no caller changes** and each site adopts it on its own reasoning. ## Main was red, and my own gate caught me first I dated the new comments `2026-07-31` while today is `2026-07-30` — **the exact defect `check-fnxc-future-dates` exists to prevent, committed while writing the feature.** The gate I added yesterday failed my own commit. Correcting mine surfaced that the merged sentinel batch, #2947, and three engine test files carried future-dated stamps too, so **the gate was failing on `main` for everyone**, not just here. All corrected to real dates rather than raising the ceiling. The stamps were simply wrong, and a baseline bump would have recorded the error as permitted — which is the failure mode that ratchet exists to prevent. Core and engine `tsc` clean, `pnpm lint` clean, census `--strict` 0, FNXC gate 0 (469 known, none added), gate green (161/487/13/71). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
b1bd571682 |
batch-sql-ratchet: the census / gate-ratchet family — collection branch, fold here (#2941)
## Family branch for consolidation directive item 4 `batch-sql-ratchet` did not exist and ~10 open PRs are waiting for a collection point, so this establishes it. **Fold your census/ratchet commit here and close your own PR as superseded.** ```bash git fetch origin batch-sql-ratchet git checkout -B batch-sql-ratchet origin/batch-sql-ratchet git cherry-pick <your-sha> # verify scoped, not full suite: pnpm --filter @fusion/core exec vitest run src/__tests__/archived-column-gate-parity.test.ts --silent=passed-only --reporter=dot git push origin HEAD:batch-sql-ratchet ``` **Candidates I can see open right now** (owners: please fold + close): | PR | branch | |---|---| | #2938 | `fix/comments-ops-sentinel` | | #2935 | `fix/task-artifacts-sentinels` | | #2933 | `chore/commit-tightened-census-baseline` | | #2931 | `fix/async-comments-sentinels` | | #2928 | `fix/audit-ops-sentinel-marker` | | #2925 | `live-task-column-lanes` | | #2923 | `fix/task-id-integrity-sentinel` | | #2921 | `fix/plugin-store-migration-marker` | | #2894 | `gate/sql-literals-match-census-placement` | That is **10 → 1** once folded. I have not cherry-picked anyone else's commits — folding someone's work without them verifying it is how a batch lands broken. --- ## What is in it so far (mine, from #2924) **Clears a live main red:** `archived-column-gate-parity` fails on `origin/main` today. ``` AssertionError: TypeScript encoding changed. async-comments-attachments.ts: 8 → 5 ``` #2886 fixed a real bug — archived-document guards failing in *opposite* directions on a renamed lane — by replacing three `column === "archived"` comparisons with `isArchivedLane(column, archivedColumns)`. The AST scan counts raw comparisons, so the tally dropped. **What I did not do is record it as three sites converted**, because measured, it is not: ``` grep -rn "archivedColumns:" packages/core/src packages/engine/src --include="*.ts" | grep -v __tests__ → (no matches) ``` No caller passes it. The parameter defaults to `LEGACY_ARCHIVED_LANES = new Set(["archived"])`, so every call resolves to the literal it replaced — byte-identical behaviour, resolved branch dead. That matters for this guard's whole argument: its header warns that converting the TypeScript half while the Drizzle and raw-`sql` halves still compare the string is a split brain *"no test would catch, because every builtin workflow spells the column `archived` so the two halves agree by accident on every board we ship."* **There is no split brain today precisely because the resolved half is unwired** — it becomes one the moment a caller threads real lanes in without the SQL sides moving. Recorded inline so `5` cannot be read as "3 sites done"; flagged on #2886. Verified not a split brain: the Drizzle and raw-sql inventories are unchanged and both pass — worth stating because those assertions run *after* the TypeScript one, so a plain red says nothing about them. Scoped edit to `AUDITED_TS_SITES` by line range: these paths appear in more than one inventory here, and an unscoped replace would quietly edit the raw-sql side too, making the parity guard agree with itself (the trap I hit in #2817). Guard still bites: appending a real `task.column === "archived"` to an audited file fails it. Core **4852 passed / 0 failed**, lint clean, test-only. Closing #2924 as superseded by this. 🤖 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 task delegation messages when workflow pickup cannot be confirmed. * Delegation results now clearly indicate when a task has not been verified for pickup. * **Quality Improvements** * Added validation checks to catch future-dated markers and inconsistent SQL-column usage. * Refined workflow checks to distinguish stale configuration from incomplete configuration. * **Documentation** * Updated lifecycle conversion guidance with more accurate audit findings and limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
c3df0f641b |
executor: orphaned tasks were never resumed after a restart on a renamed board (#2947)
`resumeOrphaned` is the only path that recovers tasks after a crash or
restart. On a board with renamed columns it recovered **nothing**.
## A missed pair, not an unconverted read
```ts
const tasks = await this.listWipLaneTasks(); // resolved by role — already converted
const inProgress = tasks.filter(
(t) => t.column === "in-progress" && …, // literal — discards everything the read found
);
```
The read was already resolved. The filter directly beneath it
re-asserted the literal on the rows that read returned, so the sweep
found the orphans and threw them all away.
**This is the worse half of the class, and it hid well:**
- the read *looks* converted, so scanning for `listTasks({ column: "…"
})` finds nothing;
- the census scores only the comparison, so the backlog number moves the
**wrong way** as you convert;
- a **structural test already existed** pinning "the read asks for
resolved lanes" — `executor-resume-query-lanes.test.ts` — and it was
green the entire time the sweep was dead. A test asserting the read
exists says nothing about the filter beneath it.
The failure only surfaces after a crash, when an operator is already
investigating the crash and has every reason to blame that instead.
## The ratchet, generalised
#2944 ratcheted this class inside `self-healing.ts` after review found
one instance and a follow-up audit found five more. This generalises it
to every engine source: a function that resolves lanes **and** compares
a column id in the same body is a pair.
Excluded, deliberately:
- the **fallback arm** of a resolved ternary (`lanes ? lanes.has(c) : c
=== "done"`) — the correct shape;
- four files whose literals are deliberate, each with the reason
recorded: `ephemeral-worker-manager` (unresolvable-workflow default),
`triage` (the U11 orphan case), `scheduler` and `replan-target` (sync
listeners on the inert sync IR reader, already pinned by
`sync-workflow-ir-is-always-default.pg.test.ts`);
- `self-healing.ts`, because it has a **dedicated** ratchet that is
strictly more precise. Two ratchets allowlisting the same site is one
fact with two owners, free to drift — the exact failure mode this
program keeps hitting. One file, one ratchet.
It carries a positive control: a wrong source path would make every case
pass by scanning nothing.
**I swept the rest of the engine with it and executor.ts was the only
genuine hit** — everything else is documented-deliberate or blocked on
the inert sync reader.
## Revert results
Each measured by restoring the literal filter and re-running:
| | reverted → |
| --- | --- |
| behavioural case | fails — the renamed card is dropped and the sweep
returns before touching it |
| the ratchet | fails, naming the site: `resumeOrphaned:
executor.ts:5974` |
A non-vacuous companion (card in the review lane → not resumed) rules
out a filter that matches everything: a card in review has no session to
resume, and re-dispatching it would restart finished work.
**Measured:** `executor.ts` column guards 8 → 7; baseline re-recorded
downward.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71; executor prompt/soft-delete/resume
suites plus the new ratchet, 357 passed; `tsc` engine clean; `pnpm
lint`, `check:changesets`, census `--strict` and
`check-sql-column-literals` clean, each run explicitly.
|
||
|
gsxdsm
|
8503a2b12f |
batch-census-sentinels: six sentinel-marker PRs in one (supersedes #2921 #2928 #2931 #2935 #2938 +1) (#2943)
Fifth family, not in the four you listed — it was about to sit while the others consolidated. **Six folded; two need arbitration.** ## Folded (cherry-picked clean) migration marker · async archived check · audited-sentinel missing its marker · five of six `archived` checks in one file · the two artifact/comment read-only guards · the last unmarked `getLiveTaskColumn` sentinel. One root cause, which is why they belong together: **a literal compared against a SENTINEL value is not a lifecycle-lane guard** — the census counts it, and the fix is a marker, not a conversion. ## The baseline conflicted on every cherry-pick All six re-recorded `lifecycle-column-census-baseline.json` independently. I resolved by **regenerating once from the folded tree** rather than merging six hand-edits: the baseline is a derived artifact, so the measured value is the only correct resolution, and hand-merging derived JSON is how a wrong ceiling gets locked in. That is the strongest case for the family model I can give you: six PRs touching one derived file conflict pairwise regardless of merge order — 15 possible pairs — and auto-rebase would have churned them serially. ## NOT folded — one line for arbitration **#2925 (`live-task-column-lanes`) conflicts with #2923 (`fix/task-id-integrity-sentinel`) on `packages/core/src/task-store/task-id-integrity.ts`.** #2923 marks a sentinel there; #2925 converts lanes. Different intents, same file. I did not guess which wins — land one, rebase the other, fold both after. ## Verification `--strict` exit 0 · backlog **158**, reviewed **122** · core typecheck clean · scoped, not full suite. ## Queue **52 → 39** after my two folds (this + #2940 portal). The ~24 "self-healing … on a renamed board" family is still the dominant block. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified lifecycle-state terminology and migration markers throughout task and project management documentation. * Documented the distinction between archived-task sentinels and workflow column identifiers. * Updated lifecycle documentation tracking to reflect the latest coverage. * **Bug Fixes** * No runtime behavior changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
8b75a42d22 |
batch: self-healing sweeps were blind on renamed boards (26 sweeps, folds 23 PRs) (#2944)
**Consolidation of 23 open PRs into one.** Every one shared a single root cause and mostly touched a single file; 23 CI runs for that was indefensible. Folds and supersedes: #2867 #2869 #2876 #2879 #2883 #2891 #2899 #2901 #2902 #2905 #2906 #2914 #2916 #2918 #2919 #2920 #2922 #2927 #2929 #2932 #2934 #2937 #2939. (#2865, #2882, #2897, #2909, #2912 already merged and are not re-folded.) ## The root cause A self-healing sweep selects its work with `listTasks({ column: "in-review" })`. On a board whose lanes are renamed that returns **nothing**, so the sweep never runs — no error, no log line, no failed task. Several sweeps had already had their *predicates* converted to resolved lanes, which dropped a census count and changed nothing, because the query above the loop had already returned an empty list. **26 sweeps converted.** Each one: read the project's columns for the role, then decide each card against **its own** workflow, with the legacy ids unioned so a board mid-rename is never skipped. ## What each sweep stops silently failing to do | | | | --- | --- | | stale merger status | one finished card held the **merge queue** for everything behind it | | stale `blockedBy` / completed-task release | dependents stayed blocked on work that had already finished — the board stops moving | | workspace partial lands | a task left with **some repos merged and some not** | | mid-merge retry stamp | the card stalled *and* the operator's manual Retry was gated by the same stamp | | in-progress limbo / no-progress failures | dead cards held a work slot forever | | partial-progress retry | real work parked failed with its **retry budget unspent** | | orphaned-execution signal | visibility only — the one signal pointing at an orphan went silent | | zero-commit audit | went **half-blind**: the error arm kept working, the lane arm did not | Plus: ghost review cards, transient merge failures, misclassified failures, branch misbinding, missing-worktree failures, merged-but-unfinished finalization, done-metadata repair, self-owned branch conflicts, orphan-only scope violations, post-done wedges, idle assigned agents, PR-conflict worktree ownership, and orphaned workspace worktrees. ## Two defects the conversion itself introduced, both caught and fixed 1. **Missed pairs.** Widening a read without converting the guards beneath it is *worse than not converting*: the sweep starts admitting renamed-board cards and then mis-decides every one. Review caught a second guard on a re-read row; the audit that triggered found **five more**, one of which gates the `reviewProof` triple-proof — a renamed review card would have been moved backward with the safety check silently skipped. Column guards 86 → 81. 2. **Duplicate processing.** The literal reads were disjoint by construction; resolved reads are not, so a column carrying two role flags put one card in two buckets — duplicate moves, duplicate audit rows, inflated counts. Both now have ratchets. `self-healing-converted-sweeps-have-no-literal-lane-guards.test.ts` **derives** its sweep list (a sweep counts as converted when its body calls `resolveProjectColumnsForRoles`), so it cannot go stale, and it carries two positive controls because a broken regex finds no offenders and a broken derivation iterates nothing — an empty loop registers no tests and reads green. ## Deliberately unchanged - 22 `moveTask` destinations carrying `recoveryRehome: true` — `moves.ts` exempts these so a card stranded in an undeclared column stays rescuable. - One literal in `clearStaleBlockedBy`'s log-dedup closure (allowed by name in the ratchet, with the reason). - `surfaceInReviewStalls` — hot list-read path, needs a batched prefetch; that is a performance design decision, not a conversion. - `scheduler.ts` and `replan-target.ts` — built on `resolveTaskWorkflowIrSync`, which returns the default IR for every task in production. Converting there produces inert code. ## The fold itself is worth one note All 23 branches appended to the **same test file at the same anchor**, so every automatic strategy — git 3-way, `merge-file --union`, and three hand-written resolvers — interleaved them mid-block. Two attempts committed conflict markers before I caught it. The file is therefore **reconstructed**: head authored once, body assembled as the union of each branch's own intact top-level segments keyed by test title, with the nested `already-merged hard blocker` describe appended whole (flattening it orphaned its helper). Verified by *parsing after every step* rather than trusting the merge — which is how each interleaving was caught. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71. Scoped suites 592 passed (self-healing, the blindness suite at 68 cases, the ratchet, and the notification suite). `tsc` engine clean; `pnpm lint`, `check:changesets`, `lifecycle-column-census --strict` and `check-sql-column-literals` all clean, each run explicitly. Each folded conversion was individually revert-proven on its original branch — the read reverted alone, and the per-card verdict reverted alone — and those measurements are recorded in the commit messages carried into this branch. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
7b68f20501 |
batch(docs): fold the three workflow-learnings / annotation PRs into one (#2942)
## Family batch — replaces #2926, #2892, #2887 Per the consolidation directive: the u9/e2e **docs family**, folded into one branch and one CI run. Three PRs, five commits, **five files, comment and markdown only**. | folded PR | commits | |---|---| | #2892 `docs/union-vs-per-task` | the project union and the per-task answer are not ranked; date correction | | #2926 `docs/date-my-measured-claims` | date the measured claims (one was wrong); date the grep-vs-AST measurement in the SQL gate header | | #2887 `docs/archived-state-literals` | mark the three archived STATE literals as deliberate | Cherry-picked in original order with authorship preserved; all five applied clean, no conflicts. ## Scope is provably comment-only ``` docs/solutions/workflow-learnings/lifecycle-conversions-that-score-as-wins.md docs/solutions/workflow-learnings/project-union-versus-per-task-lanes.md packages/core/src/task-store/async-maintenance.ts ← FNXC DELIBERATE-LITERAL annotation packages/core/src/task-store/workflow-definitions.ts ← FNXC DELIBERATE-LITERAL annotation scripts/check-sql-column-literals.mjs ← header prose only ``` Every added line in `packages/` and `scripts/` is inside a comment — checked by filtering the diff for declarations, conditionals and returns, which returns nothing. The two core files gain `DELIBERATE-LITERAL` markers explaining that `'archived'` is a **state** marker there, not a lane: the sweep collects rows Fusion itself archived or soft-deleted, so widening to the resolved archived set would pull live cards into a cleanup pass. ## Verification (scoped, per the directive — not the full suite) - `pnpm lint` — clean - `check-sql-column-literals` — exit 0 (the file it annotates) - `check:lifecycle-columns` — exit 0 (the markers it adds are census-visible) - `sync-workflow-ir-callsite-allowlist.test.ts` — 3/3 ## A correction worth recording Mid-fold I saw a changeset, `self-healing.ts` and a test file in `git diff origin/main..HEAD` and nearly reported the batch as impure. They were **main's own commits** — `origin/main` advanced between branch creation and the diff, so the comparison was against a stale base. Rebasing onto current `main` reduced it to the five files above. Worth flagging for anyone else folding a family today: with `main` moving this fast, diff the branch **after** rebasing or the file list will lie to you. ## Closing the originals #2926, #2892 and #2887 are superseded by this and are being closed. I hold no PRs of my own in this family — all mine merged — so this fold is on behalf of the family rather than a rollup of my own work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
b4ed12e9c8 |
batch-u7-lane-fixes: three core/engine renamed-board fixes folded (was #2925, #2930, #2936) (#2925)
**Consolidated per the queue freeze.** Three single-fix PRs of mine folded into this one branch; #2930 and #2936 are closed as superseded. Net effect on the queue: **3 → 1**. All three are the same root cause — a lifecycle lane compared against a legacy id — and all three carry a measured revert proof. Verified scoped (not full suite) on the folded branch: `tsc --noEmit` clean, `pnpm lint` clean, SQL-literal gate green, census `--strict` green, and 61 tests across five suites plus the guard at 9/9. --- ### 1. `getLiveTaskColumn` produced the archived sentinel from a literal (was #2925) `getLiveTaskColumn` **manufactures** the string `"archived"` that a dozen comparisons across five files trust — and it tested `row.column === "archived"`. A live row in a renamed archived lane read as **live**, so the gates hiding an archived card's artifacts and document listings never closed. Fixing those twelve comparisons individually would have been wrong twice over: **they are sentinels, and the defect was in the producer.** One line, once, and all twelve become correct. `resolveArchivedLanes` moved to `project-lane-vocabulary.ts` — three private copies of one fact is how the "write guard says yes, publication guard says no" disagreement happens at scale. *Revert proof (real PostgreSQL):* restore the literal → `expected [ { …(14) } ] to deeply equal []`. **Caught myself shipping the unwired shape here:** I added the parameter to seven functions and wired none of their impl callers — the exact inert-conversion defect this program exists to remove. The failing test is the only reason I noticed. ### 2. Mission delivery repair refused a completed card (was #2930) `getTerminalTaskEvidence` tested only `column === "done"`, so a completed card on a renamed board classified as `nonterminal` and `reconcileFeatureDoneWithTerminalTask` threw `TASK_NOT_TERMINAL: … not shipped`. Valid operator work refused — with the message naming the real column while the check couldn't see it. The **type** blocked the fix from the far end: `TerminalTaskEvidence` pinned `column: "done"` / `"archived"`, so the resolver couldn't report the real column without a compile error. `kind` already carries the role, so `column` is free to carry the truth. *Revert proof (real PostgreSQL):* restore the literal → `TerminalTaskReconciliationError: … not shipped`. I had deferred this twice on the premise that `AsyncMissionStore` "holds a layer, not a store". It holds an **optional `taskStore`**, and the single production construction site supplies it. ### 3. The unwired-lane guard reported two FALSE entries (was #2936) `unwired-lane-parameter-guard.test.ts` has been **red on main** since #2875, flagging two `InReviewDurationLanes` properties as unwired when the impl demonstrably supplies both. Cause: my own owner-scoping rule requires a mention from a file naming the declaring symbol — correct for a function, structurally impossible for an interface passed as an inferred object literal. Fixed at the caller (name the type) after trying the tool three ways: relaxing type-owned properties hid **12** genuine entries; resolving owners to consuming functions hid **6**. Each refinement traded the false positive for false negatives — the sign a co-occurrence heuristic has hit its limit. Recording two *wired* parameters in `KNOWN_UNWIRED` was rejected: that puts non-debt in the debt list, which is how a ratchet starts lying. Guard back to **9/9**, baseline unchanged at 17. **This un-reds main.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
97c987c40b |
batch-portal-test-fixes: both portal query-root failures in one PR (supersedes #2911, #2913) (#2940)
Family fold: #2911 + #2913, cherry-picked clean. One root cause (portalled modals queried the render container instead of document.body). Scoped verification: 167/167. NOTE: the portal family is 2, not ~8 — the real jam is ~24 self-healing 'renamed board' PRs plus an unlisted census/sentinel family of ~9. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated modal test coverage to correctly validate content rendered through portals. * Improved assertions for task counts, timestamps, mobile detail views, and keyboard-related behavior. * Added test comments clarifying portal-based rendering and assertion behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
c6767cb258 |
self-healing: foreign-only contamination never cleared on a renamed board (fourteenth sweep) (#2891)
`recoverForeignOnlyContaminatedInReviewTasks` classifies a branch carrying **only foreign commits** and clears the contamination park that nothing else clears. Two literal reads meant that on a renamed board it classified nothing, and the task stayed parked indefinitely. ## The two redundant guards were the interesting part Both filters carried a `task.column === …` check that was **redundant** while the query pinned the column. Under a resolved read they stop being redundant and become the per-card verdict — so they convert here rather than being deleted. Deleting them would have silently widened the sweep, which is the failure this whole class is about. ## Dedupe matters more here than elsewhere The concatenated candidate list is deduped (the P1 reviewed on #2879). It bites harder in this sweep because the two filters have **different predicates**: a column carrying both a review role and the wip role could satisfy both and classify one branch twice. Explicit `has` guard rather than `new Map(entries)` — that constructor keeps first insertion *order* but the **last** value for a repeated key, so it reads as first-bucket precedence while doing the opposite. (Corrected in #2879 and #2883 for the same reason.) ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed, so the classifier is never called | | the review verdict | fails — the renamed review lane does not match and the card is filtered out | Observable is **candidacy**: `classifyForeignOnlyContamination` runs once per accepted card and not at all for a rejected one, which is exactly the read-plus-verdict under test. It is a static named import, so it is intercepted with a scoped `vi.mock` (spyOn cannot rebind an already-resolved ESM binding); only that one export is overridden, so the sweeps in this file that use `inspectBranchConflict` are unaffected. A non-vacuous companion (same card in the board's hold lane → never classified) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
f6e368205e |
self-healing: a card stuck mid-merge could not be retried on a renamed board (twenty-first sweep) (#2912)
`recoverStaleMergingStatus` clears a `merging`/`merging-pr` stamp left on a review card with no live merger behind it. The literal read meant that on a renamed board the stamp was never cleared. **The operator's escape hatch was closed by the same bug that caused the stall.** That stamp is consulted by the merger *and* by the dashboard's manual Retry gate, so the card could neither progress on its own nor be retried by hand. ## The redundant guard converts `task.column !== "in-review"` was redundant while the query pinned the column; under a resolved read it becomes the per-card verdict. Carries the #2891 shape — **narrow when the card can answer, broad when it cannot**. ## Fixture note worth keeping `updatedAt` in the test is deliberately ancient. `isStaleMergeActiveStatus` requires the stamp to have sat untouched for `minAgeMs`, so a fresh fixture would be filtered out for a reason that has nothing to do with lanes — and would then have passed with the fix reverted. That is the shape of most of the vacuous assertions on this branch: a *later* filter rejecting the card, masking whether the lane logic worked at all. ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved read | fails — the card is never listed | | the per-card verdict | fails — the renamed review lane is filtered out | A non-vacuous companion (same stamp on a wip card → untouched) rules out a read that returns everything; a merge stamp in the wip lane belongs to `recoverInProgressLimbo` and the executor, not here. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` and `check-sql-column-literals` clean, each run explicitly. |
||
|
gsxdsm
|
1adf886f04 |
fix(tests): definition-actions queries container; the modal is portalled (12 → 0) (#2893)
## Third file, same root cause The largest block in the dashboard `app:backfill 2/4` shard — and the same defect as #2885 and #2890. **Probed before converting, not pattern-matched:** ``` PROBE container=false document=true ``` `TaskDetailModal` mounts through `createPortal`, so its subtree hangs off `document.body` rather than the container `render()` returns. All **25** container-rooted lookups in this file could only ever return null. ## The regex handles two shapes separately, on purpose `X.container.querySelector` (render-result-scoped) and a bare destructured `container.querySelector` are converted in separate passes, because a blanket replace on the previous file rewrote the former into `renderResult.document...` — not a thing — and broke two passing tests. A lookbehind keeps `triageContainer.` / `todoContainer.` out of the bare pass. ## One case needed more than a query-root swap `does NOT show Changes tab for triage/todo tasks` renders the triage modal and the todo modal back to back and told them apart by their container handles. **Both were empty**, so `querySelectorAll(".detail-tab")` returned `[]` and the assertion compared `[]` against twelve tab labels — it could not have failed for the reason it was written. Querying `document` alone does **not** fix that one: with two modals mounted at once, a document-rooted `.detail-tab` lookup returns *both* tab strips concatenated. Unmounting the first render is what makes each assertion about one modal again. Verified by measurement rather than reasoning — the file only reached 66/66 after the unmount, not after the query swap. ## Evidence | | result | |---|---| | the file | **66/66** (was 12 failed) | | shard `2/4` | **22 → 10** | | mutation: rename `.detail-spec-edit-trigger` in `TaskDetailModal` | **1 failed** | `pnpm lint` clean. Test-only; `TaskDetailModal.tsx` restored clean. ## Running total on the portal defect | PR | file | cleared | |---|---|---| | #2885 | `TaskDetailModal.models-progress-workflow` | 30 | | #2890 | `settings-mobile` | 17 | | this | `TaskDetailModal.definition-actions` | 12 | **59 of the ~111 backfill failures**, all one defect: tests querying `container` for components that render through a portal. It hid because `screen.*` queries in the same files always worked (they query the document), so the failures read as "the component never rendered" rather than "we asked the wrong root". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
b3e16b3988 |
fix(tests): rendering.test queries container; the modal is portalled (28 → 4) (#2895)
## Fourth file, same defect
`TaskDetailModal.rendering.test.tsx` is the whole of the dashboard
`app:backfill 4/4` shard bar one case (28 of 29).
**57** container-rooted lookups converted. `TaskDetailModal` mounts
through `createPortal`, so `container` is empty and every one returned
null — visible in the two failure shapes this file produced:
```
10x expected null to be truthy
~14x expected undefined to be '<some text>' ← container.querySelector(x)?.textContent
```
## Four remain, deliberately
They assert the modal's **wrapper structure**, not its contents:
```ts
expect(document.querySelector(".modal-overlay.open")).toBeTruthy();
```
`FloatingWindow` renders `.floating-window-overlay`
(`FloatingWindow.tsx:627`). The only `.modal-overlay open` left in
`TaskDetailModal` is the unrelated *refine* overlay at `:6801`. So these
pin the **pre-FloatingWindow** wrapper.
Re-pointing them means encoding the *current* modal-shell contract — a
UI structure decision that belongs with whoever owns the FloatingWindow
adoption, not bundled into a query-root fix where it would be easy to
miss. Left failing and flagged rather than guessed at.
It is also a different failure shape from the rest: `expected <div …> to
be null` on a mobile-variant badge, i.e. an assertion that *found*
something, versus 24 that found nothing. Different cause, different fix,
different reviewer.
## Evidence
| | result |
|---|---|
| the file | **120 tests, 4 failed** (was 28) |
| mutation: rename `.detail-id` in `TaskDetailModal` | **5 failed** |
The mutation matters because the change is "query a different root" —
the risk is assertions that now find *something* and stop
discriminating. They still observe the real component.
`pnpm lint` clean. Test-only; `TaskDetailModal.tsx` restored clean.
## The portal defect, totalled
| PR | file | cleared |
|---|---|---|
| #2885 | `TaskDetailModal.models-progress-workflow` | 30 |
| #2890 | `settings-mobile` | 17 |
| #2893 | `TaskDetailModal.definition-actions` | 12 |
| this | `TaskDetailModal.rendering` | 24 |
**83 of the ~111 backfill failures**, one defect: tests querying
`container` for components that render through a portal.
It hid for so long because `screen.*` queries in the same files always
worked — they query the document — so the failures read as *"the
component never rendered"* rather than *"we asked the wrong root"*. And
it was invisible to CI: the quality runner stops after the first failing
lane, and `app:app` failed ahead of every backfill shard.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
1dc839743e |
census: tell the reader where a DELIBERATE-LITERAL marker has to go (#2909)
A `DELIBERATE-LITERAL` marker in the wrong **position** is indistinguishable from no marker, and the miss is silent until CI. **Measured on #2883:** the marker sat inline in the middle of a conditional expression, so it attached to the wrong AST node and three reviewed literals scored as new debt (`self-healing.ts` 86 → 89). The message the tool printed at the time said *"record why at the site with a `DELIBERATE-LITERAL` marker"* — which I had done. Nothing in the output suggested placement was the problem. Two lines added to the failure message: - Markers are read from a node's **leading** comments, so put one on the declaration and hoist the literal into a named helper if needed. - **`pnpm lint` does not run this census** — CI's Lint job does. That is why the usual "lint passed locally, push" loop cannot catch either mistake, and why the tool itself is the only place a reader sees this in time. ## Verified, not assumed I induced a real failure (a temporary `t.column === "in-review"` guard in `self-healing.ts`) and read the printed output rather than trusting that the string lands in the right branch — the message has two branches and only one is the guard-count-rose path: ``` packages/engine/src/self-healing.ts: 89 -> 90 Resolve a lifecycle column from the task's own workflow (…) correct, record why at the site with a DELIBERATE-LITERAL marker. Put the DELIBERATE-LITERAL marker in the DECLARATION's leading comments, not inline in an expression: markers are read from a node's leading comments, so a mid-expression one attaches to the wrong node and is silently ignored. Hoist the literal into a named helper if you need to. Note that `pnpm lint` does NOT run this census — run it explicitly before pushing. ``` Guidance only — no scanner behaviour changes, so no counts move. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71; `pnpm lint` and census `--strict` clean. |
||
|
gsxdsm
|
85ca9fe461 |
fix(tests): agent-detail mobile padding — jsdom cannot compute an unparsed shorthand (#2910)
## The last failure in `app:backfill 1/4` ``` AgentDetailView mobile scroll regression (FN-4231) > adds mobile row gaps to the overview hero for long health and skills metadata (FN-7958) AssertionError: expected '0' to be 'var(--space-md)' ``` **Not a style regression — the CSS is unchanged.** jsdom does not substitute `var()`, and what it does *instead* changed at the **27 → 29** bump (`4819c2634`): a directly-declared **longhand** still echoes its raw text, while a **shorthand** fails to parse and computes to the initial value. Same cause as the TaskCard failures fixed in #2782. **The asymmetry is visible three lines above the failure** — `rowGap` and `columnGap` assert the same kind of token and still pass, because they are declared as longhands. Only `padding` broke, which is why this read as a one-property regression rather than a jsdom behaviour change. ## `paddingTop` does not rescue it That was my first attempt, and it still returns `'0'` — measured, not assumed. jsdom cannot derive a longhand from a shorthand it failed to parse, so **computed style cannot answer this at all**. ## The fix Assert the **declared rule**, which is the pattern this file already uses for its desktop counterpart: ```ts expect(loadAllAppCssBaseOnly()).toContain("padding: var(--space-md) calc(...);"); ``` One difference that matters: the **full** sheet is needed rather than the base-only one. This padding is a mobile override inside `@media (max-width: 480px)` (`AgentDetailView.css:2129-2131`), and `loadAllAppCssBaseOnly` strips at-rules by design — so the obvious copy of the neighbouring assertion would have silently matched nothing. ## Evidence | | result | |---|---| | the file | **7/7** (was 1 failed) | | mutation: mobile card padding `md → xl` | **1 failed** | The mutation is the important one here: a regex that merely found the `@media` block would pass regardless. It tracks the actual declaration. `pnpm lint` clean. Test-only; `AgentDetailView.css` restored clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
2811a4a2df |
fix(tests): TaskDetailModal renders through a PORTAL — query the document, not container (#2885)
## What this clears
**30 of the 55 failures** in the dashboard `app:backfill 3/4` shard —
all in one file, all reading `expected null to be truthy`.
## The symptom points the wrong way
That message reads as *"the modal never rendered"*, and that is how this
survived. The file's helpers took the `container` returned by `render()`
and asked it for the modal's elements:
```ts
const header = container.querySelector("[data-testid='agent-log-model-header']");
```
`TaskDetailModal` mounts inside `FloatingWindow`, which uses
**`createPortal`** — so the modal subtree is attached to
`document.body`, **not** beneath the container React handed back. Every
`container.querySelector` in the file returns null no matter what
renders.
## Probed, not inferred
I had already spent one wrong hypothesis on this exact file — the shared
`TaskDetailModal.test-helpers.ts` carries a genuinely stale `{
flagEnabled: false, workflows: [] }` fixture of the kind #2833 fixed for
`App.test.tsx`, so it looked like the 30-failure lever. Adopting
`DEFAULT_BOARD_WORKFLOWS` **changed nothing** (still 30 failed).
Reverted.
So I instrumented instead:
```
P1_after_tab_click menu=true items=["Live","Feed","Raw","Interventions"]
P2_after_select viewer=true header=true empty=false
```
The Activity menu opens, `Raw` selects, and the viewer **and** its model
header are both present — via `document`. Only the container-rooted
lookup could not see them.
**Why the file half-worked:** `screen.getByRole(...)` in the same
helpers always succeeded, because `screen` queries the document. That
mix of query roots is what made a query-root bug look like a rendering
fault.
19 `container.querySelector` call sites converted.
## Scope — deliberately narrow
**Only this file.** Eight other `TaskDetailModal` specs use
`container.querySelector` too — 95 of them in `attachments-and-tabs`
alone — and they **all pass today**, because they render
`TaskDetailContent` rather than the portalled modal. Converting green
files would be churn with real risk and no red to justify it.
## Evidence
| | result |
|---|---|
| the file | **48/48** (was 30 failed) |
| shard `3/4` | **55 → 25** failures |
| mutation: rename the `agent-log-model-header` testid in
`AgentLogViewer` | **18 failed** |
The mutation matters here: the fix is "change what we query", so the
risk is assertions that now find *something* and stop being
load-bearing. They still observe the real component.
`pnpm lint` clean. Test-only; `AgentLogViewer.tsx` restored clean.
## Remaining in this shard
`settings-mobile` (17), `NodesView` (2), `MailboxModal` (2),
`agent-modals-mobile` (2), `TaskDetailModal.pr-tab` (1),
`onboarding-flow` (1). Tracked on #2784, which I have been keeping
current with per-lane numbers.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
aacd18e847 |
docs(lanes): audit the last three files the census points at with no reason attached (#2908)
Second pass of #2873's sweep, over the files that still carry lifecycle guards and **zero** audit notes. No source change — every literal stays counted, none gets an exemption marker. ## `project-store-ops.ts` (1) — dead sync path, do **not** convert The literal would leak a merge-queue entry on a renamed board: a card leaving review would never be dequeued. Except the function cannot run — it reaches for `store.db.prepare`, which throws in PostgreSQL backend mode. The live path is `dequeueMergeQueueOnColumnExitInTransaction` (`async-merge-coordination.ts`, called from `moves.ts`), and it is **already converted** — it takes `moveReviewColumns` and the caller supplies them. Recorded so the census entry is not mistaken for unconverted debt, and so it can be deleted alongside the rest of the sync SQLite residue. ## `task-id-integrity.ts` (2) — one real, one sentinel, and the real one must not go alone ```ts return cached?.column === "archived"; // ← board lane: real if (live === "archived") return true; // ← getLiveTaskColumn's manufactured value: sentinel ``` Converting the first while `getLiveTaskColumn` still keys on the literal would leave the two disagreeing about what "archived" means. It waits for that one, which is the single highest-leverage line in this cluster — fixing it makes five downstream sentinel checks correct without touching any of them. ## `auto-merge-finalization.ts` (3) — one real but diagnostic-only, two non-defects `task.column === "done"` selects which **reason string** is reported; both arms return `{ ok: false }`. So a renamed board is refused with the generic `missing-merge-confirmation` instead of the specific `done-without-merge-confirmation`. Real, and worth less than the signature change required to fix it — the resolver two functions up already computes `isCompleteColumn`, but this function does not receive it. The other two are **not** defects and it is worth saying so explicitly: the `columnId === "done"` near the top is the resolver's documented degraded fallback (the live arm calls `columnHasFlag`), and the `step.status` comparison is a **step status**, not a column. ## The pattern across both passes Of **6 files and 15 guards** audited: **2** were live defects worth converting, **4** were sentinels or dead paths that would have *broken* a renamed board if converted, and the rest were diagnostics or misfiled step statuses. That ratio is the argument for these notes existing. A file's census count is an upper bound on convertible sites, not a work estimate — and in this cluster the naive reading of the number would have made things worse more often than better. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`, `@fusion/engine`) — clean - census `--strict` — exit 0, counts unchanged (that is the point) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
27501a53da |
fix(tests): summary-tab queries container; the modal is portalled (2 → 0) (#2907)
## Fifth file, same defect `TaskDetailModal.summary-tab.test.tsx` — the last `TaskDetailModal` spec still failing on the portal/query-root defect (#2885, #2890, #2893, #2895). **Probed before converting**, as with each of the others: ``` PROBE container=false document=true ``` `TaskDetailModal` mounts through `createPortal`, so `container` is empty and its 5 lookups returned nothing. Both failures carried the signature that shape produces on a text read: ``` expected undefined to be 'Activity' ← container.querySelector(x)?.textContent ``` ## Evidence | | result | |---|---| | the file | **17/17** (was 2 failed) | | mutation: rename `.detail-tabs` in `TaskDetailModal` | **3 failed** | `pnpm lint` clean. Test-only; `TaskDetailModal.tsx` restored clean. ## Deliberately not bundled: the other three in this shard Each is a **different** cause, and lumping them in would hide that: - **`AgentDetailView.mobile-scroll`** — `expected '0' to be 'var(--space-md)'`. That is the **jsdom-29 `var()` computed-style** case, the same one fixed for TaskCard in #2782: jsdom does not substitute custom properties, and what it does *instead* changed at the 27→29 bump. Not a query root. - **`AgentListModal`** — `expected +0 to be 3`. - **`SubtaskBreakdownModal`** — an undefined-vs-string assertion mismatch. Three separate small fixes, not one sweep. Keeping them apart also keeps each mutation-check honest about what it proves. ## Portal defect, running total | PR | file | cleared | |---|---|---| | #2885 | `models-progress-workflow` | 30 | | #2890 | `settings-mobile` | 17 | | #2893 | `definition-actions` | 12 | | #2895 | `rendering` | 24 | | this | `summary-tab` | 2 | **85** backfill failures from one defect: tests querying `container` for components that render through a portal. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved coverage for task detail modal behavior, including tab ordering, chat content, merge-card containment, and summary rendering. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
60bfebdc98 |
fix(reliability): the duration query hid its lane ids inside a SQL template (#2875)
The Reliability panel's **third and last** blind input — and my own loose end. #2861 fixed the two counts beside it, so the panel went from uniformly wrong to **partially** wrong: entries and bounces populated, duration reporting `no-in-review-entries` forever. Partial blindness is harder to notice than total, which is why finishing it matters more than one site suggests. ```sql metadata->>'to' = 'in-review' OR (metadata->>'from' = 'in-review' AND metadata->>'to' = 'done') ``` ## The class, not just the site **This shape is invisible to every check we have.** The lifecycle census scans `===`/`!==` comparisons; the unwired-lane-parameter guard scans declarations. Neither sees a lane id inside a `sql` template, so this class is **not in the backlog total at all** — the number is a floor for this reason as well as the usual one. `scripts/check-sql-column-literals.mjs` (#2841, in flight) is the detector for exactly this: it freezes the surface at 30 sites rather than converting any, so this one was unowned. That PR and this one are complementary — it stops the surface growing, this shrinks it by one. ## The fix Lanes resolve **once per call** via `resolveProjectColumnsForRoles` and arrive as parameterised equality fragments, one branch per id — no interpolated list, no string building. Resolution lives in `getInReviewDurationEventsImpl` because that is where the store is; `async-audit.ts` takes a bare `db` handle and cannot resolve anything. Best-effort, defaulting to the legacy pair, so a caller that cannot resolve keeps exactly today's query. **The union is correct rather than a widening hack**, for the same reason as #2861: these are *move records*, and a past move recorded the column name as it was at the time. A board renamed last month has rows under both ids, so the honest query covers both — which is precisely what `resolveProjectColumnsForRoles` returns. ## Tested against real PostgreSQL, deliberately This is a **SQL predicate** change. A mocked store would assert the arguments and prove nothing about the query that actually runs — which is the entire risk when the literal lives inside `sql`. The new case inserts real `activity_log` rows on a renamed board and reads them back through the real store method. The legacy-lane case in the same file stays green, which is the compatibility half. **Revert proof, measured:** restore the hardcoded fragments and the new case fails with ``` expected [] to deeply equal [ 'renamed-entered', 'renamed-done' ] ``` ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - `activity-log-parity.pg.test.ts` — 5 passed against real PostgreSQL With this, all three Reliability inputs read the board's own lanes. 🤖 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** * Reliability duration metrics now work correctly with renamed workflow lanes. * Completion tracking recognizes configured completion lanes instead of relying on fixed defaults. * Improved handling of transitions between multiple review lanes and review-to-work-in-progress movements. * Legacy lane behavior remains supported when configured lane information is unavailable. * **Tests** * Added coverage for renamed lanes, historical lane IDs, and transition edge cases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
de677b231a |
fix(tests): settings-mobile queries container; SettingsModal renders through a portal (#2890)
## What this clears All **17 failures** in `settings-mobile.test.tsx` — the second-largest block in the dashboard `app:backfill 3/4` shard after the TaskDetailModal file (#2885), and the **same root cause**. ## Probed, not assumed Every failure read `expected null to be truthy`, which looks like the modal never rendered: ``` PROBE container.settings-layout=false document.settings-layout=true document.modal=true bodyLen=36637 ``` `SettingsModal` mounts through `createPortal`, so its subtree hangs off `document.body`, not the container `render()` returns. The markup is there; the container-rooted lookup cannot see it. ## Nine of these assertions could never have failed They are **absence** checks: ```ts expect(container.querySelector(".settings-scope-banner")).toBeNull(); expect(container.querySelector("#settings-mobile-section")).toBeNull(); ``` `container` is empty for this component no matter what, so these passed on an **empty root** rather than on absence — they would have kept passing if the element appeared. Converting them makes them mean what they say. All nine still pass, so they were correct, just unproven. ## Two rounds of my own errors, both caught by measuring 1. A blanket `container` → `document` replace also rewrote `renderResult.container.querySelector` into `renderResult.document...` — **not a thing**. That broke the two *"embedded Settings surface"* star tests. Because the embedded surface is genuinely not portalled, this first read as *"embedded needs container"*. It doesn't; the JS was simply invalid. 2. Fixed by restoring those seven, then converting them to **bare `document`** once I confirmed each test unmounts its surface before rendering the next (`modalRender.unmount()` precedes the embedded render), so a document-rooted query cannot match a stale instance. **Verified no regressions rather than assuming** — diffed the failing-test list before and after: 13 fixed / 0 new, then the remaining 4 fixed. Every failure in the final state was already failing at the start. ## Evidence | | result | |---|---| | the file | **41/41** (was 17 failed) | | shard `3/4` | **55 → 38** with this change alone | | mutation: rename `.settings-layout` in `SettingsModal` | **1 failed** | `pnpm lint` clean. Test-only; `SettingsModal.tsx` restored clean. ## Together with #2885 #2885 clears the TaskDetailModal file (30). Both are the same portal/query-root defect in different files, and both were sitting behind `app:app` in the runner's fail-fast — which is why they went unnoticed. Shard `3/4` should land at **~8** with both applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
d1ea33ee79 |
docs(lanes): three measured claims of mine had gone stale — date them or delete them (#2904)
Comment-only. No source change, no behaviour change. #2903 corrected a note that named a caller which had since been converted. This applies the same check to my own notes, and all three measured claims I wrote are now wrong: | claim | where | actual | |---|---|---| | "`self-healing.ts` alone issues **49** such reads" | `project-lane-vocabulary.ts` + its test | **37** | | "**51** such destinations exist in production" | `workflow-lifecycle-traits.ts` | ~34 | | "**22** deliberately pass `recoveryRehome: true`" | same | ~18 | All were accurate when measured. The shq fleet has been converting `self-healing.ts` since, and this program has been converting `moveTask` destinations all day. The repo-wide read-shaped total is now 37 *in total*, so "49 in one file" could not have remained true regardless. ## Two different repairs, because the claims differ in one way that matters **The self-healing figure has a reproduction.** `node scripts/lifecycle-column-census.mjs --json` reports `queryByFile` and `queryRoles`. So the number is kept, marked explicitly as a dated measurement, and the reader is pointed at the command rather than asked to trust the figure. **The `moveTask` counts have none.** Nothing regenerates them — the census cannot see call arguments, which is the very point the note is making. So they are **deleted** rather than refreshed, with the grep that approximates them inlined and labelled approximate. Refreshing an un-reproducible number just resets the clock on the same failure. The shape of the finding is what the note is for; the count was decoration that decays. ## Two process notes worth recording **Notes that assert facts about other files are a decay class with no detector.** The census counts literals; the unwired-lane guard counts declarations; neither reads prose. Two of these corrections in a row (#2903 and this) came from *reading a note and checking its claim*, which is not something the toolchain will ever do for us. The durable form is: cite a command, or state the shape without the number. **I nearly published a wrong replacement figure.** My first probe against the census AST returned `0` because I wired `summarize()` incorrectly — the third time this session a probe has been wrong before the product was. That is why the corrected note cites `--json` output rather than another hand count. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - `project-lane-vocabulary.test.ts` — 9 passed - census `--strict` — exit 0, counts unchanged 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
8e505dc4e1 |
docs(recovery): the reason this parameter is optional stopped being true (#2903)
Comment-only. No source change, no behaviour change.
The note on `isInReviewMissingWorktreeSessionStartFailure` said:
> Optional rather than required because the other caller
(`extension.ts`) still asks BOTH questions with the literal.
**It doesn't.** All three production callers pass the resolved answer:
```
packages/cli/src/extension.ts:1927 retryReviewColumns.has(task.column)
packages/cli/src/commands/task.ts:1390 retryReviewColumns.has(task.column)
packages/dashboard/src/routes/register-task-workflow-routes.ts:2885 retryReviewColumns.has(task.column)
```
Left standing, that sentence tells the next reader an unconverted caller
exists — and "we keep the fallback because someone still needs it" is
exactly the justification that keeps an inert-conversion shape alive. It
is the specific failure this program has spent the day removing, in the
form of a comment rather than code.
## The parameter stays optional, for a reason that does not rot
I checked whether to make it **required** — the unwired-lane-parameter
guard's own failure message suggests exactly that ("make the parameter
required so the compiler finds the call sites") — and decided against
it, for measured reasons:
- **25 test call sites** use the optional form, several of them
*precisely* to pin the degraded mode (`cli-active-count-lanes.test.ts`
exercises the no-argument path on both a legacy and a renamed lane).
Requiring the parameter deletes that coverage.
- The enforcement it would buy already exists: `isReviewColumn` is in
the guard's vocabulary, so if any of those three callers stops passing
it, the build fails.
So the note now gives the durable reason instead of the expired one.
## How this surfaced
Not from the census — the count here is unchanged, and an *omitted
argument* is invisible to it anyway. It came from reading an audit note
that named a specific caller and checking the claim. Notes that assert
facts about other files decay silently; this one had.
## Verification
- `pnpm lint` — clean
- `tsc --noEmit` (`@fusion/engine`) — clean
- `restart-recovery-coordinator.test.ts` — 12 passed
- `cli-active-count-lanes.test.ts` — 10 passed
- unwired-lane guard — 9/9, no new entries
- SQL-literal gate — green
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
10f9df1600 |
fix(overseer): the whole oversight loop was inert on a renamed board (#2898)
`resolveWatchedStage` keyed on the literals `in-progress`/`in-review`, so on a board that renames either it returned `null` for **every** card. That is three literals with an outsized blast radius. `observeTask` returns early on a null stage, so: - no `OverseerStageObservation` is recorded, - no `overseer:intervention` entry is emitted, - and `PlannerRecoveryController`, which consumes those observations, has nothing to steer, retry or targeted-fix. **The entire oversight loop was inert and silent about it** — the same shape as the self-healing sweeps whose queries returned empty arrays. ## I deferred this myself, on a cost argument that was wrong The audit note I wrote for this site said resolving inside `observeTask` "buys a workflow read per card per poll". Then I read the caller: the poll **already awaits `resolveEffectiveSettings` per task**. It is a per-task async loop regardless, so with an IR cache keyed by workflow the addition is *(distinct workflows)* resolutions, not *(cards)*. Pricing the fix before checking the caller cost a deferral. Worth recording, because "this needs a cost judgement" is the most comfortable place in this program to leave something. ## The review test is the three-trait union, deliberately `isReviewColumnRole` checks only `mergeBlocker || humanReview`. A board whose review lane carries `merge` (**mergeOrchestration**) — the built-in default's own shape — would classify as *not in review* and be skipped. Reaching for the obvious helper would have reintroduced the bug this change removes, through the helper meant to fix it. There is a case asserting exactly that. ## Wiring Both call sites, because either alone leaves a hole: | site | why it matters | |---|---| | the poll (`project-engine.ts`) | per-poll IR cache — a workflow edit is picked up next tick rather than served stale | | the manual nudge | otherwise a renamed board answers `no-active-stage` to an operator pressing the button | `columnFlags` is in the `unwired-lane-parameter` vocabulary, so the wiring cannot silently rot — the guard reports it if a future change drops the argument. Fail-soft throughout: an unresolvable workflow yields `undefined` and the callee falls back to the legacy ids, which is exactly today's behaviour. A v1 IR declares no columns, so it takes the same path. ## Revert proof (measured) Drop the `columnFlags` branch and **exactly the three renamed-lane cases fail**: ``` expected null to be "executor" expected null to be "merger" (mergeOrchestration lane) expected null to be "merger" (humanReview lane) ``` The legacy-id and neither-role cases stay green — the gate must still gate, and watching every column would be its own defect. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/engine`) — clean - `planner-overseer.test.ts` + `planner-recovery-controller-human-control.test.ts` — 64 passed - unwired-lane guard — 9/9, no new entries Carries the one-line SQL-baseline re-record (`team-analytics.ts: 6 → 3`) that #2864 left behind, same as my other open branches — main is red on it, and identical changes to that line merge without conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
defe48d30f |
fix(core): per-workflow metrics read zero on a renamed board (#2866)
Second of the 14 lane-bound SQL sites from #2839, after #2864. Independent of it — different file, different caller argument. ## The defect `aggregateWorkflowAnalytics` filtered in SQL on `t."column" = 'done'` and `IN ('in-progress','in-review')`. On a renamed board those match nothing, so `tasksCompleted`, `tasksInProgress` and `tasksInReview` come back **zero for every workflow** while the board is busy. Nothing errors. Same shape and same fix as #2864: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, and thread the store from the single Command Center caller so the parameter has a supplier immediately rather than becoming an inert seam. ## What the test caught that I had not **The renamed case still failed with the query fixed.** The bucketing at lines 296–297 already uses `isWipColumnRole` / `isReviewColumnRole` — correctly converted — but those read `query.columnFlagsByName`, which production supplies and my fixture did not. So: - the **SQL** decides *which rows come back*; - the **trait map** decides *which bucket each row lands in*. Both halves have to be right. Fixing only the query would have shipped a "conversion" that still reported zero on a renamed board, and the file would have scored as converted twice over. That is exactly the partial-conversion shape this program keeps re-finding — caught here only because the test asserts `tasksInReview` alongside `tasksCompleted`, since those two paths take **different** resolved sets (complete vs wip+human-review). Asserting the completed count alone would have left the second conversion unproven. ## Measured Reverted, only the renamed case flips: ``` ✓ default vocabulary: completed and in-review work are counted × renamed vocabulary: completed and in-review work are counted ✓ renamed vocabulary: a card in the HOLD lane counts as neither ✓ without a lane store, the legacy ids still answer Tests 1 failed | 3 passed (4) ``` The hold-lane negative is there so resolving real lanes cannot degrade into "every column counts" — trading an undercount for an overcount is harder to notice than the original bug. ## Scope The sync SQLite arm in the same file keeps its literals: it throws in backend mode and has no production caller, the same dead-arm conclusion reached for `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · both Command Center analytics suites 8/8 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
c143327d4b |
fix(core): the archived-document guards failed in OPPOSITE directions on a renamed lane (#2886)
Two of the four convertible sites my own learnings doc **miscounted as sentinels** — the #2877 review corrected "8 of 9 must not be converted" to "5 of 9", and these are two of the three that correction freed. They read `task.column` straight off a row `select`, so they are board lanes by exactly the test that document gives, and a renamed archived column is simply not seen. What makes the pair worth fixing together is that they fail in **opposite directions**: | guard | on a renamed archived lane | consequence | |---|---|---| | `upsertTaskDocument` | fails to **reject** | an archived card's documents stay **writable** — the read-only contract silently does not hold | | `publishArchivedTaskDocumentAddition` | fails to **accept** | a legitimate archived-document publication is refused as `parent-not-archived` | The second is the sharper one: valid operator work refused, and refused with a message that reads as a data-integrity error rather than a lifecycle mismatch. ## Shape Both take an `AsyncDataLayer` and can resolve nothing themselves; their store-level impls hold the store, so the lane set arrives as a parameter resolved once per call — the shape #2875 used for the SQL predicate. **One shared `resolveArchivedLanes` for both paths**, deliberately: if the write guard and the publication guard could disagree about whether a card is archived, a card ends up both read-only *and* un-publishable. ## The revert proof caught my own fixture first My first version set `deletedAt` alongside the renamed column, and **the revert proof passed with the fix removed**. Both guards are `column-is-archived || deletedAt != null`, so a soft-deleted fixture short-circuits the exact comparison under test — the assertion was holding for an unrelated reason. Dropping `deletedAt` isolates it, and is also the *real* shape: a live row in a workflow-declared archived lane is what a renamed board produces, and what `getLiveTaskColumn` was written to catch. Revert proof, measured honestly the second time: restore `task.column === "archived"` and the renamed-lane case fails — the upsert resolves instead of rejecting. ## Real PostgreSQL, deliberately These are row predicates inside a transaction. A mocked store would assert the arguments and prove nothing about the comparison that runs — the same reasoning as #2875. Three cases: the renamed lane rejects, the **legacy** `archived` id still rejects (most boards never rename anything), and a live card is still allowed through (a guard that rejects everything is its own bug). ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`) — clean - new `archived-document-lanes.pg.test.ts` + existing `artifacts-documents-evals.pg.test.ts` — 12 passed against real PostgreSQL Note: the SQL-literal baseline is untouched here — #2881 owns re-recording it after #2864's conversion left main's gate red. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
63e1f81244 |
fix(gate): a dropped SQL-literal count tightens the baseline instead of failing the gate (#2888)
## Why `check-sql-column-literals` runs inside `pnpm test:gate` — the **blocking** lane. It hard-fails when a count *drops*, so a single converting PR that doesn't re-record takes down the gate for **every worker in the program** until someone fixes the baseline by hand. That is not hypothetical. It is happening on `main` right now (`team-analytics.ts` 6 → 3, fixed by #2880), and it is the **second** instance of the shape — the lifecycle census hit it from a merge wave that dropped eleven files at once. ## The census already resolved this exact trade-off From `docs/testing.md`, on why the census stopped hard-failing on a drop: > "the drop is almost never the failing author's to fix ... A permanently-red gate is a bigger hole than a stale allowance, because it gets ignored and then nothing is guarded at all." That reasoning applies here **with more force**, because the census is *not* in the blocking lane and this check *is*. Same failure mode, higher cost, opposite policy — this aligns them. ## What changes A drop now rewrites the baseline downward, reports what it lowered, and exits 0: ``` [check-sql-column-literals] baseline TIGHTENED — fewer literals than it allowed packages/core/src/team-analytics.ts: allowed 6, now 3 The baseline has been rewritten downward. COMMIT IT so the allowance cannot be regrown into; in CI this write is discarded with the runner, which is why the gate is green and not silent. ``` **The rise check is untouched.** "No new SQL column literals" is the ratchet's actual purpose and still fails hard. The stale-allowance concern the old comment raised is real and is preserved: the rewritten file must be committed, and in CI the write is discarded with the runner — so the gate goes green rather than silently passing a stale allowance, exactly as the census does. ## Verified in both directions | scenario | result | |---|---| | drop (`team-analytics.ts` 6 → 3, the live case) | **tightens, exit 0** | | rise (a literal added to a zero-allowance file) | **fails, exit 1** — `task-age-staleness.ts: 1 SQL column literal(s), baseline allows 0` | The rise probe needed a zero-allowance file: adding one literal to `team-analytics.ts` keeps it at 4 against an allowance of 6, which is correctly *not* a rise. Worth noting because it is an easy way to conclude the guard is dead when it is working. ## Relationship to #2880 #2880 fixes the **instance** — it re-records the current drift so the gate goes green now. This fixes the **class**, so the next conversion doesn't take the gate down again. They are independent and either can land first; if #2880 lands first, this becomes a no-op on a matching baseline. ## Verification - `pnpm test:gate` — exit 0 with this change applied - `pnpm lint` — clean No changeset: gate tooling, not published behaviour. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
9366bc8382 |
fix(workflow): the review handoff killed the walk on a renamed review lane (#2900)
The sharpest lane defect left in the backlog, and the one I have been
deferring since the first sweep.
```ts
if (seam === "review-handoff") {
const result = await primitives.transitionTask(primitiveCtx, context.task, {
column: "in-review", // ← post-U12 this is a rejected destination on a renamed board
```
Post-U12 `moveTask` **rejects** a destination the workflow does not
declare. So on any board with a renamed review lane, the handoff threw
`TransitionRejectionError` and **killed the workflow walk mid-run**. Not
a silent wrong answer for once — a hard failure in the middle of a task,
which is why it outranked everything else once it became reachable.
**Why it was deferred:** every fix threads a resolver out of
`executor.ts`, and #2820 was editing that file. It merged at 22:08, so
this was finally free of the conflict.
## The role travels, not the column
Seam handlers in `workflow-node-handlers.ts` are pure functions over an
IR node and a task — no store, no task id to resolve from — so a handler
can only ever name a literal. The runtime primitive in `executor.ts`
**does** hold the store, so the seam now asks for `columnRole: "review"`
and the primitive resolves it against the task's **own** selection.
One authority, deliberately. Answering one question with two reads is
what took #2843 five review rounds, and I would rather not relearn it
here.
Compatibility is preserved in both directions:
- `column` still wins when both are supplied — an explicit destination
is an explicit destination;
- an unresolvable role falls back to the legacy `in-review` rather than
failing the transition, which is exactly the behaviour every caller had
before.
## The test asserts the literal is *gone*, not merely accompanied
`column` takes precedence over `columnRole` downstream, so a diff that
added the role while leaving the literal would look converted and be
completely inert. That is the exact shape this program keeps finding — a
documented fallback in front of a literal that still decides everything
— so the assertion is:
```ts
expect(input.columnRole).toBe("review");
expect(input.column).toBeUndefined(); // ← the half that matters
```
**Revert proof, measured:** restore `column: "in-review"` in the seam
and it fails with `expected undefined to be 'review'`.
## Verification
- `pnpm test:gate` — 161 / 487 / 13 / 71 passed
- `pnpm lint` — clean
- `tsc --noEmit` (`@fusion/engine`) — clean
- new `review-handoff-lane.test.ts` plus the two neighbouring seam
suites — 41 passed
Carries the one-line SQL-baseline re-record (`team-analytics.ts: 6 → 3`)
that #2864 left behind, same as my other open branches — main is red on
it, and identical changes to that line merge without conflict.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
gsxdsm
|
a453912ddf |
self-healing: merged-but-unfinished tasks never finalized on a renamed board (fifteenth sweep) (#2897)
`recoverMergedReviewTasks` finalizes a task whose merge is **confirmed** but which never reached the complete lane. Two literal reads meant that on a renamed board it was never found, so a card whose commit is already on the base branch sat in review or hold indefinitely — merged work the board still shows as unfinished. ## The two redundant guards convert, they don't get deleted Both `t.column === …` checks were redundant while the query pinned the column. Under a resolved read they become the per-card verdict. Deleting them would have silently widened the sweep — the same trap called out in #2891. ## Carries the two shapes review established earlier in this series - **Narrow when the card can answer, broad when it cannot** (#2891). `resolveWorkflowIrForTask` *substitutes* the built-in IR rather than failing, so a card with an unreadable selection would otherwise be rejected by the very verdict that the project-scoped query had just admitted it under. It falls back to the project sets instead. - **Deduped across the buckets** (#2879), so a column carrying both a review role and the hold role cannot finalize one card twice. Both were review findings on earlier PRs in this series, applied here up front rather than waiting to be caught again. ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed | | the per-card review verdict | fails — the renamed review lane does not match | Observable is `resolveSelfHealingMergeTarget`, a private method called once per candidate, so the assertion sits downstream of both halves without a git fixture. A non-vacuous companion (merge-confirmed card in the wip lane → untouched) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly. |
||
|
gsxdsm
|
32617b81bb |
fix(gate): re-record the SQL column-literal baseline — main's MERGE GATE is red (#2884)
## Main's merge gate is red ``` [check-sql-column-literals] SQL column-literal population changed: packages/core/src/team-analytics.ts: 3 site(s) now, baseline still allows 6 — re-record it (--update-baseline) ``` The count went **down**: three raw column literals inside query strings were resolved away, which is exactly the direction this gate exists to encourage. The baseline was not re-recorded in the same commit, which the check's own message asks for. **This one is in the merge gate.** Unlike the census ratchet — which auto-tightens and exits 0 — `check-sql-column-literals` exits **1** on a drop (measured), so it blocks every PR in the queue rather than reddening a non-blocking suite. ## The fix `--update-baseline`: 28 sites in 14 files, one entry changed. ```diff - "packages/core/src/team-analytics.ts": 6, + "packages/core/src/team-analytics.ts": 3, ``` Verified: the check exits **0** afterwards, and `pnpm test:gate` is **732 green**. ## Follow-up worth considering — deliberately not done here This is the **fourth time today** a derived baseline going *down* has reddened something: | baseline | effect of a drop | |---|---| | lifecycle census | reddened a non-blocking guard test — fixed in **#2856** | | SQL column literals | **blocks the merge gate** — this PR, one instance | #2856 fixed the shape for the census: a tightening now reports healthy instead of failing, so "somebody improved the tree and hasn't re-recorded yet" stops being an emergency. This gate has the same shape with **higher stakes** — a conversion PR that improves the tree stops the whole queue until a human notices and re-records. The census CLI already models the better behaviour: auto-tighten, exit 0, print *"COMMIT IT"*. Porting that here is a small change, but it alters **merge-gate semantics**, so it deserves its own review rather than riding along in a red-clearing commit. Flagging rather than doing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
b3b377d367 |
docs(workflow-learnings): two lane-literal classes no tool of ours can see (#2877)
Docs only. Two findings from this unit that cost real time to derive and would otherwise be re-derived by whoever reaches these files next. ## 1. `=== "archived"` is usually a SENTINEL `packages/core/src/task-store/async-comments-attachments.ts` carries **9** census guards — the second-largest single-file count outside `self-healing.ts`. Reading all nine: **exactly one** is a board-column comparison. The other eight compare against a value `getLiveTaskColumn` *manufactures*: ```ts if (row.column === "archived" || row.deletedAt != null) return "archived"; // ← fabricated return row.column; ``` Converting those eight to `isArchivedColumnRole` would keep passing on the built-in board and start **failing** on a renamed one — a soft-deleted parent's documents would become readable. **The conversion makes the renamed board worse**, which is the opposite of what the census count implies. The rule that separates them: look at where the compared value *came from*, not at its type. From `task.column` or a DB field → a board lane. From a function that *returns* `"archived"` as a documented outcome → a sentinel. Consequence worth stating plainly: **a file's census count is an upper bound on convertible sites, not a work estimate.** ## 2. Lane literals inside raw `sql` are in no total at all The Reliability panel had three inputs. Two were call arguments and converted routinely (#2861). The third encoded its lanes in a `sql` fragment: ```sql metadata->>'to' = 'in-review' OR (metadata->>'from' = 'in-review' AND metadata->>'to' = 'done') ``` The census scans `===`/`!==` comparisons; the unwired-lane-parameter guard scans declarations. **Neither can see a string inside a `sql` template**, so this class is not in the backlog number — a second, independent reason the total is a floor. Second known instance after the archived gate in PR #2724, which makes it a pattern rather than an accident. Fixed in #2875, and the doc says so rather than leaving it described as outstanding — a learnings doc that reports a fixed defect as open sends the next reader to a dead end. `scripts/check-sql-column-literals.mjs` (#2841) is the detector for the class and freezes the surface at 30 sites; the two are complementary. ## 3. Sibling files The GitLab importer's `column: "triage"` was fixed in #2843. The Linear importer — written from the same template, with **two tests pinning the bug** — still had it, and was found only by re-grepping an area I had already declared clean (#2860). When a defect is found in a file that has a sibling, the sibling is the next place to look, and no tool will tell you that. ## Verification `pnpm lint` clean. No source change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
cfb713bda1 |
notification: record the measured reason four wedge-progress ids stay literal, and un-red main's gate (#2882)
Two small things, neither of which changes behaviour. ## 1. A conversion I attempted, measured, and reverted `hasProgressed` in the wedge-episode path names four column ids outright. I converted them to a resolved lane set. It **broke an existing gate test** — `task-wedge-notification.test.ts` → *"sends one actionable push and mailbox message per active terminal episode"*: 1 message delivered, 2 expected. The note already in that file was right, and stronger than it read. The hazard is **not** specific to the resolve/claim ordering — it is **any `await` added before the resolve**. Column resolution needs one. `task:updated` listeners fire synchronously, so a re-wedge arriving close behind a recovery reaches `claim` while the first episode is still open, and the operator's second alert is dropped. Product change reverted; only the comment lands, now carrying the measurement and naming the failing test as the acceptance check for whoever owns the wedge-episode contract. **Left counted, not exempted** — the census should keep pointing here. Worth stating: the pre-existing note was a warning written speculatively. Attempting the conversion is what turned it into evidence, and the evidence says the blocker is real but sits somewhere else (per-task serialisation) than the note implied. ## 2. `main`'s gate is red, and not from this branch `pnpm test:gate` fails on a clean `origin/main` tree at `check-sql-column-literals`: ``` packages/core/src/team-analytics.ts: 3 site(s) now, baseline still allows 6 — re-record it ``` A reduction landed without re-recording the baseline in the same commit, which that check explicitly asks for. Reproduced on `origin/main` with my changes stashed, so it is not mine — but it blocks **every** open PR until recorded. Ratchets **31 → 28** sites across 14 files, downward only. ## The vacuous assertion this round (sixth) The first version of the reverted test passed **with the fix reverted**. `hasProgressed` is a three-clause OR, and the middle clause — *status is a string and is not `failed`* — is true for a recovered task on any board, so `status: "in-progress"` in the fixture satisfied it regardless of column. Same shape as the other five: something the code does anyway. Found by running the revert, not by reading it. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71 (green only with the baseline commit); `tsc` engine clean; notification suite 77 passed; `pnpm lint` and census `--strict` clean. |
||
|
gsxdsm
|
890e1f87e7 |
fix(core): issue panels reported nothing fixed on a renamed board (#2871)
Fourth and last of the lane-bound analytics sites from #2839, after #2864, #2866 and #2870. ## The defect `aggregateGithubIssueAnalytics` and its GitLab twin filtered their resolved-issue query on `"column" = 'done'`. On a renamed board that matches nothing, so `fixed` is **zero**, the resolved-issue list is empty, and `net` reports every filed issue as still outstanding — while the team closes issues all week. Nothing errors. Same fix as the previous three: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, thread the store from each Command Center caller so the parameters have suppliers immediately. ## Both providers in one change, deliberately These two files are **copies** — same query, only the provider literal differs — and a copy is exactly what gets half-fixed. Converting one and not the other type-checks, passes that provider's test, and leaves the second silently broken with no signal anywhere. The suite runs every case against both, so the pair cannot drift. ## Measured Reverted, exactly the two renamed cases fail — **one per provider** — while both default-vocabulary controls, both WIP-lane negatives, and both omitted-store legacy cases stay green: ``` ✓ github: default vocabulary counts a resolved issue × github: renamed vocabulary counts a resolved issue ✓ github: renamed vocabulary does NOT count an issue still in the WIP lane ✓ github: without a lane store, the legacy id still answers ✓ gitlab: default vocabulary counts a resolved issue × gitlab: renamed vocabulary counts a resolved issue ✓ gitlab: renamed vocabulary does NOT count an issue still in the WIP lane ✓ gitlab: without a lane store, the legacy id still answers Tests 2 failed | 6 passed (8) ``` That the failures are symmetric is itself the check on the copy-paste risk. ## Scope The sync SQLite arms keep their literals: they throw in backend mode and have no production caller, the same dead-arm conclusion as `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · Command Center + GitLab issue analytics suites 10/10 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. --- **This closes the lane-bound half of #2839.** All 14 sites the hand-review identified as genuinely vocabulary-bound are now converted across four PRs. What remains there is the 11 `!= 'archived'` exclusions, which are probably correct as literals — archiving writes `task.column = 'archived'` unconditionally as a state rather than a lane — plus one dead SQLite arm. Those need per-site judgment, not conversion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
216632bd3a |
fix(core): task-duration stats were computed from an empty set on a renamed board (#2870)
Third of the 14 lane-bound SQL sites from #2839, after #2864 and #2866. Independent of both. ## The defect `aggregateProductivityAnalytics` filtered its duration query on `"column" = 'done'`. On a renamed board that matches nothing, so the entire task-duration distribution — median, p90, average, total — is computed from an **empty row set** and reports zeros while the project ships work. Nothing errors. Same shape and fix as the previous two: resolve per **project** via `resolveProjectColumnsForRoles`, bind an `IN` list, thread the store from the single Command Center caller so the parameter has a supplier immediately rather than becoming an inert seam. ## Measured Reverted, only the renamed case flips: ``` ✓ default vocabulary: a finished task contributes to the duration stats × renamed vocabulary: a task in the RENAMED complete lane contributes ✓ renamed vocabulary: a task still in the WIP lane does NOT contribute ✓ without a lane store, the legacy id still answers Tests 1 failed | 3 passed (4) ``` ## The negative asserts the median, not just the count This fix's failure mode is **worse than the bug it fixes**. Resolving too many lanes would pull unfinished work into the distribution and produce a plausible-but-wrong median — a number nobody questions — where the bug produces an obvious zero. So the WIP-lane case asserts `medianMs` is null as well as `completedTasks` being 0. ## A fixture error worth naming My first version asserted `taskDuration.count`. `TaskDurationSummary` exposes `completedTasks`. Every case failed with `expected undefined to be 1` — **including the controls** — which reads exactly like a broken product until you notice the control is failing too. A control that fails is a fixture bug, not a finding; that asymmetry is the fastest way to tell them apart. ## Scope The sync SQLite arm keeps its literal: it throws in backend mode and has no production caller, the same dead-arm conclusion as `cleanupStaleMergeQueueRowsImpl` on #2839. ## Verification `pnpm test:gate` green · both Command Center analytics suites 8/8 · `tsc` core 0, dashboard 0 · lint 0 · changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
995b52d21d |
fix(gate): re-record the SQL baseline — main is red after #2864 (#2878)
**`pnpm test:gate` and both `pretest` hooks fail on `main` right now.** Merge this first. ## What happened #2841 (the SQL gate) merged, then #2864 merged. #2864 removed three legacy comparisons from `team-analytics.ts`, but its baseline entry still allows six — and this gate **fails on a lowered count by design**, so a migrated slot cannot be silently regrown into later. Baseline 30 → 28. ## This is my sequencing error The four analytics conversions were branched and reviewed **before** the gate existed, so none of them carries a baseline update. The gate then landed first, which means **each of them breaks `main` as it merges**. I opened all five without thinking about the order they would land in. The three still open — #2866, #2870, #2871 — will each do this again. I am adding baseline updates to them next so they land clean. ## Note on the downward check The "count went down" failure looks like pedantry until it fires. It exists so a migrated site cannot leave an unused allowance behind for the surface to regrow into — the same rot as an allow-list entry for a deleted function. The real cost is that a conversion and its gate have to land in a known order, which is a coupling I created and did not plan for. ## Verification `pnpm test:gate` green with the re-recorded baseline · lint 0 · `node scripts/check-sql-column-literals.mjs` exit 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
ab15e5f9f7 |
docs(lanes): audit the three files the census points at with no reason attached (#2873)
**No source change.** Every literal stays counted and none gets an exemption marker. What changes is that the census now points at these three with the analysis attached, instead of making each worker who reaches them re-derive it. Peers have already done this well for `notification-service.ts` — converted it, *measured* a real delivery regression, reverted, and left it counted with the reason. These three had nothing at all, and one of them is the highest-impact unowned site I found. ## `planner-overseer.ts` (3 guards) — REAL, and larger than three literals suggest On a renamed board `resolveWatchedStage` returns `null` for every card. `observeTask` returns early on a null stage, so **no observation is recorded**, no `overseer:intervention` entry is emitted, and `PlannerRecoveryController` — which consumes those observations — has nothing to steer, retry, or targeted-fix. **The entire oversight loop is inert and silent about it**, exactly like the self-healing sweeps whose queries returned empty arrays. Not mechanical, which is why it is flagged rather than converted. `resolveWatchedStage` is a pure sync function over a `Partial<OverseerTaskRef>` with no store and no task id, so the lane answer has to arrive as a parameter. Its only production caller, `observeTask`, *is* async and the monitor *does* hold a store — but it runs **once per task per poll**, so resolving inside it buys a workflow read per card on a timer. The shape that works is the one the board-load enrichment landed on in #2845: resolve at the **poll**, once, with an IR cache keyed by workflow, and pass the flags down. That makes it a change to `project-engine.ts`'s poll as much as to this file — a cost judgement about a periodic engine loop, not a rename. `columnFlags` is in the unwired-lane-parameter vocabulary, so whoever adds the parameter cannot leave it unwired. ## `async-mission-store-queries.ts` (1 of 3) — REAL `getTerminalTaskEvidence` tests only `column === "done"` for its `done` verdict, so a completed card on a renamed board falls through every branch to `{ kind: "nonterminal" }`. The caller is mission **terminal evidence repair**, so a finished feature reads as unfinished — a wrong *verdict*, not an error. The `archived` test beside it has the same defect, masked for soft-deleted rows by its `deletedAt` companion, which is why only the `done` half bites in practice. Takes a bare `QueryHandle`: no store, no task object, no workflow. The fix is a resolved terminal-lane set threaded in by the caller — the same shape `getLiveTaskColumn` needs, and it should land *with* it so the two cannot disagree about what "finished" means. ## `audit-ops.ts` (2) — one sentinel, one real, and they look identical ```ts if (state === "archived") // ← getLiveTaskColumn's MANUFACTURED value: do NOT convert if (pgRow.column === "archived") // ← a real board lane: convertible ``` The first compares against a string `getLiveTaskColumn` *fabricates* for an archived-or-soft-deleted parent, so converting it to `isArchivedColumnRole` would keep passing on the built-in board and start **failing** on a renamed one — a soft-deleted task's log would become writable. The second reads the task row, so a renamed archived column keeps accepting log writes; its `deletedAt` companion masks that in practice. Two lines that look the same and need opposite treatment is precisely the reason these notes are worth more than the count. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/core`, `@fusion/engine`) — clean - `planner-overseer.test.ts` — 50 passed - census `--strict` — exit 0, **counts unchanged** (that is the point) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
gsxdsm
|
ee8ae1eb23 |
fix(census): the header claimed 0 trait-fallback branches while sites of that shape existed (#2874)
The census header has been printing `of the column guards, 0 are trait-fallback branches (already converted)` while sites of exactly that shape exist. I flagged this on #2842 as a suspected classifier gap; this confirms and fixes it. ## The miss Only `cond ? trait : literal` was recognised. The other spelling — a **negative** test with the literal on the **true** branch — is what a caller writes once it hoists its resolved lanes: ```ts complete: completeLanes === undefined ? columnId === "done" : completeLanes.includes(columnId) ``` That is `github-tracking-state.ts:245-246` — a fully converted resolver whose two degraded arms were reported as unconverted debt. **The backlog read higher than the remaining work**, and a reader chasing it was sent to lines that are already correct. Second half of the miss: `completeLanes` matches no hint. Adding `Lanes` to the hint list does **not** work, and the reason is itself a prior fix — hints are word-bounded because the unbounded form once let `hold` match `threshold` and `household`. `\bLanes\b` cannot match inside `completeLanes`, where the boundary does not exist. So resolved-lane identifiers get an explicit suffix rule. ## Both guards on the new rule exist because I broke them while writing it Worth stating, because each failure ran in the **dangerous direction** — marking a *live* line "already converted", which removes a real guard from a backlog people trust: | mistake | what it excused | |---|---| | widened the shared `testsTraitData` | fed the ancestor-walking rules too, which marked `step.status === "done" \|\| step.status === "in-progress"` at `register-task-workflow-routes.ts:941` — a step-**status** comparison, not a column guard — as converted | | let the new rule walk ancestors | excused any literal inside a block governed by a negative lane test | Measured: the count went to **6 with two of them wrong** before I caught it. The rule is now immediate-parent-only with its widened identifier match local to it, and reports exactly the **2 real sites**. ## Verification - Census: **176 guards, 2 trait-fallback** (was 176 / 0). The total is unchanged — this sub-count is diagnostic and does not move the ratchet, so `--strict` exits 0 with no baseline re-record. - 5 cases in `scripts/__tests__/lifecycle-census-inverted-fallback.test.mjs`, including both negatives that pin the mistakes above plus one for the suffix rule not over-reaching (`airplanes` is not a lane test). - `pnpm lint` clean; gate green (161/487/13/71). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved lifecycle analysis accuracy for trait fallback logic, including inverted conditions, legacy fallback syntax, and null or undefined checks. * Added safeguards to avoid misclassifying complex conditions, unrelated identifiers, and nested expressions. * Improved handling of lifecycle lane and column naming patterns. * **Tests** * Expanded coverage for valid and invalid fallback scenarios, identifier boundaries, parent-expression restrictions, and property-path checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |