Repository navigation
Keep Stop available when a newer Agent revision is unreadable - #3940
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. What this changesThe Agents list keeps Stop available based on Agent state and operate permission rather than a failed deployment from filtered revision history. Example: Mixed Agent has readable failed v1, unreadable queued v2, and no selected version; mixed-operator has Agent operate permission.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review This PR fixes a current-main defect in authorized Stop availability; no actionable introduced bugs were found. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe browser Console combines Agent state, filtered revision history, and advisory permissions into row controls; the Controller independently authorizes Stop and records the requested stopped state. flowchart TB
A[Operator] --> B[Agents list]
C[Filtered revision history] --> B
D[Agent state and permission advice] --> B
B -->|Confirmed Stop| E[Controller API]
E --> F[Exact Agent authorization]
F --> G[Stopped state and reconciliation work]
Technical reviewBest possible solution: Keep Stop availability tied to Agent state and exact operate permission while using readable deployment history only to explain confirmation. Do we have a high-confidence way to reproduce the issue? Current-main source establishes the failed-v1/hidden-v2 path, and the supplied before/after screenshots support the reported regression; no target code was executed during this review. Is this the best way to solve the issue? Removing the filtered-history suppression is the smallest complete repair for the linked issue and reuses existing confirmation, authorization, and lifecycle owners. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 1258c43f75fb. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 28, 2026, 7:26 AM ET / 11:26 UTC. |
Fixes #3707.
An operator could lose Stop on the Agents list when the last deployment they could read had failed, even though the Agent could still be stopped. The list now uses the Agent's state and the member's operate permission to offer Stop. A readable deployment can still explain what confirmation will do. Delete permissions are unchanged.
The browser regression uses the same member throughout: failed v1 is readable and queued v2 is not. The list sends no Stop before confirmation, sends one after confirmation, and the Controller returns 202. Reading the Agent back shows that its desired state is stopped. The browser never requests v2, and Delete stays disabled.
The regression failed on the upstream base and passed with the fix. All 12 selected browser cases passed against the Controller with native IAM and in-memory state; the test supplies the failed v1 deployment response. The rebuilt Storybook preview covers the list and detail controls with simulated data.
Verification
Controller-backed browser, with in-memory state and a supplied failed-v1 response:
stop-walkthrough.webm
Storybook: Components / Stop Agent / Failed first deployment (simulated).