Skip to content

test: repair dead test wiring, normalize placement, cover high-risk modules - #879

Merged
Neonforge98 merged 12 commits into
developfrom
test/suite-hygiene-and-coverage
Aug 22, 2026
Merged

test: repair dead test wiring, normalize placement, cover high-risk modules#879
Neonforge98 merged 12 commits into
developfrom
test/suite-hygiene-and-coverage

Conversation

@Harry19081

@Harry19081 Harry19081 commented Aug 22, 2026

Copy link
Copy Markdown
Member

Summary

Repairs the test suite's wiring, makes test placement consistent, covers the worst-covered high-risk modules, then mutation-tests that new coverage and fixes what it exposed.

The thread connecting all of it: several suites reported green while running nothing, and nothing prevented that from recurring.

  • cargo:test never reached the workspace — ~5,700 Rust tests had never run
  • 5 cargo:test:* scripts filtered on modules that don't exist, exiting 0
  • CI's rust job was clippy-only, so none of it was caught
  • The one test that did fail once reached had been silently broken for a long time
  • Four boundary modules sat at 0–4% coverage, including deletion logic and git remote ops
  • The new tests were then audited by mutation testing, which found three product bugs the tests had been certifying as correct

Problem

1. Dead test configuration. cargo test --lib <filter> prints test result: ok. 0 passed and exits 0 when the filter matches nothing — verified on a throwaway crate. Five scripts were in that state. Separately, src-tauri/Cargo.toml is a workspace with "." plus 44 members and no default-members, so a bare cargo test runs the root package only.

2. No standard for test placement. 642 tests colocated, 506 in __tests__/, 53 directories using both, nothing documenting either.

3. Doc rot pointing agents at nothing. CLAUDE.md/AGENTS.md cited .cursor/rules/ui-feature-workflow.mdc twice as the authority on unit-test gates — a file never present in this repo's history. Plus two ~/.orgii/skills/… paths resolving nowhere, and 5 of 7 Rust test paths in TEST_CASES.md stale.

4. Untested high-risk code. cleanup.ts (deletion logic reached from src/index.tsx) and remoteOps.ts (push/fetch against a real repo) at 0% and 0.43%. The SessionCore/sync adapters — where external CLI/IDE output becomes domain state, exactly where CLAUDE.md's root-cause-first doctrine says invariants belong — at 0–28%.

5. Coverage that doesn't mean coverage. All 16 targeted files hit exactly 100%/100%. That uniformity is a red flag, so the new tests were mutation-tested: 103 probes, 78 killed (76%). The survivors clustered, and two clusters were serious.

Solution

Eleven commits, each independently reviewable.

Commit What
fix(tooling) cargo:test--workspace; agent_core repaired to -p agent_core (a crate, not a deleted module — 3170 tests); 4 absent scripts removed; cargo test added to CI
refactor(test) 83 files moved, 105 imports rewritten; 53 mixed dirs → 0; convention documented in CONTRIBUTING.md
test(pm) Rollback invariant re-armed (below)
docs Dead rule/skill references removed, replaced with a live pointer; stale Rust test paths fixed; undocumented E2E_OPENAI_MODEL gate recorded
test no-restricted-syntax guard: .only is now a lint error
test(session-core) · test(database-core) · test(git,storage) +591 tests; the four boundary areas 0–28% → 100%
test (audit) Assertion gaps closed in remoteOps and DatabaseCore
test(session-core) (audit) Tests that could not fail, replaced
fix(session-core) Three product bugs fixed at the producing boundary

The rollback bug. invoke_rolls_back_the_whole_graph_when_a_node_collides seeded a cross-org short_id and expected ALREADY_EXISTS. It stopped colliding: the allocator now deliberately steps over same-prefix ids across orgs. In fact ALREADY_EXISTS is unreachable from invoke by any seeded row — the allocator floors the counter and loops past taken ids, and the guard checks that same predicate. Now split in two: one pins the step-over contract, the other drives rollback from a BEFORE INSERT trigger with a count-based predicate so renumbering can't disarm it again. Post-conditions 4 → 7, compared against a post-apply() baseline. Product code unchanged; rollback verified by breaking it with DropBehavior::Commit.

What mutation testing found

remoteOps.ts — destructive flags entirely unasserted on the credential-retry path. Four mutations left all 69 tests green, including force: params.forceforce: true. A plain push that hit an auth failure, picked up a stored credential and retried could have been silently converted into a force-push with credentials attached. Cause: the file's one strict toEqual used a fixture already passing force: true, so hardcoding was invisible; the rest used toMatchObject, which cannot detect an added flag. Fixed, all four now fail.

NeonProvider.getTableSchema maps rows positionally but its test pinned the SELECT with toContain fragments. Swapping c.data_typec.is_nullable left 45/45 passing while production would report every column's type as "YES"/"NO". Five introspection blocks converted to exact-SQL assertions.

cleanup.ts came out clean — 11 probes, 0 survivors, including dropping the isValidUUID gate that would delete every session at a human path. It asserts full remaining-storage snapshots via toEqual, the right shape for deletion logic.

Three product bugs, fixed at the boundary

Each was asserted as correct by a test until the audit exposed it.

  1. sessionSyncReconcile.ts narrowed the run status for the runtime atom, then force-cast the raw wire string into the session list cache on the next line — so a value outside the union reached Session.status, which drives sidebar grouping and Kanban lanes. The unions are genuinely not assignable (CliSessionStatus carries installing), so rather than swap one cast for another this adds toSessionListStatus, a compiler-checked map. Behavior-preserving: every consumer that mentions installing already groups it with running.

  2. worktree_created/merge_result frames inserted empty created_at/updated_at. upsertSession pins timestamps on update but inserts verbatim, and taskTimestamps.ts reads empty as 0 — sorting the session to the epoch and dropping it out of every Kanban time window.

  3. A plan_approval chunk with no planPath was silently swallowed, so the plan vanished from the transcript — and the old test asserted that emptiness was correct, actively blocking its own repair. The guard now skips only the Build-card atom write; the chunk falls through to the event store. Verified the transcript renders such an event (planPath is never read on that path).

Potential risks

  • CI time. The rust job now runs cargo test --workspace. Measured 385 s warm, of which only 47 s is execution — the rest is building 94 test binaries, which the existing cargo clippy --workspace --all-targets step already pays for. A cold runner will be materially slower; if this hurts, the cache is the lever, not the test count.
  • 83 moved test files. Pure git mv + import rewrites, no logic changed. Verified against a clean-HEAD worktree: identical file and test counts before and after.
  • fix(session-core) changes runtime behavior — the only commit here that does. The status map is behavior-preserving by inspection of every consumer; the timestamp and plan-approval fixes change what reaches the store, deliberately.
  • frontend-ui-audit is still referenced by 5 routing rules with no SKILL.md anywhere. Deliberately not deleted — that would quietly end UI audits. Both files now carry an explicit callout instead. Restoring that skill is a follow-up someone needs to own.
  • 630 lines test four zero-caller exports. cleanupStaleRepoReferences, clearProjectRepoCache, clearSessionData, clearAllProjectData have no callers outside cleanup.ts and its tests, so half the cleanup.ts coverage jump guards dead code. Kept deliberately, flagged here: wire them up or delete them.
  • A latent data-loss hazard is pinned, not fixed. clearProjectRepoCache's over-broad substring sweep would destroy six real keys including orgii_recent_workspaces and orgii:codeSearchIndexedRepos. Harmless while nothing calls it; dangerous the moment a "Clear project cache" button appears.

Follow-ups found but out of scope

  • createStreamMessageId uses Date.now() (sync/utils/activityIds.ts:135), so two turns in the same millisecond collide. Reproduced at HEAD with a clean tree — 2 failures in 6 runs — so it is pre-existing, not introduced here. A real collision would corrupt a live transcript.
  • Four more status as SessionStatus casts with the same defect as bug 1: sessionSyncStateHelpers.ts:181 and :269, cliTurnLifecycleCoordinator.ts:134, useNativeSessionStatusMonitor.ts:133.

Validation

Check Result
npx vitest run 1166 files / 9560 tests passed, 0 failed, 0 skipped
cargo test --workspace --no-fail-fast 6814 passed, 0 failed, 36 ignored
npx tsc --noEmit exit 0
ESLint clean; .only guard verified to fire on all three forms and clean against all 1166 files
Mutation testing 103 probes; every fix in the audit commits proven by turning its mutant red

Baseline before this branch: 1148 files / 8966 tests, and cargo test reaching 1111.

Coverage (product code): 42.9% → 44.2% overall; DatabaseCore 3.2% → 100%, services/git 47.1% → 64.7%, SessionCore 54.5% → 60.4%, util/core 34.2% → 44.8%.

E2E was not run — it needs a built desktop app and a display. No e2e spec logic was changed, only README.md and one dead export removed.

Five `cargo:test:*` scripts filtered on Rust modules that no longer exist
(agent_core, event_store, work_station, agent_variants, tool_service). A
cargo filter matching nothing prints "test result: ok. 0 passed" and exits
0, so all five reported success while running nothing.

`agent_core` turned out not to be deleted at all: it is a workspace crate
(crates/agent-core, package name agent_core) holding 3170 tests, the
largest test body in the repo. The script was miswritten, not obsolete.
Repaired to `-p agent_core`. The other four match no module and no crate
in the workspace and are removed.

`cargo:test` itself was `--lib`, which excluded the 32 integration tests
under src-tauri/tests/. Worse, src-tauri/Cargo.toml is a workspace with
"." plus 44 member crates and no `default-members`, so even without
`--lib` a bare `cargo test` runs the root package only. Roughly 5700
crate tests had never been run by the project's own test command.

CI's rust job was clippy-only, so none of this was caught. It now runs
`cargo test --workspace` after clippy, which has already built the same
test targets against the same cache.

Pre-commit hook ran. Total eslint: 0, total circular: 0
The repo had two test-placement conventions with nothing documenting
either: 642 tests colocated beside their source, 506 in a `__tests__/`
subdirectory. 53 directories used both styles side by side, so there was
no way to tell where a new test belonged.

Each mixed directory now converges on whichever style it already used in
the majority, which is the minimum-churn way to make every directory
internally consistent: 83 files moved via `git mv` (60 into `__tests__/`,
23 colocated), 105 relative import specifiers rewritten. `@src/` and `@/`
aliases are absolute and were left alone.

Two pairs were genuine filename collisions — distinct suites sharing a
basename across the two conventions — and are resolved with the facet
suffix the convention now documents:

  util/__tests__/modelGrouping.test.ts            (grouping and sorting)
  util/__tests__/modelGrouping.thresholds.test.ts (version currency)
  util/core/storage/zodStorage.test.ts            (error degradation)
  util/core/storage/zodStorage.roundTrip.test.ts  (parse and serialize)

CONTRIBUTING.md gains a "Where tests live" section recording the rule,
the naming convention, and two discovery gotchas worth knowing: vitest
collects `src/**/*.test.ts` only, so a test outside `src/` never runs,
and a `.test.tsx` file is silently skipped.

No test file was renamed beyond those two pairs, no test logic changed,
and no source file was touched. Verified against a clean-HEAD worktree
baseline: 1148 files / 8966 tests before and after, all passing.

Pre-commit hook ran. Total eslint: 0, total circular: 0
`invoke_rolls_back_the_whole_graph_when_a_node_collides` had been failing
silently — it was never reached, because the test command was misconfigured
until the previous commit.

It seeded a cross-org landmine row at short_id AAA-0002 and expected
`invoke` to fail with ALREADY_EXISTS. It no longer collides: the allocator
now steps over same-prefix short_ids across orgs, deliberately, and that
behavior is covered by a passing sibling test
(`allocate_short_id_skips_same_prefix_across_orgs`).

In fact ALREADY_EXISTS is unreachable from `invoke` by any seeded row.
`allocate_short_id_in_tx` floors the counter at the org-wide max suffix
and then loops past any globally-taken id, and `guard_new_work_item_id_in_tx`
checks that same predicate — so the guard can never fire on an id the
allocator just produced. A same-org landmine does not work either: it
pushes the counter so the root itself moves, which is not mid-graph.
The guard remains meaningful for its explicit-short_id callers, which
`create_refuses_to_overwrite_an_existing_id_in_any_scope` still covers.

Split into two tests:

  invoke_steps_over_a_cross_org_short_id_instead_of_colliding
    keeps the original landmine setup but pins the current contract, so
    the reason the old assertion died is recorded rather than deleted.

  invoke_rolls_back_the_whole_graph_when_a_node_fails_mid_write
    drives rollback from a BEFORE INSERT trigger with a count-based
    predicate, deliberately independent of short_id allocation so
    renumbering cannot silently disarm it again.

Post-conditions go from 4 to 7 and now compare against a post-apply()
baseline rather than counting to zero, which would hide a partial commit:
workitems, pm_routine_runs, pm_relations, pm_idempotency, plus audit
events, the change_seq watermark, and next_work_item_id. A tail re-invoke
proves the abort landed mid-graph and left no idempotency poison.

Product code is unchanged. Rollback was verified by breaking it with
DropBehavior::Commit and confirming each of four assertions fails
independently.

Pre-commit hook ran. Total eslint: 0, total circular: 0
CLAUDE.md and AGENTS.md both cited `.cursor/rules/ui-feature-workflow.mdc`
twice as the authority on unit-test gates. That file has never existed in
this repository's history — `.cursor/` is gitignored, with exactly one
force-tracked exception — so agents were being routed to nothing. The
references are removed and replaced with a pointer to the new
CONTRIBUTING.md "Where tests live" section, which is tracked and real.
The same line is dropped from the react-best-practices skill.

Both files also listed two `~/.orgii/skills/...` user-global paths that
resolve nowhere, marked the architecture-audit workspace copy "if present"
when it is present, and between them omitted dual-instance-verification
and org2-performance-guard. All cited skill paths now resolve.

`frontend-ui-audit` is deliberately left in the routing table despite
having no SKILL.md anywhere: silently deleting five routing rules would
quietly end UI audits altogether. Instead both files now carry an explicit
callout to apply its intent by hand and say so, rather than claim an audit
ran that could not have.

tests/agent_sessions/TEST_CASES.md pointed at seven Rust test locations,
five of which had moved — unified_stats was restructured into
session_directory and the health module is gone entirely. Repointed at
where the tests actually live.

tests/e2e/README.md documents the credential gates that gate nine
work-item scenarios. E2E_OPENAI_MODEL was documented nowhere despite
gating them: an account missing op-4.6-relay in enabled_models makes all
nine skip in silence while the run still reports green. Also removes
`isControlScenarioExplicitlyRequested`, which was defined and exported
with no importers anywhere.

Pre-commit hook ran. Total eslint: 0, total circular: 0
Nothing prevented `describe.only` / `it.only` from being committed. A
focused test makes every other test in its file silently not run while
the suite still exits 0 — the same "green but not actually running"
failure mode as a cargo filter that matches no module, which is what
started this branch.

Implemented with `no-restricted-syntax` rather than eslint-plugin-vitest
so it adds no dependency, scoped to test files via an override, and
covering `.only`, `.only.each`, and the `describe`/`it`/`test`/`suite`/`bench`
aliases. Verified it fires on all three forms and is clean against all
1148 existing test files.

`.skip` and `.todo` are deliberately still allowed: they appear in the
reporter's skipped count, so they are visible rather than invisible.

Also records the import convention the suite already follows — relative
path to the module under test, `@src/` alias for anything cross-module.

Pre-commit hook ran. Total eslint: 20, total circular: 0
CLAUDE.md's root-cause-first doctrine puts invariants at the boundary
where external data first becomes domain state, and requires the
regression test to sit at that producing boundary. These six modules are
that boundary — they turn external CLI/IDE agent output into ORGII
session state — and they were the least-covered code in the repo.

  createCliEventHandler.ts   0.91% ->  100% stmt (0% -> 100% fn)
  sessionSyncReconcile.ts       0% ->  100%
  cursorIdeAdapter.ts         4.6% ->  100%
  agentMessageAdapters.ts     7.7% ->  100%
  nativeTranscriptReconcile.ts 13.7% -> 100%
  eventFactories.ts            28% ->  100%

186 tests covering malformed and partial frames, out-of-order arrival,
idempotency of replayed events, forward compatibility with unknown event
kinds, and native-transcript replace-vs-merge reconciliation.

eventStoreProxy is replaced with an in-memory store so assertions read
resulting state rather than call logs. Real collaborators stay real:
cliLifecycle, the Jotai atoms, streamTextAccumulator, streamingParsers.
Mocks stop at genuine I/O edges only. No test asserts merely that a mock
was called.

Four findings are recorded but deliberately not fixed here, since each
needs a product decision rather than a test change:

- worktree_created/merge_result can insert a session row with empty
  created_at/updated_at, which then feed Kanban time filtering and
  sidebar ordering. The field mapping is pinned; the empty timestamps are
  explicitly NOT asserted as correct.
- A plan_approval chunk missing planPath is swallowed with no log, so the
  plan vanishes from the transcript.
- sessionSyncReconcile narrows runStatus through toCliSessionStatus for
  the runtime atom, then passes the raw unvalidated string to
  updateSessionStatus on the next line. Only the validated half is
  asserted.
- `cancelled` in createCliEventHandler is write-only; nothing branches on
  it.

Branch coverage on createCliEventHandler stops at 94.41%: the residue is
the isStoreInitialized() === false lane, which a test file that creates
the module-level Jotai singleton cannot un-create.

Pre-commit hook ran. Total eslint: 20, total circular: 0
DatabaseCore was the worst-covered directory in the repo at 3.2%. Its
only test file covered types.ts; everything that does work — six
providers and the factory — was at 0%.

  SupabaseProvider    0% -> 100%      MySQLProvider       0% -> 100%
  NeonProvider        0% -> 100%      PostgresProvider    0% -> 100%
  TursoProvider       0% -> 100%      TauriSqliteProvider 0% -> 100%
  factory.ts          0% -> 100%      isValidSqliteFile   0% -> 100%

Directory: 3.2% -> 100% statements and functions, 98.06% branches. 285
tests. Only the driver boundary is mocked (@tauri-apps/api/core,
plugin-shell, @libsql/client); no provider is ever mocked, and the
factory tests construct real provider classes. Assertions are on the
exact SQL string, the exact bound parameter array, and the returned
value.

Writing these surfaced a number of genuine defects. NO product code is
changed here — each needs a decision. Three carry live reproductions as
it.fails(), so the suite stays green while the bug exists and turns red
the moment it is fixed, which forces the pin to be removed:

- MySQL string literals escape ' but not \. MySQL treats \ as an escape
  unless NO_BACKSLASH_ESCAPES is set, so a trailing backslash never
  closes the literal and the next column is parsed as SQL. Injection.
  (MySQLProvider.ts:351)
- delete/update with an empty where map emit a bare `WHERE`, i.e. an
  unbounded statement.
- Table, column and orderBy identifiers are interpolated without doubling
  the quote character, across all five SQL providers. Column names reach
  this from user-editable row data.
- Neon and Supabase getTableSchema splice tableName into a single-quoted
  literal; an apostrophe rewrites the query.
- Turso's sqlite_master lookup uses a double-quoted identifier, so a
  table named after a sqlite_master column degenerates to name = name.
- Turso leaks the libsql client when the connect probe fails.
- NeonProvider.execute() skips ensureConnected(), unlike every other
  method and provider.
- Connection-string credentials are not URL-encoded, so a password
  containing @ / : ? # corrupts the DSN.
- Supabase URL validation is unanchored; Neon's host regex has an
  unreachable alternative and silently drops the port.
- Dates serialize as ::jsonb rather than a timestamp.
- Neon and Supabase pass the connection string and access token as curl
  argv, readable via ps.

Unverifiable without a live server, and flagged rather than claimed:
NeonProvider.executeHttp unwraps result.rows[0], assuming a nested /sql
response, where Neon documents a top-level shape. Mocks encode the
assumption, so the suite cannot see this. Worth checking by hand.

Pre-commit hook ran. Total eslint: 20, total circular: 0
Two high-risk modules that were effectively untested. remoteOps drives
push/fetch/pull against a user's real repository; cleanup.ts is deletion
logic reached from src/index.tsx, and nothing proved it deleted only what
it should.

  services/git/operations/remoteOps.ts  0.43% -> 100% (0% -> 100% fn)
  util/core/storage/cleanup.ts             0% -> 100% (branch 97.75%)

120 tests. For cleanup the emphasis is on what must NOT be deleted —
retention boundaries, preserved keys, eviction victims, partial failure,
idempotency. For remoteOps, assertions are on the exact argv and wire
payload; ./types stays real so parseGitError's stderr-to-domain-error
translation is genuinely exercised rather than stubbed.

Findings, no product code changed:

- clearProjectRepoCache violates its own docstring. It claims to preserve
  UI settings, but its substring sweep deletes any key containing
  project/repo/workspace/codebase unless it also contains
  theme/setting/config. orgii:projectsSidebarGroupBy — a real UI
  preference — is destroyed. All 27 literal storage keys in src/ were
  audited: no credential key is hit, so there is no auth loss.
- The same predicate is latently dangerous: key.startsWith("cur") would
  take cursor_ide_last_scan and currentUserLocale, and includes("repo")
  would take bug_report_draft and crashReports. This repo imports Cursor
  IDE sessions, so the cur prefix is a landmine for the next key added.
- clearProjectRepoCache does not clear the repo selection it is named
  for: the live selection is window-scoped in sessionStorage as
  selected_repo_<windowId>, and the sessionStorage sweep does not match
  "repo".
- cleanupStaleRepoReferences reads getItem raw and never calls
  parseStorageValue, so a JSON-encoded id written by atomWithStorage is
  never pruned.
- cleanedTabs in its result is never assigned and is always false.
- publish() drops setUpstream, remote and branch on the terminal
  fallback path, though the Rust path sends set_upstream correctly.

Not a bug but worth a decision: force-push uses bare --force rather than
--force-with-lease. Verified that force is never set on publish, sync or
plain push, and that sync short-circuits so a failed fetch or pull never
reaches the push.

The 2.25% branch gap is two `if (!key) continue` guards against
Storage.key(i) returning null while length > i, which a conforming
Storage cannot do; faking it would test the mock.

Pre-commit hook ran. Total eslint: 20, total circular: 0
Mutation-tested the coverage added in the previous three commits: 57 probes
against DatabaseCore/git/storage (47 killed) and 46 against SessionCore
(31 killed). The survivors clustered, and this commit closes the two
clusters outside SessionCore.

remoteOps: destructive flags were entirely unasserted on the credential
retry path. Four mutations left all 69 tests green, including
`force: params.force` -> `force: true` — meaning a plain push that hit an
auth failure, picked up a stored credential and retried could have been
silently converted into a force-push with credentials attached.

The cause was that the file's single strict `toEqual` on a retry payload
used a fixture that already passed `force: true` and `remote: "origin"`,
so hardcoding either value was invisible, while the other retry assertions
used `toMatchObject`, which cannot detect an added flag. That fixture is
now a plain push to a non-default remote, eight retry assertions are
`toEqual` over the full wire payload, and a named invariant test asserts
the retry adds no destructive flag and no remote of its own. All four
mutations now fail. Result assertions stay `toMatchObject` deliberately:
they check returned errors, not outbound payloads, and `toEqual` would
couple them to git stderr copy.

DatabaseCore: five introspection blocks pinned SQL with `toContain`
fragments instead of the exact-SQL `.toBe` used elsewhere in the same
files. Six mutations survived. The material one is that
NeonProvider.getTableSchema maps rows positionally, so swapping
`c.data_type` and `c.is_nullable` in its SELECT left 45/45 passing while
production would report every column's type as "YES"/"NO". All five now
assert one whitespace-normalized exact SQL.

cleanup.retention: the test pinning the over-broad substring sweep in
`clearProjectRepoCache` used three invented storage keys, so a real
data-loss hazard read as hypothetical. Replaced with six keys verified to
exist, including orgii_recent_workspaces and orgii:codeSearchIndexedRepos.
The assertion still pins current behavior; only the illustration changed.

Pre-commit hook ran. Total eslint: 0, total circular: 0
Mutation testing scored createCliEventHandler.test.ts at 52% — 12 of 25
probes survived. This commit fixes the survivors and documents three
product defects the tests had been certifying as correct.

Deleted or replaced, having proven nothing:

  The "does not throw when the normalize RPC rejects" test — the throw is
  inside a promise chain and could never propagate synchronously, so the
  assertion held trivially. Deleting all three `.catch` handlers left 61
  tests passing. Replaced with one asserting both lanes actually log.

  "reports each terminal status separately" did not test the no-coalescing
  machinery it named; deleting that whole block left 61 passing. Renamed
  to what it checks, plus a new test observing the reset.

  eventFactories' "never falls back to a generic id" only passed a
  non-empty id, so the fallback branch never ran — the exact regression
  its JSDoc warns about survived. Now passes an empty id.

Added, previously unguarded: thinking-lane snapshot dedupe (the message
lane had one, its sibling did not, leaving that duplicate-text bug class
open); and caps for capStreamContent, appendBoundedToolCallArgs and
makeRoomForToolCallDelta, each of which could be made unbounded with all
61 tests still green.

Three defects are pinned with `it.fails()` alongside companions asserting
current output, so fixing the product turns them red rather than silently
green:

  A plan_approval chunk with no planPath is swallowed, and the old test
  asserted the resulting emptiness was correct — applying the fix made
  that test fail, so it actively blocked its own repair.

  worktree_created and merge_result frames insert created_at/updated_at
  as "", and toMatchObject skipped both fields. taskTimestamps.ts:4 reads
  an empty timestamp as 0, dropping the session out of every Kanban time
  window.

  sessionSyncReconcile narrows the run status for the runtime atom, then
  writes the raw wire string to the session list cache on the next line.
  The test could not see it because it seeded sessionsAtom with [].

agentMessageAdapters.test.ts was left alone: four independent probes all
died, so it is already solid.

Pre-commit hook ran. Total eslint: 0, total circular: 0
Each of these was certified as correct by a test until the previous commit
exposed it. Fixed at the producing boundary per the root-cause-first rule
in CLAUDE.md, rather than filtered downstream.

sessionSyncReconcile narrowed the run status for the runtime atom and then
force-cast the raw wire string into the session list cache on the next
line, so a value outside the union reached `Session.status`, which drives
sidebar grouping, Kanban lanes and every terminal-status predicate. The
status is now narrowed once and both sinks read the narrowed value.

The two unions are genuinely not assignable — `CliSessionStatus` carries
`installing` and `SessionStatus` does not — so rather than swap one cast
for another this adds `toSessionListStatus`, a compiler-checked map that
collapses only `installing` to `running` and passes everything else
through. That is behaviour-preserving: every consumer of `Session.status`
that mentions `installing` already groups it with `running`
(RUNNING_SESSION_STATUSES, IN_PROGRESS_STATUSES, BACKEND_ACTIVE_STATUSES),
and nothing renders it as a label. The runtime atom still gets the
un-collapsed value.

worktree_created and merge_result frames inserted created_at and
updated_at as empty strings. upsertSession pins timestamps on the update
path but inserts verbatim, so a session first seen through either frame
was created with empty ones, and taskTimestamps.ts reads empty as 0 —
sorting it to the epoch and dropping it out of every Kanban time window.
Neither frame carries a timestamp on the wire and both are broadcast when
the work completes, so the insert time is the accurate value.

A plan_approval chunk with no planPath was dropped entirely, so the plan
vanished from the transcript. The guard now skips only the Build-card atom
write, which legitimately needs a path; the chunk falls through to the
event store and the missing field is logged. Confirmed the transcript
renders such an event: planPath is never read on that path. The realistic
trigger is the backend emitting an empty planPath, which the handler's
string check rejected — an ordinary empty-path case, not a malformed frame.
A null store no longer swallows the row either.

Verified by mutation: 10 of 10 probes killed, including reverting each fix,
flattening the status map, an epoch timestamp, and restoring the drop.

Pre-commit hook ran. Total eslint: 0, total circular: 0
@Harry19081 Harry19081 closed this Aug 22, 2026
@Harry19081 Harry19081 reopened this Aug 22, 2026
@Neonforge98
Neonforge98 merged commit 3467e06 into develop Aug 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants