test: retire ai-vault cases whose inputs the scanner never reads by nwparker · Pull Request #23992 · stablyai/orca

nwparker · 2026-09-30T03:32:08Z

	Files	Added	Deleted	Net
Test	11	$\color{#1a7f37}{\Huge{\mathbf{+}}}$2	$\color{#cf222e}{\Huge{\mathbf{−}}}$190	$\color{#cf222e}{\Huge{\mathbf{−}}}$188
Prod	0	0	0	0

ELI5

The biggest single removal here is a 36-case block that checked root === join(root) — a path compared against itself. It ran six agents × six environment values, and every one of those 36 cases asserted that a value equals itself.

The rest follow one question: does the scanner actually read the field this case varies? Often not. One case varied change.type on a code path that reads only change.unified_diff ?? change.content. Another varied a spawn depth on a check that accepts any positive integer.

What Changed

−190 lines across 11 files. 48 cases removed. No production code touched, no files deleted.

A self-comparison, 36 times over

session-scanner-agent-root-overrides.test.ts's never yields a relative root for %j block asserted root === join(root). The sibling falls back to the default root for %j pins roots[0] to the exact absolute default, which is the real contract; the extra roots the block also scanned (agent_logs, .clawdbot/agents) are homedir-derived and unaffected by the env var it varied. The #13082 rationale comment is preserved on the surviving case.

Inputs the production path never reads

removed	the production fact
the `{ type: 'delete' }` FileChange row	`session-scanner-codex-tool-records.ts:89` reads only `change.unified_diff ?? change.content`, never `change.type` — identical path to `add`. The `add`/`update` rows stay as the two real disjuncts
"carries the depth of a nested child instead of flattening it"	`codexSpawnDepth` accepts any positive integer, so depth 2 is the same branch as depth 1
"keeps the rest of the spawn when the naming path is null"	`agentPath` is an independent `??` fallback with no cross-field logic — passes either way if the fallback is removed
"resolves homes only for distros currently reported as running"	`getAiVaultWslHomeDirs` reads only `platform` and a `hasCachedWslDistros()` gate; the "only running" filtering lives entirely inside a mocked async call

Cases that cannot fail for the reason they name

an all-unknown-agent response whose throw requires malformedSessionCount > 0 — which is 0 in that fixture;
a symlink rejection byte-identical to the directory case above it (both isFile: () => false);
a runtime restamp whose fixture already carries the executionHostId and id it asserts, so restampAiVaultListResult returns the object unchanged;
a scaling case whose listing count is N-independent, because fixedChildFileSegments does one readDir plus a direct stat per child — a per-session-readDir regression fails at N=1 anyway, and its companion toHaveLength(40) is inert against a scan limit of 1000.

Why — three keeps that draw lines this audit needed

Spy-counting tests are not automatically the bench pattern. An earlier PR in this series deleted a 137-line bench whose assertion reduced to 120 < 134.33 over two constants declared in the test file. session-scan-cutoff and session-scanner-dedup-batches look identical in shape — they count Array.prototype.sort and RegExp.prototype.test calls — but they drive the real scanner over 128 fixtures and assert the limit-ordered result too, so a re-sort-per-candidate regression fails them for the right reason. Same shape, opposite verdict; the difference is whether production code sits between the spy and the assertion.

A locally re-spelled encoder was kept on purpose. session-scanner-claude-unicode-scope.test.ts duplicates Claude's directory-name encoding rather than importing it — normally copied-logic junk. Its comment gives the reason: importing the production encoder would hide Orca drifting from Claude's actual naming. It is a differential against a third party, so the two sides genuinely can diverge.

A vacuous assertion was kept because it anchors a type-level ratchet. session-parse-cache-persistence.test.ts:175 asserts keys.length > 0, which cannot fail. But it is the only reference to the satisfies Record<keyof AiVaultSession, true> table, and nothing else enforces that table. Deleting the case would make the ratchet dead code.

Also kept: the per-agent session-delete-target rows, each the registration ratchet that an agent is in the deletable table with the right root and extension — the same shape as the #7253 ratchet an earlier PR preserved; and never plans file-history, a deliberate-absence guard over a user-data rewind buffer.

Linked Issue

N/A — test-suite hygiene, no pre-existing issue.

Visual Proof

N/A — no production code changes in this PR.

Testing

Verified locally on macOS after committing and rebasing onto current main:

All 11 modified test files: 161 passed.
node config/scripts/check-reliability-gates.mjs — passes, 140 gates.
pnpm run check:code-quality:changed — passes, 0 findings.
pnpm tc not required — zero production files changed.

Not run locally: e2e and the Windows/Linux/SSH jobs — left to CI.

I manually tested these changes locally
Automated tests added/updated, or explained why not below

Removal-only; no new tests.

Review

Every deletion cites the production line or sibling that owns it, so the reasoning is checkable directly. The three keeps are the judgement calls — particularly the vacuous keys.length > 0 case, which stays only because it is load-bearing for a satisfies table.

Agent skill upstream boundary

Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Security / bounds: no limit, bound or guard removed. The pathReads <= 2000 admission bound, the title-index LRU cap, and the depth-contradiction bound were all explicitly kept, as was the OpenCode-SQLite-never-a-delete-target invariant.
Cross-platform: remote-session-scanner-sources.test.ts was kept whole despite looking like a copied inventory — it is the only assertion of Devin's platform split (AppData/Roaming on win32 vs .local/share elsewhere).
Performance / back-compat: no runtime behavior changes; the two spy-count performance guards are retained.

Checklist

This PR is small and focused
I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
Before/after screenshots or videos attached for UI changes, or N/A with reason
Self-reviewed for correctness, security, and performance
Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Semantic sweep of src/main/ai-vault. 48 cases removed across 11 files; no production code touched, no files deleted. The largest single removal is a 36-case block (6 agents x 6 env values) in `session-scanner-agent-root-overrides.test.ts` whose assertion was a self-comparison: `root === join(root)`. The sibling `falls back to the default root for %j` pins `roots[0]` to the exact absolute default, and the extra roots the block also scanned (`agent_logs`, `.clawdbot/agents`) are homedir-derived and unaffected by the env var it varied. The #13082 rationale comment is kept on the surviving case. Inputs the production path never reads: - `session-scanner-codex-tool-records.ts:89` reads only `change.unified_diff ?? change.content` and never `change.type`, so the `{ type: 'delete' }` row was the identical path as `add`; the `add`/`update` rows remain as the two real disjuncts. - `codexSpawnDepth` accepts any positive integer, so depth 2 was the same branch as depth 1. - `agentPath` is an independent `??` fallback with no cross-field logic, so "keeps the rest of the spawn when the naming path is null" passes either way. - `getAiVaultWslHomeDirs` reads only `platform` and a `hasCachedWslDistros()` gate, so a case varying which distros are "currently running" took the default branch; the filtering lives entirely inside a mocked async call. Cases that cannot fail for the reason they name: an all-unknown-agent response whose throw requires `malformedSessionCount > 0` when it is 0; a symlink rejection byte-identical to the directory case above it (`isFile: () => false`); a runtime restamp whose fixture already carries the `executionHostId` and `id` it asserts. Also removed: duplicates of a stronger sibling in `session-list-results`, `session-parse-cache-persistence` (same `schemaVersion !==` gate), `session-scanner-claude-title` (owned by the subagent-prune test, which also asserts `subagentTranscriptCount`), and a session-scanner listing case whose count is N-independent because `fixedChildFileSegments` does one readDir plus a direct stat per child — so a per-session-readDir regression fails at N=1. Three keeps worth recording. Spy-counting tests were kept where real code runs: `session-scan-cutoff` and `session-scanner-dedup-batches` count `Array.prototype.sort` / `RegExp.prototype.test`, but drive the real scanner over 128 fixtures and assert the limit-ordered result too, so a re-sort-per-candidate regression fails them for the right reason — unlike a bench whose assertion was arithmetic over its own constants. `session-scanner-claude-unicode-scope` keeps its locally re-spelled dir-name encoder deliberately: importing the production one would hide Orca drifting from Claude's actual naming. And `session-parse-cache-persistence.test.ts:175` stays although `keys.length > 0` cannot fail — it is the only reference to the `satisfies Record<keyof AiVaultSession, true>` table, so deleting the case would make that type-level ratchet dead.

greptile-apps · 2026-09-30T03:35:54Z

Confidence Score: 5/5

[Low risk] Removes test cases from the AI vault test suite.

The PR appears safe to merge; the removed cases do not leave an actionable coverage gap.

Summary

The PR removes redundant AI Vault tests without changing production code.

Retains assertions for host stamping, provider-root fallback, session filtering, cache compatibility, and remote discovery bounds.
No actionable loss of coverage was identified.

_{Reviews (1) · Last reviewed commit: "test: retire ai-vault cases whose inputs..."}

pullfrog

✅ No new issues found.

Reviewed changes

Read the full diff (11 test files, +2/−190) and checked every deletion against the production path or its surviving sibling; ran the 11 files (161 passed) and oxlint on all of them (0 findings).

cached-session-list.test.ts — dropped the "only running distros" case; the filtering lives inside the mocked listRunningWslHomeDirsAsync, and the surviving "still discovers running distros…" case asserts the same passthrough.
remote-session-scanner.test.ts — dropped the 40-session Antigravity scaling case; the surviving "parses only canonical Antigravity transcripts" case asserts the identical single-readDir filter.
runtime-session-scanner.test.ts — dropped the host-stamp case; its beforeEach fixture already carried the asserted id and executionHostId, and the surviving "stamps sessions … for a different execution host" case exercises the real restamp.
session-delete.test.ts — dropped the symlink case, byte-identical (isFile: () => false) to the surviving directory case.
session-list-result-validation.test.ts — dropped the all-unknown-agent case; the throw requires malformedSessionCount > 0, which is 0 there, and the mixed-unknown case pins the no-throw path.
session-list-results.test.ts — dropped the per-host scope case; the merge is a flat issue concat covered by two surviving cases.
session-parse-cache-persistence.test.ts — dropped the schema-1 case; SCHEMA_VERSION is 3, and the surviving mismatched-schema (999) case pins the same rejection.
session-scanner-agent-root-overrides.test.ts — replaced the tautological root === join(root) block (36 cases) with the surviving roots[0] === defaultRoot() assertion; the extra roots it also scanned are homedir/default derived and unaffected by the varied env var.
session-scanner-claude-title.test.ts — dropped the Task-subagent exclusion case; session-scanner-claude-subagent-prune.test.ts pins the same behavior plus subagentTranscriptCount.
session-scanner-codex-non-user-origin.test.ts — dropped the depth-2, null-agent_path, and user-thread cases; each is either the same branch as a surviving case or covered by a surviving loop.
session-scanner-codex-tool-records.test.ts — dropped the { type: 'delete' } row; session-scanner-codex-tool-records.ts:89 reads only unified_diff ?? content, identical to the surviving add row.

^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

coderabbitai · 2026-09-30T03:40:39Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

The changes update AI Vault tests only. They remove several standalone tests and assertions, move runtime host assertions into a test that also checks issue stamping, and update the relative-root test to check the configured default.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 69404

A future regression could silently omit deleted-file records. Restoring the small test case would close this bounded coverage gap before merging.

Architecture Summary

Architecture risk: 🔵 Low · up to 69404

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — src (service) was modified; 11 changed files map to changed impact.

Before / after behavior

observed — Modified behavior in src/main/ai-vault/cached-session-list.test.ts: Removed the test that mocked a running WSL home directory and expected getAiVaultWslHomeDirs() to return it after one call to listRunningWslHomeDirsAsync.
observed — Modified behavior in src/main/ai-vault/remote-session-scanner.test.ts: Removed the test that created 40 Antigravity transcripts and expected all 40 sessions to be found with only one listing of the brain directory.
observed — Modified behavior in src/main/ai-vault/runtime-session-scanner.test.ts: Removed the standalone test that scanned a runtime environment and asserted the accepted session’s ID and execution host; those session-stamping assertions are now covered alongside issue-stamping assertions in the preceding test.
observed — Modified behavior in src/main/ai-vault/session-delete.test.ts: Removed the test asserting that a symlink is rejected with unexpected-target-kind and is not passed to trashItem.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: removing AI Vault tests for inputs that production code does not read.
Description check	✅ Passed	The description is complete and focused. It explains the changes, rationale, retained safeguards, testing performed, platform coverage, and absence of production or visual changes. The linked issue is…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

src/main/ai-vault/session-scanner-codex-tool-records.test.ts (1)
68-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Keep the delete case in the paginated file-change test.

publishCodexCompletedTool accepts each file-change entry without filtering its type and publishes unified_diff or content. Therefore, the delete record with content: '+ addedneedle' is valid for this parser boundary. Removing it leaves no assertion that paginated delete records are published. A regression that drops delete records can pass the remaining add and update cases.
Suggested fix
 it.each([
   { type: 'add', content: '+ addedneedle' },
+  { type: 'delete', content: '+ addedneedle' },
   { type: 'update', unified_diff: '+ addedneedle', move_path: null }
 ])('publishes paginated $type file changes', (change) => {

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c6f994c2-48a7-4dd3-b435-810a34cd10df

📥 Commits

Reviewing files that changed from the base of the PR and between 0978474 and 6940461.

📒 Files selected for processing (11)

src/main/ai-vault/cached-session-list.test.ts
src/main/ai-vault/remote-session-scanner.test.ts
src/main/ai-vault/runtime-session-scanner.test.ts
src/main/ai-vault/session-delete.test.ts
src/main/ai-vault/session-list-result-validation.test.ts
src/main/ai-vault/session-list-results.test.ts
src/main/ai-vault/session-parse-cache-persistence.test.ts
src/main/ai-vault/session-scanner-agent-root-overrides.test.ts
src/main/ai-vault/session-scanner-claude-title.test.ts
src/main/ai-vault/session-scanner-codex-non-user-origin.test.ts
src/main/ai-vault/session-scanner-codex-tool-records.test.ts

💤 Files with no reviewable changes (10)

src/main/ai-vault/cached-session-list.test.ts
src/main/ai-vault/remote-session-scanner.test.ts
src/main/ai-vault/session-list-results.test.ts
src/main/ai-vault/session-delete.test.ts
src/main/ai-vault/session-list-result-validation.test.ts
src/main/ai-vault/session-scanner-codex-tool-records.test.ts
src/main/ai-vault/session-scanner-codex-non-user-origin.test.ts
src/main/ai-vault/session-scanner-claude-title.test.ts
src/main/ai-vault/session-parse-cache-persistence.test.ts
src/main/ai-vault/runtime-session-scanner.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

pullfrog Bot reviewed Sep 30, 2026

View reviewed changes

coderabbitai Bot reviewed Sep 30, 2026

View reviewed changes

nwparker merged commit 34d6041 into main Sep 30, 2026
33 checks passed

kiwidream mentioned this pull request Sep 30, 2026

sync: upstream d2dbe2c385 into swaplabs/main SwapLabsInc/orca#29

Merged

Navigation Menu

Repository navigation

test: retire ai-vault cases whose inputs the scanner never reads - #23992

test: retire ai-vault cases whose inputs the scanner never reads#23992
nwparker merged 1 commit into
mainfrom
nwparker/test-audit-wave16

nwparker commented Sep 30, 2026 •

edited by github-actions Bot

Loading

Uh oh!

greptile-apps Bot commented Sep 30, 2026 •

edited

Loading

Uh oh!

pullfrog Bot left a comment

Uh oh!

coderabbitai Bot commented Sep 30, 2026 •

edited

Loading

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

nwparker commented Sep 30, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ELI5

What Changed

A self-comparison, 36 times over

Inputs the production path never reads

Cases that cannot fail for the reason they name

Why — three keeps that draw lines this audit needed

Linked Issue

Visual Proof

Testing

Review

Agent skill upstream boundary

Notes

Checklist

Uh oh!

greptile-apps Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Architecture Summary

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nwparker commented Sep 30, 2026 •

edited by github-actions Bot

Loading

greptile-apps Bot commented Sep 30, 2026 •

edited

Loading

coderabbitai Bot commented Sep 30, 2026 •

edited

Loading