fix(config): allow explicit main agent bindings when agents.list is non-empty#89419
fix(config): allow explicit main agent bindings when agents.list is non-empty#894191052326311 wants to merge 1 commit into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 3:09 PM ET / 19:09 UTC. Summary PR surface: Source +12, Tests +82. Total +94 across 6 files. Reproducibility: yes. at source level: a populated Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the three-boundary compatibility repair together, retaining exact-ID validation so only the reserved implicit Do we have a high-confidence way to reproduce the issue? Yes, at source level: a populated Is this the best way to solve the issue? Yes. Treating only the exact existing AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cadad3b7bd12. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +12, Tests +82. Total +94 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
edcb95b to
b3bb2cf
Compare
|
@clawsweeper re-review — updated proof with live before/after runtime verification showing 'main' agent binding rejection is fixed. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
…on-empty (openclaw#89419) Treat DEFAULT_AGENT_ID ('main') as a valid explicit binding target in both config schema validation and doctor binding pruning. Previously route bindings to agentId 'main' were rejected when agents.list was non-empty, breaking the documented multi-agent config pattern. Added regression tests for explicit main accepted and unknown agentId still rejected.
b3bb2cf to
047d736
Compare
|
Updated with regression tests:
The fix remains minimal (+5 lines source, +30 lines test). This addresses the proof/regression concerns. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
…w#89419 v2) Schema and doctor pruning now use raw string comparison (agentId === DEFAULT_AGENT_ID) instead of normalized comparison. This ensures only the exact literal 'main' bypasses the unknown-agent check — variants like 'MAIN', ' main ', or 'Main' are still rejected. Added: - Strict variant rejection test (schema: MAIN/ main /Main → rejected) - Doctor regression test (preserves 'main', prunes 'MAIN') - Doctor 'normalize-equivalent variant' test
047d736 to
893bfb8
Compare
|
v2: raw-id guard for explicit main agent bypass. P1 addressed — Tighten schema exception:
P1 addressed — Doctor raw-id guard:
New tests:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
…w#89419 v2) Schema and doctor pruning now use raw string comparison (agentId === DEFAULT_AGENT_ID) instead of normalized comparison. This ensures only the exact literal 'main' bypasses the unknown-agent check — variants like 'MAIN', ' main ', or 'Main' are still rejected. Added: - Strict variant rejection test (schema: MAIN/ main /Main → rejected) - Doctor regression test (preserves 'main', prunes 'MAIN') - Doctor 'normalize-equivalent variant' test
893bfb8 to
266c56a
Compare
|
v3: fixed test file regression (restored full 3151-line doctor test suite), added 2 doctor regression tests, tightened schema to raw-id guard. Critical fix: Previous force push accidentally replaced the full 3151-line Changes summary:
Net diff: +105, -4 (4 files) @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
v5 fix (commit 1045e21): addressed sweeper P1 predicate-symmetry issue. What changed in doctor:
Schema and route resolution unchanged (already correct in v4). @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
1045e21 to
e75eb86
Compare
e75eb86 to
4ca154b
Compare
|
Rebuilt this fix as a single commit on current Current-head behavior covered:
Verification on head
Current-head production-function proof (schema + doctor repair + route resolution): {
"schemaAcceptedMain": true,
"schemaRejectedGhost": true,
"doctorBindings": ["main"],
"doctorChanges": [
"Removed 1 binding that referenced missing agents.list ids."
],
"routedAgentId": "main",
"routedSessionKey": "agent:main:main",
"matchedBy": "binding.account"
}This was produced directly by @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
4ca154b to
94b448e
Compare
|
Rebased onto current |
94b448e to
8516414
Compare
|
Rebased cleanly onto latest @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
8516414 to
764c465
Compare
|
Rebased onto @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Latest exact-head CI is complete: all 30 jobs passed; the branch is mergeable and current-head schema/doctor/routing proof is in the PR body. The prior review lease for |
764c465 to
5373df4
Compare
|
Rebased cleanly onto current |
Closes #89412
What Problem This Solves
Route bindings to the implicit default agent
"main"are rejected at config load and silently removed bydoctor --fixwhenagents.listis non-empty. This breaks the documented multi-agent pattern where named agents are listed while the implicit main agent handles a catch-all route.Why This Change Was Made
The exact reserved
DEFAULT_AGENT_IDis treated as a valid explicit binding target across the three ownership boundaries that consume it:"main""main"while pruning missing named agents and normalized variantsEvidence
5373df42a4, rebased onto currentmainatcadad3b7bd.bindings.0.agentId: Unknown agent id "main" (not in agents.list).OpenClawSchema.safeParse,pruneBindingsForMissingAgents, andresolveAgentRoutedirectly:{ "schemaAcceptedMain": true, "schemaRejectedMainVariant": true, "doctorBindings": ["main"], "doctorChanges": [ "Removed 2 bindings that referenced missing agents.list ids." ], "routedAgentId": "main", "routedSessionKey": "agent:main:main", "matchedBy": "binding.account" }oxfmt,oxlint, andgit diff --checkpassed.