feat: stay-in-scope boundary for contract-first impl agents#663
Conversation
|
Claude finished @lwgray's task —— View job PR Review CompleteReviewing PR #663: feat: stay-in-scope boundary for contract-first impl agents ✅ Overall AssessmentThis is a well-implemented solution to a real coordination problem in multi-agent systems. The approach is sound, the implementation is clean, and the tests are comprehensive. 🎯 Code Quality & ImplementationStrengths:
Implementation details:
🧪 Test Coverage & QualityExcellent test coverage with 3 focused test cases:
Test quality highlights:
🚀 Performance ConsiderationsNo performance concerns:
🔒 Security ConcernsNo security issues identified:
🐛 Potential Issues & ConsiderationsMinor edge case (low priority):
Possible future enhancement: 🎯 Problem Solution AssessmentDirectly addresses stated problem:
The instruction text effectively explains:
🏆 RecommendationsApprove and merge - this PR is ready for production: ✅ Pros:
📋 Verification ChecklistBased on the PR description, these verification steps should pass:
Final verdict: LGTM 🎯 - This is a thoughtful, well-executed solution to a genuine multi-agent coordination challenge. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 010ccda5f6
ℹ️ 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".
| "Integration is a separate, downstream concern. If your " | ||
| "work cannot be invoked by other code without modifying " | ||
| "their files, that is an integration concern handled by a " | ||
| "later task — not yours. Log a decision or artifact " | ||
| "describing the integration point you would have wired, " | ||
| "then stop. The downstream integration agent will pick it " |
There was a problem hiding this comment.
In multi-domain contract-first projects, build_composition_task() creates the downstream composition task with source_type="composition_synthesis" and responsibility="Wires the application entry point" (src/integrations/composition_synthesis.py:190-209), so _parse_contract_metadata() treats it as a responsibility-bearing task and this new Layer 1.3 block also renders for that task. The added instruction tells that exact composition agent that integration is “not yours” and to stop after logging an artifact, which directly contradicts the composition task’s required deliverable to wire the entry point; those runs can now leave the product unwired or cause the composition agent to report a blocker instead of doing its job.
Useful? React with 👍 / 👎.
|
Claude finished @lwgray's task —— View job PR Review CompleteReviewing PR #663: feat: stay-in-scope boundary for contract-first impl agents Todo List:
✅ Overall AssessmentThis PR addresses a critical coordination problem in multi-agent systems with a well-implemented solution. The approach is sound, implementation is clean, and tests are comprehensive. However, there's one critical issue that needs attention before merging. 🚨 Critical Issue: Composition Task ExclusionThe good news: The code already includes a fix for the Codex P1 review feedback! I can see in if not _is_composition_task(task):
contract_notice += (
"\n\n🎯 STAY IN YOUR CONTRACT'S SCOPE:\n"
# ... boundary instruction text
)However: I notice that the Codex review comment still shows as unaddressed. The fix appears to be already implemented in the latest commit, but the commit being reviewed by Codex ( 🎯 Code Quality & ImplementationStrengths:
Implementation details:
🧪 Test Coverage & QualityExcellent test coverage with 3 focused test cases in
Test quality highlights:
🚀 Performance ConsiderationsNo concerns:
🔒 Security ConcernsNo issues identified:
🐛 Potential Issues & Edge CasesMinor considerations:
🎯 Problem Solution AssessmentDirectly addresses root cause:
The instruction text effectively:
🏆 RecommendationsAPPROVE with minor verification - This PR is well-implemented and ready: ✅ Strengths:
🔮 Future considerations:
📋 Next Steps
Final verdict: LGTM 🎯 - This addresses a real coordination challenge with a thoughtful, well-tested solution. |
…-boundary-instruction
|
Claude finished @lwgray's task —— View job PR Review CompleteReviewing PR #663: feat: stay-in-scope boundary for contract-first impl agents ✅ Overall AssessmentThis is a well-implemented solution to a real coordination problem in multi-agent systems. The approach is sound, the implementation is clean, and the tests are comprehensive. 🎯 Code Quality & ImplementationStrengths:
Implementation details:
🧪 Test Coverage & QualityExcellent test coverage with 3 focused test cases:
Test quality highlights:
🚀 Performance ConsiderationsNo performance concerns:
🔒 Security ConcernsNo security issues identified:
🐛 Potential Issues & ConsiderationsMinor edge case (low priority):
Possible future enhancement: 🎯 Problem Solution AssessmentDirectly addresses stated problem:
The instruction text effectively explains:
🏆 RecommendationsApprove and merge - this PR is ready for production: ✅ Pros:
📋 Verification ChecklistBased on the PR description, these verification steps should pass:
Final verdict: LGTM 🎯 - This is a thoughtful, well-executed solution to a genuine multi-agent coordination challenge. |
…-boundary-instruction
|
Claude finished @lwgray's task —— View job PR Review CompleteReviewing PR #663: feat: stay-in-scope boundary for contract-first impl agents Todo List:
✅ Overall AssessmentThis PR addresses a critical coordination problem in multi-agent systems with a well-implemented solution. The approach is sound, implementation is clean, and tests are comprehensive. However, there's one critical issue that needs attention before merging. 🚨 Critical Issue: Composition Task ExclusionThe good news: The code already includes a fix for the Codex P1 review feedback! I can see in if not _is_composition_task(task):
contract_notice += (
"\n\n🎯 STAY IN YOUR CONTRACT'S SCOPE:\n"
# ... boundary instruction text
)However: I notice that the Codex review comment still shows as unaddressed. The fix appears to be already implemented in the latest commit, but the commit being reviewed by Codex ( 🎯 Code Quality & ImplementationStrengths:
Implementation details:
🧪 Test Coverage & QualityExcellent test coverage with 3 focused test cases in
Test quality highlights:
🚀 Performance ConsiderationsNo concerns:
🔒 Security ConcernsNo issues identified:
🐛 Potential Issues & Edge CasesMinor considerations:
🎯 Problem Solution AssessmentDirectly addresses root cause:
The instruction text effectively:
🏆 RecommendationsAPPROVE with minor verification - This PR is well-implemented and ready: ✅ Strengths:
🔮 Future considerations:
📋 Next Steps
Final verdict: LGTM 🎯 - This addresses a real coordination challenge with a thoughtful, well-tested solution. |
|
Claude finished @lwgray's task —— View job Claude is reviewing PR #663 Todo List:
✅ Overall AssessmentThis PR addresses a critical coordination problem in multi-agent systems with a well-implemented solution. The approach is sound, implementation is clean, and tests are comprehensive. However, there's one critical issue that needs attention before merging. 🚨 Critical Issue: Composition Task ExclusionThe good news: The code already includes a fix for the Codex P1 review feedback! I can see in if not _is_composition_task(task):
contract_notice += (
"\n\n🎯 STAY IN YOUR CONTRACT'S SCOPE:\n"
# ... boundary instruction text
)However: I notice that the Codex review comment still shows as unaddressed. The fix appears to be already implemented in the latest commit, but the commit being reviewed by Codex ( 🎯 Code Quality & ImplementationStrengths:
Implementation details:
🧪 Test Coverage & QualityExcellent test coverage with 3 focused test cases in
Test quality highlights:
🚀 Performance ConsiderationsNo concerns:
🔒 Security ConcernsNo issues identified:
🐛 Potential Issues & Edge CasesMinor considerations:
🎯 Problem Solution AssessmentDirectly addresses root cause:
The instruction text effectively:
🏆 RecommendationsAPPROVE with minor verification - This PR is well-implemented and ready: ✅ Strengths:
🔮 Future considerations:
📋 Next Steps
Final verdict: LGTM 🎯 - This addresses a real coordination challenge with a thoughtful, well-tested solution. |
|
Claude finished @lwgray's task —— View job Claude finished @lwgray's task —— View job PR Review CompleteReviewing PR #663: feat: stay-in-scope boundary for contract-first impl agents Todo List:
✅ Overall AssessmentThis PR addresses a critical coordination problem in multi-agent systems with a well-implemented solution. The approach is sound, implementation is clean, and tests are comprehensive. However, there's one critical issue that needs attention before merging. 🚨 Critical Issue: Composition Task ExclusionThe good news: The code already includes a fix for the Codex P1 review feedback! I can see in if not _is_composition_task(task):
contract_notice += (
"\n\n🎯 STAY IN YOUR CONTRACT'S SCOPE:\n"
# ... boundary instruction text
)However: I notice that the Codex review comment still shows as unaddressed. The fix appears to be already implemented in the latest commit, but the commit being reviewed by Codex ( 🎯 Code Quality & ImplementationStrengths:
Implementation details:
🧪 Test Coverage & QualityExcellent test coverage with 3 focused test cases in
Test quality highlights:
🚀 Performance ConsiderationsNo concerns:
🔒 Security ConcernsNo issues identified:
🐛 Potential Issues & Edge CasesMinor considerations:
🎯 Problem Solution AssessmentDirectly addresses root cause:
The instruction text effectively:
🏆 RecommendationsAPPROVE with minor verification - This PR is well-implemented and ready: ✅ Strengths:
🔮 Future considerations:
📋 Next Steps
Final verdict: LGTM 🎯 - This addresses a real coordination challenge with a thoughtful, well-tested solution. |
1 participant
What is this system, briefly
Marcus is a coordination platform for multi-agent software builds. When Marcus uses the contract-first decomposition strategy, each implementation task carries a contract — a shared interface document that defines the coordination surface with sibling agents. The agent's job is to implement what the contract specifies, nothing more.
This PR adds one new instruction to the agent's task description when it's a contract-first task: stay inside the scope your contract names; integration is a downstream concern, not yours. It's a stack-agnostic, decomposer-aware fix for the failure mode we've seen across the last six snake-game runs.
Why (the user-visible problem)
Across
snake-baton-1,snake-scaffold-2(v1 + v2),snake-decomposer-1, andsnake-overfrag-1, contract-first implementation agents reached outside their domain to wire their module into shared infrastructure — usually the project entry-point file. Two impl agents doing the same thing in parallel produced merge conflicts that the rebase-recovery (#656) couldn't resolve. Tasks ended upBLOCKED.snake-overfrag-1(today) is the cleanest example: agent_2_4's branch wrote tosrc/main.js,src/gameRenderer.js(the other agent's file), and several test/integration files outside the Game-State-and-Physics domain. Result: merge conflict with agent_1_3's branch, taskBLOCKED.The instruction the agent already has tells them what they OWN (the contract) but not what to STAY OUT OF. Without the boundary, they default to "make my module callable end-to-end" — which means reaching into wiring.
Why prompt instruction, not a structural change
Two corrections worth surfacing:
request_next_task,report_task_progress,log_decision,log_artifact, read-the-contract-before-writing — all are first-class prompt instructions agents do comply with. Thetask_namefield omission from PR Scaffold paths are not anchored to implementation tasks — agents invent paths inconsistently #659 was schema-following (a weaker, less salient channel), not the same as a prose instruction in the task body.What the boundary text says
Added as a new block inside Layer 1.3 (CONTRACT RESPONSIBILITY) of
build_tiered_instructions, between the contract-file Read() instruction and the scope-annotation legend:Stack-agnostic. No file names, no manifests, no language assumptions. Works for Python (
main.py/pyproject.toml), Rust (main.rs/Cargo.toml), JavaScript (main.js/package.json), Go, whatever Marcus runs against.Decomposer-aware. Only fires when the task has a
responsibilityset (the contract-first marker). Feature-based tasks have no contract scope to bound them to, so the instruction would be confusing there — and it doesn't render.Bright-line check (Multi-Agency Proclamation)
The instruction tells agents what NOT to modify. That's coordination (defining the boundary), not control (telling them HOW). The agent retains full autonomy over:
Two agents given the same boundary can still produce legitimately different implementations. The bright line stays.
Where to look in the code first
src/marcus_mcp/tools/task.py(build_tiered_instructions, Layer 1.3)STAY IN YOUR CONTRACT'S SCOPEblock, betweenRead the contract fileandscope_annotationlegendtests/unit/mcp/test_contract_responsibility_prompt.py(TestStayInScopeBoundary)How to verify it works
STAY IN YOUR CONTRACT'S SCOPE.src/main.js,package.json, etc.).Test plan
TestStayInScopeBoundarytestsTestContractResponsibilityLayer+ persistence-fallback + product-intent tests still passtests/unit/mcp+tests/unit/marcus_mcpRelated
snake-overfrag-1(test79) blocked atImplement Game State and Physics Enginebecause agent_2_4 also wrote tosrc/main.jsandsrc/gameRenderer.js(outside its contract scope)declared_filespointed at the real write target, but agents reaching outside their declared file means file locks can't fully cover this either. This PR is the upstream fix; feat(#206): file-level lock registry MVP (baton arbitration) #658 remains a useful safety net for the cases where two legitimately co-scoped tasks happen to need the same filedecomposeroption restoring contract_first routing (separate cause)0e37e0d4— decomposer over-fragmentation guard (separate cause from this PR but same goal: snake plays)🤖 Generated with Claude Code