fix(discovery): keep eval imports and repeated discovery off the agent run path

[kwakayama]

Fixes veryfront/veryfront-issue-inbox#1620.

Problem

Every agent run turn for agentic-email-processing-outlook spent ~10s between Accepted internal agent stream request and Starting internal agent runtime stream. Each turn logged Primitive discovery completed with errors (agents 5, errors 12), and every parent resume after an invoke_agent park paid it again.

Two causes in ensureProjectDiscovery:

1. Eval modules are imported on the run path. Runs and resumes never use evals, and eval execution discovers them itself (src/eval/discovery.ts). This project's evals/mock-tools.ts reads knowledge/email-categories.md by a CWD-relative path at import time, so 12 eval imports fail with ENOENT wherever the CWD isn't the project root.
2. Any per-file error evicts the discovery cache. A production release with a persistent error therefore rediscovers on every run and resume. In production that goes through the proxy file system, which is where the ~10s comes from.

Local reproduction

I ran the production discovery path (ensureProjectDiscovery, release-scoped, real local adapter) three times against the actual project source, with the CWD outside the project:

	pass 1	pass 2	pass 3
before	1,021 ms, 5 agents, 12 errors (ENOENT … 'knowledge/email-categories.md' from evals/*.eval.ts)	150 ms, 12 errors (rediscovered)	156 ms, 12 errors (rediscovered)
after	334 ms, 5 agents, 0 errors	0 ms (cached)	0 ms (cached)

Locally a rediscovery costs ~150 ms. In production each one reads the project through the proxy file system.

Change

• The run path calls discoverAll with evalDirs: []. The configured evalDirs are unchanged, so browser module admission and hosted executor roots still treat evals/ as server-only.
• A production release (release ID set) keeps a result with errors for PRODUCTION_DISCOVERY_ERROR_RETRY_MS (60s) instead of evicting it. A release can't change, so a defect costs one discovery per window, and a transient read failure is still retried after the window. Mutable sources (preview) keep retrying immediately; the existing "retries partial discovery within the same source snapshot" test still passes.

Tests

• New: does not import eval modules on the run path; reuses a production release discovery with errors until its retry window ends (clock injected via __setProjectDiscoveryClockForTests).
• Each new test fails with its half of the fix removed.
• project-discovery.test.ts passes (27 steps), and so do project-run-execute.handler, api-handler-wrapper, agent-stream.handler, discovery/index, auto-discovery.integration and eval/discovery. deno check, deno lint and deno fmt are clean on the changed files.

[chatgpt-codex-connector]

You have reached your Codex usage limits for security reviews. Please try again later.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[coderabbitai]

Warning

Review limit reached

Next included review available in 45 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 7ee41b2d-d152-4987-9fa5-660da0202230

📥 Commits

Reviewing files that changed from the base of the PR and between ce61f60 and b15292a.

📒 Files selected for processing (2)

• src/server/handlers/request/api/project-discovery.test.ts
• src/server/handlers/request/api/project-discovery.ts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-22T06:36:09.452730Z	b15292a	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

📦 Client bundle boundary

Entrypoint	Modules	Source size	Server leaks
src/index.client.ts	291	2329 KiB	✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

[gitar-bot]

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

_Gitar

[kwakayama]

Code Review

Score: 87/100 — Good: a well-scoped, well-reasoned fix with meaningful regression tests; a couple of minor polish points, no blockers.

Strengths

• The root-cause analysis holds up: discoverAll's helper does dirs ?? defaultDirs (discovery-engine.ts:244), so passing evalDirs: [] (not undefined) actually skips the evals/ directory instead of silently falling back to the default. Verified evalHandler (discovery/handlers/eval-handler.ts) has no side effect beyond populating result.evals, so dropping eval discovery from the run path is safe — nothing else in the run path reads that map.
• Confirmed the "browser module admission / hosted executor roots still treat evals/ as server-only" claim: both browser-module-admission.ts and agent/hosted/executor-discovery-roots.ts call createProjectDiscoveryConfig independently, so they're unaffected by the local evalDirs: [] override in project-discovery.ts.
• The new retryAt cache-eviction logic is correct at the boundary: now() < existing.retryAt is strict, so the new test correctly asserts a cache hit at retryAt - 1 and a miss exactly at retryAt. The test injects a real clock hook (__setProjectDiscoveryClockForTests) rather than faking timers, which keeps it deterministic without being brittle.
• Both new tests are meaningful regression tests (PR description states each fails with its half of the fix reverted) rather than tests that just restate the implementation.
• PR description is excellent: clear problem statement, real local repro numbers, and an explicit list of what was re-run to check for regressions.

Minor points (non-blocking)

• shouldCacheCompletedDiscovery(ctx) is called twice (once for cacheCompletedDiscovery at the top, again in the error branch after the promise resolves). Since ctx doesn't mutate mid-call these return the same result, but hoisting to a single local would make the two related-but-distinct booleans easier to follow.
• __setProjectDiscoveryClockForTests mutates a module-level let now. Fine for this test file's sequential it blocks with its try/finally restore, but worth double-checking there's no parallel-test-runner scenario in this suite where two tests could interleave and clobber each other's clock.
• No new test explicitly re-exercises the "mutable/preview source retries immediately" branch against the new retryAt field together (the PR description says the pre-existing test for that still passes, which is a reasonable way to cover it, just noting it wasn't extended).

Nothing here blocks merge — this is a solid, surgical fix for a real production latency issue with good test coverage.

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[kwakayama]

Score: 93/100

Reviewed exact head: b15292a

Why:

• Correctly removes eval discovery from the request-time agent run path while preserving the explicit eval execution path.
• Preserves release-scoped production caching and adds a bounded retry window for completed discoveries with partial errors.
• Regression coverage verifies both no eval imports and retry-window behavior, including the expiry boundary.
• Focused validation passed: deno task test:file src/server/handlers/request/api/project-discovery.test.ts (27 steps).
• Diff check passed and there are no unresolved review threads.
• Required PR checks are green at this exact head.

Minor deduction: the module-level test clock seam is global state, so future parallelization of this suite would need care. This is test-only risk and does not block merge.

Recommendation: merge.