Skip to content

fix(ci): repair guidance and stabilize detection timeout proof - #732

Merged
steipete merged 4 commits into
mainfrom
fix/ci-guidance-20260913
Sep 13, 2026
Merged

steipete merged 4 commits into
mainfrom
fix/ci-guidance-20260913

Conversation

@steipete

@steipete steipete commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Strengthen selected-CLI guidance checks and remove a scheduler-sensitive timeout-test failure. Main’s direct support for literal peekaboo and selected "$PB" commands is preserved; this PR extends the app/PID unsafe/safe matrix, restores the read-only clipboard example and assertion, repairs published guide links, and runs guidance validation in regular macOS CI.

The timeout fixture confirms that the runner’s own operation entered, receives the expected timeout while work remains gated, then releases and drains it. Its watchdog fails an accidental join. Production timeout behavior and main’s #730 window-close implementation, tests, and CI coverage are unchanged.

Validation for ccd8f3837401cbcffae3596670da86b9bd1bab44:

  • Independent autoreview: scoped-clean through P2.
  • Complete package and safe suites: all four groups passed.
  • macOS CI and CodeQL.
  • Local: 28 guidance/workflow tests after integration; three focused Swift timeout/cancellation tests; formatting/lint, docs build, and HTTP 200 checks for the homepage and repaired skill guide.

Regression evidence: guidance failure and the 468 ms Core fixture failure. No tests were deleted or skipped to obtain green results. This is the prerequisite for dependency PR #731.

@steipete
steipete requested a review from a team as a code owner September 13, 2026 15:42
@clawsweeper

clawsweeper Bot commented Sep 13, 2026

Copy link
Copy Markdown

🦞👀
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.

@cursor

cursor Bot commented Sep 13, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are limited to documentation, Node guidance tests, CI wiring, and a Swift unit-test refactor; production detection timeout logic is unchanged.

Overview
CI and docs: macOS CI’s docs-lint step now runs tests/background-capability-guidance.test.mjs, and the agent-skill maintenance recipe matches. Published agent-skill links point at canonical GitHub URLs instead of broken relative paths.

Guidance contracts: The bundled skill again shows a read-only clipboard get --json example, and the Node guidance suite treats both peekaboo and "$PB" when checking unsafe app/PID-only press routes versus window-targeted safe forms.

Flaky Core test: Detection timeout wins over noncooperative work no longer depends on finishing within ~250 ms of real sleep. It uses a ElementDetectionTimeoutWorkGate to confirm the timed operation starts, times out at the configured deadline, and leaves blocked work unreleased until explicitly drained—so the proof is about timeout behavior, not scheduler speed.

Reviewed by Cursor Bugbot for commit ccd8f38. Bugbot is set up for automated code reviews on this repo. Configure here.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 13, 2026
@clawsweeper

clawsweeper Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 13, 2026, 1:55 PM ET / 17:55 UTC (Revision 5).

ClawSweeper review

What this changes

This PR strengthens agent-guidance checks, repairs documentation links, restores a task-scoped clipboard example, adds guidance validation to macOS CI, and replaces a timing-sensitive detection test with gated work.

Merge readiness

Ready for maintainer review

The prior conflict blocker is resolved, and no actionable defect remains in the reviewed diff. Main contains the overlapping matcher repair, but the additional coverage, documentation corrections, CI wiring, and timeout fixture remain useful distinct work.

Priority: P2
Reviewed head: ccd8f3837401cbcffae3596670da86b9bd1bab44

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, useful patch with reported relevant validation and no actionable correctness or security findings.
Proof confidence 🌊 off-meta tidepool Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate and introduces no material authority change; its reported documentation and focused test runs support the changed validation surface, while production timeout behavior is unchanged.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate and introduces no material authority change; its reported documentation and focused test runs support the changed validation surface, while production timeout behavior is unchanged.
Evidence reviewed 7 items Applicable repository policy: Read the full root AGENTS.md; the repository search found no nested AGENTS.md or maintainer-notes directory. Applied its module, regression-test, documentation, and validation guidance within the read-only review contract.
Integrated change ownership: The complete base-to-head diff contains six files and no production runtime changes. Raw head parentage confirms integration of the pinned main revision. The supplied verified test merge has main followed by this head as its parents and retains the same six-file delta.
Timeout fixture preserves the relevant invariant: The test waits for operation entry, requires the exact timeout error while the gate remains unreleased, then releases and drains work. The unchanged production runner resumes its continuation without joining cancelled work; its deadline owner prevents late completion from replacing the timeout.
Findings None None.
Security None None.

How this fits together

Peekaboo’s guidance teaches agents how to operate its macOS CLI safely, while CI checks that guidance against supported commands. The detection timeout fixture verifies that UI observation can return a timeout without waiting for noncooperative work.

flowchart TD
  A[Agent guidance] --> B[Guidance contract tests]
  C[macOS CI] --> B
  C --> D[Detection timeout test]
  D --> E[Blocked observation work]
  D --> F[Timeout returns before release]
  B --> G[Validation results]
  F --> G
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Runtime and test footprint Production runtime +0/-0; tests +82/-16 Added test code replaces a wall-clock threshold with synchronization and expands guidance coverage without changing runtime behavior.

Technical review

Best possible solution:

Retain main’s shared guidance matcher and land the focused validation and documentation improvements with the gated timeout invariant.

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

No deterministic reproduction was established in this review: the original fixture depends on scheduler timing, and the captured body links a historical 468 ms failure. Source inspection confirms the brittle wall-clock assertion remains on main.

Is this the best way to solve the issue?

Yes. The patch reuses main’s matcher and verifies timeout return before releasing blocked work, avoiding the original tight elapsed-time assertion without changing production semantics.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P2: This is a bounded validation-reliability improvement with supporting documentation corrections.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate and introduces no material authority change; its reported documentation and focused test runs support the changed validation surface, while production timeout behavior is unchanged.

Evidence

What I checked:

  • Applicable repository policy: Read the full root AGENTS.md; the repository search found no nested AGENTS.md or maintainer-notes directory. Applied its module, regression-test, documentation, and validation guidance within the read-only review contract. (AGENTS.md:1, ccd8f3837401)
  • Integrated change ownership: The complete base-to-head diff contains six files and no production runtime changes. Raw head parentage confirms integration of the pinned main revision. The supplied verified test merge has main followed by this head as its parents and retains the same six-file delta. (ccd8f3837401)
  • Timeout fixture preserves the relevant invariant: The test waits for operation entry, requires the exact timeout error while the gate remains unreleased, then releases and drains work. The unchanged production runner resumes its continuation without joining cancelled work; its deadline owner prevents late completion from replacing the timeout. (Core/PeekabooCore/Tests/PeekabooTests/ElementDetectionServiceTests.swift:295, ccd8f3837401)
  • Main overlap and historical routing: Main’s merged fix(window): prevent hosts crashing when closing their own windows #730 supplies the dual-prefix matcher retained at line 33. GitHub commit metadata identifies steipete, and local history also shows repeated prior work on guidance and detection tests. Local blame and the earlier-head comparison encountered unavailable historical blobs; no unsupported introduction or unchanged-code claim relies on those failed reads. (tests/background-capability-guidance.test.mjs:33, a744b91e70b8)
  • Release comparison: The latest release, v4.3.4, still contains the original 20 ms timeout fixture with a 250 ms wall-clock assertion. The proposed gated fixture is absent from both that release and pinned main. (Core/PeekabooCore/Tests/PeekabooTests/ElementDetectionServiceTests.swift:295, 59a805b253c1)
  • Review continuity and validation: The captured body reports 28 combined guidance/workflow contracts and three focused Swift timeout tests passing after integration. The previous review had no findings and requested conflict resolution plus combined validation. Current-head GitHub checks show successful lint and several analysis checks, with build/full-suite checks still pending; these remain ordinary CI gates. No target code or tests were executed during this review. (ccd8f3837401)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (4 earlier review cycles)
  • reviewed 2026-09-13T15:46:58.704Z sha c1926ec :: needs maintainer review before merge. :: none
  • reviewed 2026-09-13T15:53:33.080Z sha aa1c1c9 :: needs maintainer review before merge. :: none
  • reviewed 2026-09-13T16:36:06.998Z sha 0ab5c6e :: needs maintainer review before merge. :: none
  • reviewed 2026-09-13T17:30:07.711Z sha 0ab5c6e :: blocked before merge. :: none

@steipete steipete changed the title fix(ci): validate guidance for the selected CLI binary fix(ci): repair guidance and stabilize detection timeout proof Sep 13, 2026
@steipete
steipete merged commit f678658 into main Sep 13, 2026
17 checks passed
@steipete
steipete deleted the fix/ci-guidance-20260913 branch September 13, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant