test: replace stale agent scaffolding with production contracts - #726
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. |
PR SummaryLow Risk Overview
Reviewed by Cursor Bugbot for commit 82a3af2. Bugbot is set up for automated code reviews on this repo. Configure here. |
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 11:03 PM ET / September 13, 2026, 03:03 UTC (Revision 2). ClawSweeper reviewWhat this changesReplaces obsolete agent test scaffolding with checks of production CLI parsing, session output, formatting, and disk persistence, and removes an outdated source comment. Merge readiness✅ Ready for maintainer review This remains a useful, focused cleanup with no actionable patch findings. The replacement coverage is absent from the reviewed main and latest release, and collaborator-authored work is protected from automatic closure. Priority: P3 Review scores
Verification
How this fits togetherPeekaboo’s agent CLI accepts task text and resume options, then uses the agent runtime to retrieve saved conversations. These tests exercise the parsing, display, and storage contracts connecting those layers. flowchart LR
A[Task and resume arguments] --> B[CLI parsing]
B --> C[Agent session runtime]
C --> D[Session files and cache]
D --> C
C --> E[Session JSON and display]
F[Contract tests] --> B
F --> D
F --> E
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep contract tests at their production owners and retire inactive scaffolding without changing runtime behavior. Do we have a high-confidence way to reproduce the issue? Not applicable: this is test maintenance, and source inspection confirms the obsolete scaffolding being replaced. Is this the best way to solve the issue? Yes: calling production parsing, formatting, and storage owners removes copied logic while preserving the existing runtime contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 8ecf8a9159ed. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
1 participant
Several legacy test files contained only commented-out APIs, while resume tests asserted copied formatter logic and literal values. Replace those checks with production CLI parsing, exact session IDs and options, Unicode/long continuation preservation, real session JSON projection, and the production time formatter. Add a disk round trip that verifies conversation messages, cached-session deletion, and missing-session lookup. Remove the obsolete temporary-session comment.
Coverage is retained at its current owners:
AgentSessionManagerStorageTestscovers persistence;UIAutomationServiceWaitTestscovers snapshot-based waits;ScreenCaptureServiceFlowTestscovers capture routing and failures;MCPToolExecutionPolicyTestsandAgentSessionExecutionPolicyTestsenforce the current shell boundary. The removed legacy shell cases described execution that Agent policy deliberately forbids. Deleted files were wholly commented, import-only, or an empty no-assertion placeholder. No failing behavior test was removed or weakened.Validation: 44 CLI checks and 54 owning-module checks passed. SwiftFormat, SwiftLint (zero violations), docs lint, and CLI build passed. Isolated Codex review found no actionable P0–P2 findings. Native proof used the signed built CLI:
agent runwith Unicode/quoted input and--dry-run --jsonpreserved the instruction, returned success with background-only authority, and reported zero model/tool calls;agent resume --helpexposed the current exact-ID interface. Runtime behavior is unchanged.