Follow up on deferred path-migration review maintenance

[acoliver]

Context

PR 2646 implements issue 2606's OS-standard application-directory migration. CodeRabbit identified several worthwhile maintenance improvements that are not required for the directory migration's runtime correctness and would unnecessarily expand that already-large PR.

Deferred maintenance work

Documentation consistency

• Add missing fenced-block language annotations reported by markdown lint.
• Normalize unrelated debug-log filename examples.
• Correct the unrelated MCP command scope example.
• Replace contradictory absolute-path examples in the approval-mode migration guide.

Test maintenance

• Replace legacy-looking neutral fixtures in memory-tool interface tests with canonical temporary fixture paths where they cannot touch real state.
• Consider a shared test utility for the repeated four-category storage environment snapshot/restore pattern across CLI and A2A extension tests.
• Improve the null-byte error assertion in the OAuth lock test to use a stable error code or name.
• Gate chmod-based permission tests on Windows.
• Add an outcome-based Todo continuation resolver-propagation test if current filesystem behavior does not already cover it; do not add constructor-spy/mock-interaction tests.

Code cleanup and efficiency

• Centralize duplicate Todo read fallback logic shared by todo-read and todo-write.
• Use the exported CoreStorageServiceAdapter singleton consistently in the production tool registry.
• Preserve Vitest default excludes in the A2A config.
• Rename the misspelled DOT_LLPRT_PATTERN constant.
• Add missing legacy-guard self-test cases and consider caching recursive walks by base directory.
• Narrow the Storage legacy-helper allowlist matcher.
• Validate sub-unit timeout overrides after flooring in API-surface and nightly Bun smoke helpers.
• Make GenAI status parsing consume the second NUL token in git rename records.

Acceptance criteria

• Each item is evaluated against current main before implementation.
• Behavioral changes have behavioral tests; no mock-interaction tests.
• No weakening of lint, legacy-path, or GenAI-enclave guards.
• The work may be split into focused PRs by documentation, tests, and tooling.

Related: PR 2646, issue 2606.

[coderabbitai]

🔗 Related PRs

#1466 - feat(storage): move secure-store fallback to platform-standard paths (Fixes #1357) [merged]
#2148 - Eliminate storage ESLint directives (Fixes #2119) [merged]
#2176 - fix(storage): harden system settings path override (Fixes #1987) [merged]
#2247 - fix(config): make legacy path migration marker-based so subagents/commands/policies migrate (Fixes #2237) [merged]
#2311 - docs: Update documentation for Bun runtime migration (Fixes #2245) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

✅ Successfully generated implementation plan.

💡 Use @coderabbitai <feedback> to replan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[coderabbitai]

Coding Plan

Summary

• Evaluate every ticket item against current main first; implement confirmed defects, and explicitly document items that are already satisfied or non-applicable (DOT_LLPRT_PATTERN, a2a excludes, code-based null-byte assertion) to honor the acceptance criteria.
• Split the work along the sanctioned boundaries — documentation, tests, tooling/code — as three focused, independently shippable phases.
• Centralize duplicated Todo fallback logic and the storage adapter singleton by extending existing DI/facade patterns rather than adding new abstractions.
• Add only behavioral/outcome-based tests (no mock-interaction tests) and preserve all lint, legacy-path, and GenAI-enclave guard strength.

Design Choices

Design Choice 1: Which Vitest config actually needs the "preserve default excludes" fix?

Options Considered:

1. Leave a2a untouched (already safe) and fix the genuine offender in the CLI config by spreading configDefaults.exclude.
2. Add a defensive explicit configDefaults.exclude spread to the a2a config even though it changes nothing behaviorally.

Chosen Option: 1

Rationale: The ticket names the A2A config, but on current main packages/a2a-server/vitest.config.ts defines no exclude at all and therefore already inherits configDefaults.exclude. The real literal-array-without-spread offender is packages/cli/vitest.config.ts (baseExcludePatterns). Fix the actual offender (packages/cli/vitest.config.ts) so defaults are genuinely preserved, and document that a2a already inherits defaults — this satisfies the ticket's intent without cosmetic churn.

Design Choice 2: How to "use the exported CoreStorageServiceAdapter singleton" when no singleton export currently exists?

Options Considered:

1. Add a module-level exported singleton instance and consume it in the registry.
2. Convert the adapter to expose static methods only, mirroring Storage.

Chosen Option: 1

Rationale: toolRegistryFactory.ts calls new CoreStorageServiceAdapter() inline; the class is stateless and delegates to the static Storage class. No getInstance/module singleton exists today. An exported singleton preserves the existing dependency-injection boundary used by IStorageService/ToolKeyStorageFacade, requires the least disruption, and matches the ticket's "exported singleton" wording.

Design Choice 3: Which Todo continuation resolver should receive the outcome-based test?

Options Considered:

1. Add the outcome-based test against the actively-wired packages/agents implementation.
2. Add it against the orphaned CLI service.

Chosen Option: 1

Rationale: Two implementations exist: packages/cli/src/services/todo-continuation/todoContinuationService.ts (imported nowhere — orphaned) and packages/agents/src/core/TodoContinuationService.ts (wired into the runtime via ChatSessionFactory/client.ts). Test the implementation that actually executes at runtime so the coverage is behaviorally meaningful; flag the orphaned CLI service for a follow-up decision rather than testing dead code.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Documentation Consistency

Resolve the documentation defects so markdown lint passes and the guides stop contradicting themselves. These are self-contained edits with no runtime impact and form the first shippable PR. Focus on internal consistency and alignment with the actual runtime defaults established by the migration work.

Task 1: Add missing fenced-block language annotations

Silence markdown-lint fenced-code warnings on migration-adjacent docs.

• Update docs/cli/commands.md to add language tags (e.g. text/bash) to the unlabeled fences in the At Commands (~lines 385–388) and Shell Passthrough (~lines 403–407) examples.
• Run the repository markdown lint pass to catch any additional unlabeled fences introduced by the migration PR and annotate them.

Task 2: Normalize debug-log filename and MCP scope examples

Remove contradictory/incorrect examples unrelated to but reported alongside the migration.

• Update docs/debug-logging.md so the filename convention is stated consistently as the PID-based llxprt-debug-<PID>.jsonl, removing the conflicting debug-YYYY-MM-DD.jsonl "Can't Find Log Files?" reference.
• Reconcile the /mcp scope description: ensure docs/cli/commands.md and docs/tools/mcp-server.md agree with the actual default scope (project, per packages/cli/src/commands/mcp/add.ts), correcting whichever example is wrong.

Task 3: Fix contradictory path examples in the approval-mode migration guide

Eliminate the ~-vs-absolute contradiction so users get one consistent rule.

• Update docs/migration/approval-mode-to-policies.md to distinguish the file creation path from the configured policyPath value: either use absolute paths consistently throughout, or clearly note that policyPath must be absolute while ~/.llxprt/... is only shorthand for locating the file. Ensure lines ~206/251/261/303/358/559 no longer contradict each other.

🤖 Prompt for AI agents

Implement Phase 1: resolve documentation consistency defects so markdown lint
passes and guides no longer contradict themselves. These are self-contained doc
edits with no runtime impact.

- In `docs/cli/commands.md`, add language tags (e.g. `text`/`bash`) to the
unlabeled fenced code blocks in the At Commands section (~lines 385–388) and
Shell Passthrough section (~lines 403–407). Run a repository markdown lint pass
to catch and annotate any additional unlabeled fences introduced by the
migration PR.

- In `docs/debug-logging.md`, normalize the debug-log filename convention to
consistently state the PID-based format `llxprt-debug-<PID>.jsonl`, removing the
conflicting `debug-YYYY-MM-DD.jsonl` reference under "Can't Find Log Files?".

- Reconcile the `/mcp` scope description across `docs/cli/commands.md` and
`docs/tools/mcp-server.md` so both agree with the actual default scope
(`project`, as implemented in `packages/cli/src/commands/mcp/add.ts`); correct
whichever doc example is wrong.

- In `docs/migration/approval-mode-to-policies.md`, fix the contradictory path
examples (around lines ~206/251/261/303/358/559) by clearly distinguishing the
file *creation* path from the configured `policyPath` value. Either use absolute
paths consistently throughout the doc, or explicitly note that `policyPath` must
be absolute while `~/.llxprt/...` notation is only shorthand for locating the
file.

Phase 2: Test Maintenance

Improve test hygiene and close a genuine coverage gap without introducing mock-interaction tests, honoring the "behavioral tests only" acceptance criterion. This forms the second shippable PR.

Task 1: Replace legacy-looking fixtures in memory-tool interface tests

Move memory-tool tests onto canonical temp fixtures so they cannot touch or imply real state.

• Update packages/tools/src/tools/memoryTool.test.ts to replace fabricated /mock/project and /mock/home literals with real temp directories created via fs.mkdtemp(path.join(os.tmpdir(), ...)), matching the canonical pattern already used in packages/storage/src/secure-store/secure-store.fallback.test.ts, and clean them up in teardown.

Task 2: Gate chmod-based permission tests on Windows

Prevent the un-gated permission test from failing on platforms lacking POSIX mode semantics.

• Update packages/cli/src/config/pathMigration.freshStart.test.ts ('normalized publish preserves source file mode', ~line 489) to use it.skipIf(process.platform === 'win32'), matching the already-gated siblings in pathMigration.test.ts.

Task 3: Stabilize the null-byte error assertion (evaluate against current main)

Confirm the OAuth/secure-store null-byte assertion already uses a stable error code, or convert it if a message-based assertion is found.

• Verify the null-byte test in packages/storage/src/secure-store/secure-store.fallback.test.ts already asserts on SecureStoreError.code === 'CORRUPT' (stable code) rather than message text; document that the ticket item is satisfied. If any lock/OAuth test asserting on error.message surfaces during review, convert it to a stable code/name assertion.

Task 4: Add an outcome-based Todo continuation resolver-propagation test

Close the confirmed gap where resolver logic is validated only via in-memory mocks and fuzzy e2e text matching.

• Add a behavioral integration test targeting the runtime-wired packages/agents/src/core/TodoContinuationService.ts that seeds real on-disk todo state through TodoStore, invokes the resolver's public methods, and asserts on the resulting continuation evaluation/prompt outcome — not on constructor spies or mock call counts.
• Note the orphaned packages/cli/src/services/todo-continuation/todoContinuationService.ts for a separate follow-up decision; do not add tests to dead code.

Task 5: Evaluate a shared storage-env snapshot/restore utility

Assess whether a shared helper for saving/restoring the four LLXPRT_*_HOME category vars is warranted, and add one only if real duplication exists.

• Survey CLI and A2A extension test suites for repeated snapshot/restore of LLXPRT_CONFIG_HOME/LLXPRT_DATA_HOME/LLXPRT_CACHE_HOME/LLXPRT_LOG_HOME. If genuine duplication is found, extract a small shared test helper (e.g. under a test-utils location) and adopt it; if the only repetition is whole-process.env snapshotting (as in packages/a2a-server/src/config/config.test.ts), document that a four-category helper is unnecessary.

🤖 Prompt for AI agents

Implement Phase 2: improve test hygiene and close a genuine coverage gap, adding
only behavioral/outcome-based tests (no mock-interaction tests).

- In `packages/tools/src/tools/memoryTool.test.ts`, replace fabricated
`/mock/project` and `/mock/home` literal paths with real temp directories
created via `fs.mkdtemp(path.join(os.tmpdir(), ...))`, matching the canonical
pattern in `packages/storage/src/secure-store/secure-store.fallback.test.ts`;
clean up these directories in teardown.

- In `packages/cli/src/config/pathMigration.freshStart.test.ts`, update the
`'normalized publish preserves source file mode'` test (~line 489) to use
`it.skipIf(process.platform === 'win32')`, matching the already-gated sibling
tests in `pathMigration.test.ts`.

- Verify the null-byte test in
`packages/storage/src/secure-store/secure-store.fallback.test.ts` asserts on the
stable `SecureStoreError.code === 'CORRUPT'` rather than message text; if
already stable, document that this ticket item is satisfied. If any lock/OAuth
test found during review asserts on `error.message`, convert it to a stable
`code`/`name` assertion.

- Add a behavioral integration test against the runtime-wired
`packages/agents/src/core/TodoContinuationService.ts`: seed real on-disk todo
state through `TodoStore`, invoke the resolver's public methods, and assert on
the resulting continuation evaluation/prompt outcome (not on constructor spies
or mock call counts). Note the orphaned
`packages/cli/src/services/todo-continuation/todoContinuationService.ts` as a
separate follow-up decision item; do not add tests to this dead code.

- Survey CLI and A2A extension test suites for repeated snapshot/restore logic
around
`LLXPRT_CONFIG_HOME`/`LLXPRT_DATA_HOME`/`LLXPRT_CACHE_HOME`/`LLXPRT_LOG_HOME`.
If genuine duplication exists, extract a small shared test helper (e.g. under a
test-utils location) and adopt it across the affected suites. If the only
repetition found is whole-`process.env` snapshotting (as seen in
`packages/a2a-server/src/config/config.test.ts`), document that a dedicated
four-category helper is unnecessary.

Phase 3: Code Cleanup and Tooling Efficiency

Consolidate duplicated logic, apply the singleton, and harden guard/tooling parsing and validation — without weakening any lint, legacy-path, or GenAI-enclave guard. This forms the third shippable PR.

Task 1: Centralize duplicate Todo read/write fallback logic

Replace the two byte-identical store-resolution chains with one shared implementation.

• Add a shared store-resolution helper (static method or free function) in packages/tools/src/tools/todo-store.ts implementing the read fallback (readTodos? → getTodos? → new TodoStore(...).readTodos()) and the mirrored write fallback (writeTodos? → setTodos? → new TodoStore(...).writeTodos()), following the tools-owned facade precedent (ToolKeyStorageFacade).
• Update packages/tools/src/tools/todo-read.ts (readTodosFromStore) and packages/tools/src/tools/todo-write.ts (readOldTodos/persistTodos) to consume the shared helper.

Task 2: Introduce and consume a CoreStorageServiceAdapter singleton

Stop creating a fresh stateless adapter per registry build.

• Add a module-level exported singleton in packages/core/src/tools-adapters/CoreStorageServiceAdapter.ts.
• Update packages/core/src/config/toolRegistryFactory.ts (registerStandardTools) to import and reuse the singleton instead of calling new CoreStorageServiceAdapter() inline.

Task 3: Preserve Vitest default excludes

Ensure test-file discovery keeps Vitest's built-in excludes.

• Update packages/cli/vitest.config.ts (baseExcludePatterns) to spread configDefaults.exclude (imported from vitest/config) alongside the custom globs, matching the correct pattern in packages/agents/vitest.config.ts.
• Document that packages/a2a-server/vitest.config.ts already inherits defaults (no exclude override) and needs no change.

Task 4: Legacy-path guard hardening and allowlist narrowing

Add missing self-tests and tighten the migration allowlist/walkers, without weakening the legacy-path guard.

• Add a companion test file for packages/cli/src/config/legacyCopyEngine.ts covering the recursive filtered-copy primitives (copyDirFiltered/copyDirFilteredWithInterceptor), including symlink-cycle guarding and excluded-entry handling.
• Evaluate caching the recursive directory walks by base directory (memoizing listings within a single startup/migration pass) and implement only if it does not compromise correctness of the cycle guard.
• Review the Set-based allowlist/categorization in packages/cli/src/config/pathMigration.ts (categorizeEntry and the *_ENTRIES sets) and narrow it so unexpected/unknown entries are handled conservatively; add self-tests covering categorization boundaries.
• Confirm no misspelled DOT_LLPRT_PATTERN constant exists on current main; document the rename item as not applicable (zero repo-wide matches).

Task 5: Validate sub-unit timeout overrides in tooling helpers

Reject or normalize fractional/sub-unit timeout overrides after parsing.

• Update scripts/windows-installed-command-smoke/constants.cjs (readEnvMs) to validate parsed values are whole, sensible units (reject non-integer/sub-unit inputs, falling back to the default) rather than passing Number(raw) through verbatim.
• Update scripts/run_bun_tests.ts --timeout parsing to additionally reject fractional/sub-unit values beyond the existing Number.isFinite && > 0 check.
• Add behavioral tests (e.g. extend scripts/tests/issue-2603-smoke-fail-fast.test.ts and scripts/tests/run_bun_tests.test.ts) asserting fractional overrides are rejected/normalized.
• Verify the API-surface checker (scripts/check-agents-api-surface.mjs) uses only a hardcoded timeout with no override path; document that there is no sub-unit input to validate there.

Task 6: Harden GenAI status parsing for git rename records

Make the enclave guard's NUL parsing robust to rename (R) records so future filter changes cannot misparse.

• Update scripts/check-genai-enclave.ts discoverFiles() porcelain-status parsing so rename/copy records (two NUL tokens: R<score> <new>\0<old>\0) are consumed correctly — advancing past the second NUL token rather than treating it as the next entry's status field — without weakening the enclave guard's coverage.
• Add a behavioral test exercising rename-record parsing to lock in correct two-token consumption.

🤖 Prompt for AI agents

Implement Phase 3: consolidate duplicated logic, apply the storage adapter
singleton, and harden guard/tooling parsing and validation, without weakening
any lint, legacy-path, or GenAI-enclave guard.

- In `packages/tools/src/tools/todo-store.ts`, add a shared store-resolution
helper (static method or free function) implementing the read fallback chain
(`readTodos?` → `getTodos?` → `new TodoStore(...).readTodos()`) and the mirrored
write fallback chain (`writeTodos?` → `setTodos?` → `new
TodoStore(...).writeTodos()`), following the tools-owned facade precedent set by
`ToolKeyStorageFacade`. Update `packages/tools/src/tools/todo-read.ts`
(`readTodosFromStore`) and `packages/tools/src/tools/todo-write.ts`
(`readOldTodos`/`persistTodos`) to consume this shared helper instead of
duplicating the logic.

- In `packages/core/src/tools-adapters/CoreStorageServiceAdapter.ts`, add a
module-level exported singleton instance. Update
`packages/core/src/config/toolRegistryFactory.ts` (`registerStandardTools`) to
import and reuse this singleton instead of calling `new
CoreStorageServiceAdapter()` inline.

- In `packages/cli/vitest.config.ts`, update `baseExcludePatterns` to spread
`configDefaults.exclude` (imported from `vitest/config`) alongside the custom
globs, matching the correct pattern already used in
`packages/agents/vitest.config.ts`. Document that
`packages/a2a-server/vitest.config.ts` already inherits Vitest defaults (no
`exclude` override present) and requires no change.

- Add a companion test file for `packages/cli/src/config/legacyCopyEngine.ts`
covering the recursive filtered-copy primitives
(`copyDirFiltered`/`copyDirFilteredWithInterceptor`), including symlink-cycle
guarding and excluded-entry handling. Evaluate caching the recursive directory
walks by base directory (memoizing listings within a single startup/migration
pass), implementing this only if it does not compromise correctness of the cycle
guard. Review the Set-based allowlist/categorization logic in
`packages/cli/src/config/pathMigration.ts` (`categorizeEntry` and the
`*_ENTRIES` sets) and narrow it so unexpected/unknown entries are handled
conservatively; add self-tests covering categorization boundaries. Confirm no
misspelled `DOT_LLPRT_PATTERN` constant exists on current `main` (zero repo-wide
matches) and document that rename item as not applicable.

- Update `scripts/windows-installed-command-smoke/constants.cjs` (`readEnvMs`)
to validate parsed timeout values are whole, sensible units, rejecting
non-integer/sub-unit inputs and falling back to the default rather than passing
`Number(raw)` through verbatim. Update `scripts/run_bun_tests.ts` `--timeout`
parsing to additionally reject fractional/sub-unit values beyond the existing
`Number.isFinite && > 0` check. Add behavioral tests (extending
`scripts/tests/issue-2603-smoke-fail-fast.test.ts` and
`scripts/tests/run_bun_tests.test.ts`) asserting fractional overrides are
rejected/normalized. Verify `scripts/check-agents-api-surface.mjs` uses only a
hardcoded timeout with no override path, and document that there is no sub-unit
input to validate there.

- Update `scripts/check-genai-enclave.ts` `discoverFiles()` porcelain-status
parsing so rename/copy records (two NUL tokens: `R<score> <new>\0<old>\0`) are
consumed correctly by advancing past the second NUL token rather than treating
it as the next entry's status field, without weakening the enclave guard's
coverage. Add a behavioral test exercising rename-record parsing to lock in
correct two-token consumption.

Research

This is a TypeScript monorepo (packages/*) with supporting scripts/ and docs/. Path-migration lives in packages/cli/src/config/ (pathMigration.ts orchestration with Set-based categorizeEntry allowlists, legacyCopyEngine.ts low-level recursive copy primitives, localFsHelpers.ts), backed by the all-static Storage class in packages/storage/src/config/storage.ts (four XDG categories: config/data/cache/log via LLXPRT_*_HOME). Tool storage uses an injected-adapter pattern (ToolKeyStorageFacade in tools, CoreStorageServiceAdapter/CoreToolKeyStorageAdapter in core, wired in packages/core/src/config/toolRegistryFactory.ts). Todo tools (packages/tools/src/tools/todo-*.ts) duplicate store-resolution fallbacks. Guard tooling lives in scripts/ (check-genai-enclave.ts, Bun smoke helpers, run_bun_tests.ts). Docs relevant to migration live under docs/ and docs/migration/.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement Phase 1: resolve documentation consistency defects so markdown lint
passes and guides no longer contradict themselves. These are self-contained doc
edits with no runtime impact.

- In `docs/cli/commands.md`, add language tags (e.g. `text`/`bash`) to the
unlabeled fenced code blocks in the At Commands section (~lines 385–388) and
Shell Passthrough section (~lines 403–407). Run a repository markdown lint pass
to catch and annotate any additional unlabeled fences introduced by the
migration PR.

- In `docs/debug-logging.md`, normalize the debug-log filename convention to
consistently state the PID-based format `llxprt-debug-<PID>.jsonl`, removing the
conflicting `debug-YYYY-MM-DD.jsonl` reference under "Can't Find Log Files?".

- Reconcile the `/mcp` scope description across `docs/cli/commands.md` and
`docs/tools/mcp-server.md` so both agree with the actual default scope
(`project`, as implemented in `packages/cli/src/commands/mcp/add.ts`); correct
whichever doc example is wrong.

- In `docs/migration/approval-mode-to-policies.md`, fix the contradictory path
examples (around lines ~206/251/261/303/358/559) by clearly distinguishing the
file *creation* path from the configured `policyPath` value. Either use absolute
paths consistently throughout the doc, or explicitly note that `policyPath` must
be absolute while `~/.llxprt/...` notation is only shorthand for locating the
file.
===============================================================================

Task: 2

Implement Phase 2: improve test hygiene and close a genuine coverage gap, adding
only behavioral/outcome-based tests (no mock-interaction tests).

- In `packages/tools/src/tools/memoryTool.test.ts`, replace fabricated
`/mock/project` and `/mock/home` literal paths with real temp directories
created via `fs.mkdtemp(path.join(os.tmpdir(), ...))`, matching the canonical
pattern in `packages/storage/src/secure-store/secure-store.fallback.test.ts`;
clean up these directories in teardown.

- In `packages/cli/src/config/pathMigration.freshStart.test.ts`, update the
`'normalized publish preserves source file mode'` test (~line 489) to use
`it.skipIf(process.platform === 'win32')`, matching the already-gated sibling
tests in `pathMigration.test.ts`.

- Verify the null-byte test in
`packages/storage/src/secure-store/secure-store.fallback.test.ts` asserts on the
stable `SecureStoreError.code === 'CORRUPT'` rather than message text; if
already stable, document that this ticket item is satisfied. If any lock/OAuth
test found during review asserts on `error.message`, convert it to a stable
`code`/`name` assertion.

- Add a behavioral integration test against the runtime-wired
`packages/agents/src/core/TodoContinuationService.ts`: seed real on-disk todo
state through `TodoStore`, invoke the resolver's public methods, and assert on
the resulting continuation evaluation/prompt outcome (not on constructor spies
or mock call counts). Note the orphaned
`packages/cli/src/services/todo-continuation/todoContinuationService.ts` as a
separate follow-up decision item; do not add tests to this dead code.

- Survey CLI and A2A extension test suites for repeated snapshot/restore logic
around
`LLXPRT_CONFIG_HOME`/`LLXPRT_DATA_HOME`/`LLXPRT_CACHE_HOME`/`LLXPRT_LOG_HOME`.
If genuine duplication exists, extract a small shared test helper (e.g. under a
test-utils location) and adopt it across the affected suites. If the only
repetition found is whole-`process.env` snapshotting (as seen in
`packages/a2a-server/src/config/config.test.ts`), document that a dedicated
four-category helper is unnecessary.
===============================================================================

Task: 3

Implement Phase 3: consolidate duplicated logic, apply the storage adapter
singleton, and harden guard/tooling parsing and validation, without weakening
any lint, legacy-path, or GenAI-enclave guard.

- In `packages/tools/src/tools/todo-store.ts`, add a shared store-resolution
helper (static method or free function) implementing the read fallback chain
(`readTodos?` → `getTodos?` → `new TodoStore(...).readTodos()`) and the mirrored
write fallback chain (`writeTodos?` → `setTodos?` → `new
TodoStore(...).writeTodos()`), following the tools-owned facade precedent set by
`ToolKeyStorageFacade`. Update `packages/tools/src/tools/todo-read.ts`
(`readTodosFromStore`) and `packages/tools/src/tools/todo-write.ts`
(`readOldTodos`/`persistTodos`) to consume this shared helper instead of
duplicating the logic.

- In `packages/core/src/tools-adapters/CoreStorageServiceAdapter.ts`, add a
module-level exported singleton instance. Update
`packages/core/src/config/toolRegistryFactory.ts` (`registerStandardTools`) to
import and reuse this singleton instead of calling `new
CoreStorageServiceAdapter()` inline.

- In `packages/cli/vitest.config.ts`, update `baseExcludePatterns` to spread
`configDefaults.exclude` (imported from `vitest/config`) alongside the custom
globs, matching the correct pattern already used in
`packages/agents/vitest.config.ts`. Document that
`packages/a2a-server/vitest.config.ts` already inherits Vitest defaults (no
`exclude` override present) and requires no change.

- Add a companion test file for `packages/cli/src/config/legacyCopyEngine.ts`
covering the recursive filtered-copy primitives
(`copyDirFiltered`/`copyDirFilteredWithInterceptor`), including symlink-cycle
guarding and excluded-entry handling. Evaluate caching the recursive directory
walks by base directory (memoizing listings within a single startup/migration
pass), implementing this only if it does not compromise correctness of the cycle
guard. Review the Set-based allowlist/categorization logic in
`packages/cli/src/config/pathMigration.ts` (`categorizeEntry` and the
`*_ENTRIES` sets) and narrow it so unexpected/unknown entries are handled
conservatively; add self-tests covering categorization boundaries. Confirm no
misspelled `DOT_LLPRT_PATTERN` constant exists on current `main` (zero repo-wide
matches) and document that rename item as not applicable.

- Update `scripts/windows-installed-command-smoke/constants.cjs` (`readEnvMs`)
to validate parsed timeout values are whole, sensible units, rejecting
non-integer/sub-unit inputs and falling back to the default rather than passing
`Number(raw)` through verbatim. Update `scripts/run_bun_tests.ts` `--timeout`
parsing to additionally reject fractional/sub-unit values beyond the existing
`Number.isFinite && > 0` check. Add behavioral tests (extending
`scripts/tests/issue-2603-smoke-fail-fast.test.ts` and
`scripts/tests/run_bun_tests.test.ts`) asserting fractional overrides are
rejected/normalized. Verify `scripts/check-agents-api-surface.mjs` uses only a
hardcoded timeout with no override path, and document that there is no sub-unit
input to validate there.

- Update `scripts/check-genai-enclave.ts` `discoverFiles()` porcelain-status
parsing so rename/copy records (two NUL tokens: `R<score> <new>\0<old>\0`) are
consumed correctly by advancing past the second NUL token rather than treating
it as the next entry's status field, without weakening the enclave guard's
coverage. Add a behavioral test exercising rename-record parsing to lock in
correct two-token consumption.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!