[vitalijssilins]

Fixes #468.

The broker retained connection identities indefinitely, so 256 past caller environments could prevent all new sessions. At the limit it now retires the least recently used transport with no queued or active call, preferring already retired entries and failed connections. It rechecks eligibility on the existing serialized retirement path and preserves browser owners, unknown outcomes, and failed retirements.

This deliberately permits reclaiming a quiet live session before its configured idle deadline, including sessions without an idle timeout. Its next call reconnects and may lose server-side session state. Below the limit, retention is unchanged. The daemon guide and Unreleased changelog document that tradeoff.

Validation: capacity regressions cover early retirement only at capacity, LRU choice, concurrent admission, rejoining callers, failed connects and retirements, and unknown outcomes. pnpm check passed. The broad Linux test run passed 2,364 tests; eight native-build tests failed because the test machine had Bun 1.4.0. Installing the repository-required Bun 1.4.4 and rerunning the two affected files passed all 25 tests. Cross-platform CI must pass at the final head before merge.

Original implementation and regression coverage by @vitalijssilins; contributor credit is retained in the squash commit and changelog.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[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-11-05T17:37:24.530880Z	c3fcd6e	New commits

ℹ️ 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.

[clawsweeper]

Codex review: blocked before merge.

What this changes

This PR reclaims unused daemon transports at the 256-transport limit so new caller identities can connect.

Example: 260 CLI callers use one keep-alive HTTP server with distinct CALLER_SESSION_ID values.

• Before: Callers 257–260 fail with transport_limit.
• After: All 260 callers succeed while the broker retains at most 256 transports.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Convincing CLI proof and focused lifecycle handling support the implementation; the unresolved retention-policy decision caps overall readiness.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The inspected terminal image, pinned script and raw log exercise compiled mcporter call against the real HTTP fixture in a private daemon on macOS with Node 24.16.0: all 260 distinct-environment callers succeed after reclamation while transport count remains 256; subsequent changes restore diagnostic guidance without altering reclamation.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Needs a maintainer decision · Fix scope: Complete
User problem: New agent sessions eventually cannot call keep-alive servers because abandoned transport identities permanently exhaust daemon capacity.
Reason: The reliability benefit is demonstrated, but connected-session eviction changes the established retention contract without recorded owner approval.

Merge readiness

⛔ Blocked before merge - 2 items remain

Keep open: this PR addresses a verified capacity failure still present on main, and the earlier diagnostic finding is resolved; approval of the retained-session eviction policy remains outstanding.

Priority: P1
Reviewed head: 6d3af151ddd27d42525d7bb6210e176585fd9701
Owner decision: Required. See Decision needed.

Decision needed

• Question: May capacity pressure retire a quiet but still-live caller's retained MCP session before its configured idle timeout, including when no idle timeout exists?
• Recommendation: Approve bounded pressure eviction: Accept the documented session reset at capacity while preserving protections for active work, uncertain outcomes, Chrome owners and failed retirements.
• Why: This changes the owner-established retention contract, and the contributor's explicit tradeoff explanation does not constitute maintainer approval.

Before merge

• Await maintainer action - The broker feature owner must resolve the retained-session eviction policy described in the maintainer decision.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

None.

Agent review details

How this fits together

The daemon broker receives view-authorized MCP requests, selects a retained transport by connection identity, and serializes calls and retirement through the runtime.

flowchart LR
 A[CLI caller] --> B[Configuration view]
 B --> C[Daemon broker]
 C --> D[Capacity admission]
 D --> E[Serialized retirement]
 D --> F[Retained runtime]
 E --> F
 F --> G[MCP server]

Loading

Technical review

Best possible solution:

Use owner-approved capacity reclamation through the existing serialized retirement path while preserving connection identity and protection for active or uncertain work.

Do we have a high-confidence way to reproduce the issue?

Current-main source establishes permanent slot accumulation and rejection of identity 257, corroborated by inspected contributor CLI output; no target code was executed during this read-only review.

Is this the best way to solve the issue?

The implementation addresses capacity accumulation through existing broker serialization and verified retirement, but evicting retained live sessions requires the area owner's policy decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 2874448832eb.

Provenance checked

• Broker admission and retained-session lifecycle changes intended behavior with a stated reason (95edea1: The singleton broker bounds resources while preserving shared MCP sessions, environment isolation, serialized calls and verified retirement.)
• Daemon lifecycle documentation changes intended behavior with a stated reason (95edea1: Retained connections survive ordinary caller exits and retire through configured idle expiry or explicit recovery; the PR explains capacity pressure as an additional retirement trigger.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Current main still exhausts transport capacity: Main rejects new identities at 256 entries; idle retirement retains entries, and releasing or expiring configuration views does not remove their transports. (src/daemon/broker.ts:172, 2874448832eb)
• Reclamation uses the existing retirement owner: The introduced code excludes queued or active calls, Chrome owners, uncertain outcomes and failed retirements, rechecks eligibility on the entry's serial chain, and removes entries only after successful retirement. (src/daemon/broker.ts:408, 6d3af151ddd2)
• Previous finding resolved: The revised capacity error tells callers to retry after active calls finish and deliberately drain transports with daemon stop if exhaustion persists; documentation and the in-flight regression test were updated. (src/daemon/broker.ts:183, 6d3af151ddd2)
• Retained-session contract and feature history: The merged fix(daemon): share one user daemon across configurations #346 established shared retained sessions, environment isolation, serialized calls and verified retirement; subsequent broker and daemon history continues to identify Peter Steinberger as the central area contributor. (src/daemon/broker.ts:172, 2874448832eb)
• Inspected shipped CLI proof: The downloaded terminal image and pinned demo.log show main refusing callers 257–260 and the demonstrated PR build serving all 260 at 256 transports; demo.sh invokes the compiled CLI against the real HTTP fixture in a private daemon. (proof/demo.sh:1, 60b445bfdb1a)
• Discussion leaves the policy choice unresolved: The author explicitly acknowledges retirement of quiet but still-live callers, including servers without idle timeouts, and offers to gate it differently; no maintainer approval is recorded in the current discussion or reviews.

Likely related people:

• Peter Steinberger: Raw commit 95edea1 adds src/daemon/broker.ts:157 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 95edea12d715; files: src/daemon/broker.ts)

Review metrics

Metric	Value	Why it matters
Production and regression coverage	Production +63/-3 lines; tests +293 lines across 10 cases	The larger test contribution covers distinct admission, ordering, concurrency and retirement-safety failures.

Root-cause cluster

Relationship: same_root_cause
Canonical: #468
Summary: This PR implements the transport-capacity repair requested by the canonical report.

Members:

• canonical: Daemon refuses new callers once 256 transports pile up from callers that are gone #468 - The report identifies the same retained-entry accumulation and 256-transport admission failure.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Labels

Label changes:

• add status: ⏳ waiting on maintainer: Only explicit maintainer approvals or decisions remain; merge is still blocked.
• remove status: ⏳ waiting on author: Current PR status label is status: ⏳ waiting on maintainer.

Label justifications:

• P1: Accumulated transports block new agent sessions on a long-lived daemon until an operator drains or restarts it.
• merge-risk: 🚨 session-state: Pressure eviction can discard a quiet live caller's retained session before its configured idle deadline or when no deadline exists.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: ⏳ waiting on maintainer: Only explicit maintainer approvals or decisions remain; merge is still blocked.
• proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)

• reviewed 2026-10-26T09:01:15.743Z sha 5b7304c :: blocked before merge. :: [P3] Restore recovery guidance in the capacity error

Reviewed October 26, 2026, 5:26 AM ET / 09:26 UTC (Revision 2).

[vitalijssilins]

Addressed the P3 in 6d3af15: the transport_limit message keeps the recovery steps ("Retry after active calls finish; if it persists, drain transports with daemon stop."), docs/daemon.md says the same, and the in-flight test asserts the guidance. pnpm check clean, pnpm test 2288 passed.

On the session-retention question: reclaiming only happens at the 256 limit, never touches active, queued, unknown-outcome, Chrome-owned or failed-retirement transports, and the alternative today is that every new caller identity is refused until a manual restart. Happy to gate it differently if you prefer.

@clawsweeper re-review

[clawsweeper]

🦞👀
Exact review queued.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/40299703157
• Updated: 2026-10-26T09:27:56.535Z

[steipete]

Refreshed this branch onto current main and moved its release note into Unreleased. The capacity regressions reproduce on the old broker; the proposed implementation passes all 10 targeted cases and the complete Linux gate (2,302 tests). Independent review found no actionable implementation issue after checking the reported authorization-order concern against the marker-only helper.

Leaving this open for the retained-session policy decision identified above. At capacity, the proposal may reset a quiet but still-live caller's session before its configured idle deadline, including when no deadline is configured. This round does not resolve that product-policy hold, so #468 remains open as well.

[vitalijssilins]

@steipete Thanks for refreshing and validating this. I propose narrowing this PR so it does not introduce early eviction of retained sessions.

The remaining cleanup bug is that configured idle retirement closes the transport and marks its entry idle, but leaves the entry in the map counted against the 256 limit. We have prepared explicit idle timeouts in our consumer configs; those timeouts alone cannot recover capacity.

Proposed scope:

• Reclaim entries whose transports have already retired successfully under their configured idle timeout.
• Reclaim failed-connect entries only after cleanup is verified.
• Never retire a connected session early, including one with no idle timeout. Preserve the protections for queued/active work, connects in progress, Chrome ownership, unknown outcomes, and failed retirement. Recheck eligibility on the serialized lifecycle path before removing an entry.

This would be a partial fix for #468. It would not resolve the original case where all 256 transports remain connected indefinitely, and the current 260-caller proof would no longer demonstrate the proposed behavior. I would replace Fixes #468 with Related to #468, update the description and tests to show capacity reuse after permitted retirement, and explicitly test that 256 retained connected sessions still reject a new identity.

To keep the review clear, I suggest revising this PR in place, resolving its conflicts, and leaving pressure eviction for a separate policy discussion rather than opening a competing implementation now. Does this narrower scope work for you? No implementation change has been pushed with this proposal.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c3fcd6edd6

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Refresh eviction priorities after an awaited retirement

At capacity, if the first candidate's retirement waits and the entry is subsequently kept because a caller rejoins it—or retirement fails—later candidates can change priority during that wait. For example, another entry's idle timer can finish retiring it, but this loop still closes the next connected entry from the original snapshot instead of reclaiming the now-idle entry; a completed call that updates lastUsed is likewise ignored. This can unnecessarily destroy a live server session despite an already-retired or older unused entry being available. Re-scan the candidates after an awaited unsuccessful reclamation rather than continuing through the stale ordering.

Useful? React with 👍 / 👎.