fix(eval): close metering admission at child exit

[me2seeks]

Summary

The external-subject metering proxy stayed open for new admissions after the child process had exited while runChild() was still draining stdout and stderr. A leftover descendant or established keep-alive connection could issue another provider request in that window, so the supposedly post-exit request was counted instead of refused.

This closes proxy admission synchronously from the child exit/error boundary. report() and close() reuse the same idempotent transition, then drain only requests admitted before that cut.

The failure is reproducible on current main; PR #3208's otherwise unrelated workspace lane failed this exact regression with 2 !== 1.

Verification

• focused provider-admission regression: 50/50 consecutive passes
• npm --workspace @maka/eval run build
• npm --workspace @maka/eval run test:dist — 75/75 Node tests; all Python Harbor suites pass (4 environment-dependent cases skipped as expected)
• npx biome check packages/eval/src/harbor-external-subject.ts
• git diff --check
• simplify-audit: no P0-P3 candidates; accepting remains the single admission authority

AI disclosure: Codex implemented and verified this change under me2seeks's direction and review.

[coderabbitai]

Warning

Review limit reached

@me2seeks, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Limit details: You’ve used all 3 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 41b01266-d0da-4dfb-92ac-cfa36c9b1a44

📥 Commits

Reviewing files that changed from the base of the PR and between 444a0bb and 8cca51e.

📒 Files selected for processing (1)

• packages/eval/src/__tests__/provider-admission-integration.test.ts

📝 Walkthrough

Problem solved

The PR closes external-subject metering admission when the child process exits or errors. This prevents leftover descendants and keep-alive connections from issuing provider requests during output draining.

report() and close() use the same idempotent transition. They drain only requests admitted before the cutoff.

Design and scope

The PR extends the existing child-process and metering-proxy flow. It does not create a parallel admission path or a new public API.

runChild invokes one exit callback for both process errors and exits. The proxy then calls stopAccepting before usage reporting and request draining. This is the smallest coherent solution because it places the cutoff at the child lifecycle boundary and reuses it for both shutdown paths.

The integration test replaces a fixed-delay race with a wrapper-state barrier. This makes the regression deterministic without weakening coverage.

No code or tests are apparent deletion candidates. The new synchronization and idempotent transition are required to preserve the post-exit admission regression coverage.

Validation and risks

The regression was reproduced on main, where a post-exit request was incorrectly counted. Focused tests passed 50/50. Eval build and distribution tests, Biome checks, git diff --check, and a simplify audit also ran. The simplify audit found no P0–P3 candidates.

The main risk is incorrect ordering between child exit, admission cutoff, request settlement, and output draining. The new callback and barrier-based test directly cover this ordering.

Complexity delta

• Authorities: The child lifecycle remains the authority for admission closure. No parallel authority was added.
• States: The proxy gains an idempotent stopped-accepting transition.
• Branches: Error and exit paths share one callback, which limits branch duplication.
• Configuration: No configuration was added.
• Public surface: No exported or public declarations changed.
• Test maintenance: The test replaces timing-based coordination with explicit state-file coordination. This adds synchronization code but removes flakiness from the fixed-delay race.

Total maintenance complexity stays justified. The added state and synchronization are necessary to enforce and verify the admission cutoff.

Review-relevant risks

The current diff affects provider-request admission and usage metering behavior after child-process exit. This can affect billing or usage accounting. Material changes in this area require independent human review under repository policy.

No public contract, security, licensing, release, or governance effect was identified in the current diff. Required-check status remains unverified unless direct evidence is available. The person performing the merge must review the final diff, and a maintainer makes the final determination.

Walkthrough

The change adds explicit metering-proxy admission shutdown on child error or exit. The integration test waits for the child-exit state before issuing a late request, replacing a fixed delay.

Changes

Provider admission lifecycle

Layer / File(s)	Summary
Proxy admission and child exit lifecycle packages/eval/src/harbor-external-subject.ts	runChild invokes an exit callback once for error or exit events. The callback stops proxy admission before usage reporting, draining, or closure.
Admission shutdown integration test packages/eval/src/__tests__/provider-admission-integration.test.ts	The straggler receives the wrapper-state path and waits for child_exited before issuing its late request.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to 444a0

The production change closes provider admission at child exit, but the regression test can wait indefinitely when the detached child errors, potentially hanging CI and output draining. Merge readiness requires bounding that wait and cleaning up the straggler, or explicitly accepting the risk.

Sequence Diagram(s)

sequenceDiagram
  participant ChildProcess
  participant runChild
  participant MeteringProxy
  participant Straggler
  ChildProcess-->>runChild: exit or error
  runChild->>MeteringProxy: stopAccepting()
  runChild->>MeteringProxy: report usage and drain
  Straggler->>MeteringProxy: late request after child_exited
  MeteringProxy-->>Straggler: reject new admission

Loading

Suggested reviewers: hqhq1025, astro-han, 1625567290

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Ai Use Disclosure	⚠️ Warning	The description names Codex but selects neither required declaration; commit 4990 has Generated-by: Maka, which is undisclosed, and commit 444 has no trailer.	Select the generative-tooling declaration, name Codex and its scope, and add matching trailers to affected commits so they survive squash or amend. See “Human ownership and AI attribution” in CONTRIBUTING.md.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the main change: closing metering admission when the child exits.
Description check	✅ Passed	The description clearly explains the problem, implementation, regression, and verification results, with an AI-use disclosure.
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 unit tests (beta)

• Create PR with unit tests

---

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

[qodo-code-review]

PR Summary by Qodo

Close metering admission at child process exit

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Close metering proxy admission immediately when child processes exit or fail.
• Drain only pre-exit requests before producing settled usage and cost snapshots.
• Reuse one idempotent admission transition across exit, reporting, and cleanup.

Diagram

sequenceDiagram
  actor Child as Child Process
  participant Runner as runChild
  participant Proxy as Metering Proxy
  actor Descendant as Descendant Client
  participant Provider as Upstream Provider
  Child->>Proxy: admitted request
  Proxy->>Provider: forward request
  Child-->>Runner: exit or error
  Runner->>Proxy: stopAccepting
  Descendant->>Proxy: late request
  Proxy-->>Descendant: 503 refused
  Runner->>Proxy: report
  Provider-->>Proxy: finish in-flight work
  Proxy-->>Runner: settled snapshot

Loading

High-Level Assessment

The PR's approach is appropriate: the child exit/error event is the earliest authoritative cutoff, and routing it through the proxy's idempotent stopAccepting transition preserves one admission authority. Deferring closure to report() leaves the race intact, while closing the HTTP server alone would not reject requests over established keep-alive connections and could wait indefinitely on idle sockets.

Files changed (1) +17 / -4

Bug fix (1) +17 / -4

harbor-external-subject.tsClose proxy admission at the child termination boundary +17/-4

Close proxy admission at the child termination boundary

• Passes the proxy's admission-stop callback into runChild and invokes it synchronously for both child error and exit events. Exposes one idempotent stopAccepting transition reused by reporting and cleanup before draining previously admitted requests.

packages/eval/src/harbor-external-subject.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Astro-Han]

The production fix is at the right authority and is smaller than alternative socket or reporting changes: admission closes synchronously in the child error/exit callbacks, while already-admitted requests still drain into the settled snapshot. I found no production-code regression. The remaining P2 is in the verification layer: the existing regression test schedules the purported post-exit request with a blind 300 ms timer, so loaded CI can still send it before the child exit callback and reproduce 2 !== 1 even with this fix. Because this PR is specifically a flake fix, the regression should pin the exit boundary deterministically.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified the exact head 4990f314f4324848e6a0eacc307651f9cc3abff0, child/proxy event ordering, the existing provider-admission integration test, and current CI. No local tests were run in this review.

中文审查

生产代码修复落在正确责任层，也比关闭 socket 或推迟到 report 的方案更简单：child 的 error/exit 回调同步停止 admission，已经接收的请求继续 drain 到最终快照。未发现生产代码回归。剩余 P2 在验证层：现有测试用固定 300ms 定时器制造“退出后”请求，CI 调度变慢时它仍可能在 child exit callback 前发送，从而让修复后的代码继续出现 2 !== 1。这个 PR 本身就是 flake fix，因此回归测试应确定性锁定 exit boundary。

本次为 AI 辅助审查：Codex 协调两轮独立 reviewer 审查及 OpenCode Go DeepSeek V4 Flash high 对抗性复核，并核验了精确 head、child/proxy 事件顺序、现有 provider-admission integration test 和当前 CI。本轮未运行本地测试。

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 97512b06-5480-4ae3-9b70-4db118761407

📥 Commits

Reviewing files that changed from the base of the PR and between 62556ab and 444a0bb.

📒 Files selected for processing (2)

• packages/eval/src/__tests__/provider-admission-integration.test.ts
• packages/eval/src/harbor-external-subject.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.

[Astro-Han]

Approved. Child exit/error is the earliest truthful metering-admission boundary: stopAccepting closes the single proxy authority synchronously before the child_exited barrier is published, while already admitted requests remain in the active set and drain into the final report. The settled guard deduplicates error/exit, and report/close reuse the same idempotent transition.

The latest test no longer guesses with a fixed delay: it waits for the child-exit barrier, proves an in-flight request is retained, and proves a post-exit request is rejected. Current head is clean against main and all required checks are green.

AI-assisted review by Codex with two independent reviewers and OpenCode Go DeepSeek V4 Flash (high); I verified lifecycle ordering, late/in-flight requests, error cleanup, test discrimination, and live CI.

中文

批准。child exit/error 是最早且真实的 metering admission 边界：stopAccepting 在发布 child_exited barrier 前同步关闭唯一 proxy authority；已受理请求仍保留在 active 集合中并进入最终 report。settled guard 去重 error/exit，report/close 复用同一个幂等 transition。

最新测试不再依赖固定延迟，而是等待 child-exit barrier，同时证明在途请求保留、exit 后请求拒绝。当前 head 与 main 干净合并，必需检查全绿。

本次由 Codex、两个独立 reviewer 和 OpenCode Go DeepSeek V4 Flash high 辅助；已核对生命周期顺序、late/in-flight requests、错误清理、测试判别力和实时 CI。