Skip to content

refactor(coding-agent): remove the test-only daemon-lookup seam from main.ts - #1738

Closed
snimu wants to merge 7 commits into
snimu/remove-overflow-patterns-exportfrom
snimu/remove-daemon-lookup-fake
Closed

refactor(coding-agent): remove the test-only daemon-lookup seam from main.ts#1738
snimu wants to merge 7 commits into
snimu/remove-overflow-patterns-exportfrom
snimu/remove-daemon-lookup-fake

Conversation

@snimu

@snimu snimu commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What was wrong

main.ts's interactive-startup wrapper accepted an optional injected daemon-lookup function that only three unit tests ever passed — a test-only seam in production code. Those tests asserted the wrapper's own try/catch branches through the fake, i.e. they restated the implementation (audit: test-only production code, test-only.md finding 5).

The fix

+2/−48: the lookup type and option are deleted, findActiveDaemonSessionSummary is called directly, the three fake-lookup tests and their import are gone, and the function's now-unimported export keyword is dropped. The real fallbackOnError policy and its production caller are byte-identical.

How it's verified

Reviewer confirmed no other caller passed the option, the deleted tests covered no user-relevant obligation, and the remaining 52 routing tests pass. tsgo/biome clean; full CI-style suite failure set identical to stack base (two unrelated full-load flakes pass focused on both branches). Two-model implement/review loop, approved first pass.

Stacked on #1737 (test the whole stack at the leaf; merge base-first).

Note: intentionally no Linear ticket for this cleanup stack, so that check stays red.


Note

Low Risk
Internal refactor with no change to production lookup or error-fallback policy; only breaks out-of-tree imports of the formerly exported wrapper.

Overview
Removes the test-only injection point for interactive startup daemon active-session lookup: the optional lookup callback and its type alias are deleted, and findActiveDaemonSessionSummaryForInteractiveStartup always calls findActiveDaemonSessionSummary directly.

The wrapper is now internal (no longer exported). fallbackOnError behavior and the production caller that passes { fallbackOnError: !publicCommand.attachAgent } are unchanged.

Three unit tests that stubbed lookup to exercise the wrapper’s try/catch are removed; remaining routing tests still cover the exported decision helpers.

Reviewed by Cursor Bugbot for commit e290652. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Remove test-only daemon-lookup override seam from main.ts

Removes the ActiveDaemonSessionSummaryLookup type alias and the optional lookup property from ActiveDaemonSessionSummaryLookupOptions in main.ts. The function findActiveDaemonSessionSummaryForInteractiveStartup is no longer exported and always calls findActiveDaemonSessionSummary directly. Deletes three tests in main-interactive-routing.test.ts that depended on injecting a custom lookup.

  • Risk: findActiveDaemonSessionSummaryForInteractiveStartup is now internal; any out-of-tree importers will break. The lookup option is silently ignored if still passed.

Macroscope summarized e290652.

Linear ticket: ENG-5658
(ticket linked above)

@snimu

snimu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #1824: GitHub's stack lock prevents retargeting stacked PRs, so this PR was recreated as a plain PR against main (same branch, same content, all review threads here are resolved). Please review/merge #1824 instead.

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.

1 participant