Skip to content

Separate human-only PR intake signals#333

Closed
Jhacarreiro wants to merge 1 commit into
openclaw:mainfrom
Jhacarreiro:fix/human-only-pr-intake
Closed

Separate human-only PR intake signals#333
Jhacarreiro wants to merge 1 commit into
openclaw:mainfrom
Jhacarreiro:fix/human-only-pr-intake

Conversation

@Jhacarreiro

@Jhacarreiro Jhacarreiro commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Update: rebased on current main and kept focused

This PR was rebuilt on current main after the author-wide intake rewrite landed.

It remains intentionally focused:

- human-only / metadata-only classification only;
- no watch-silence state;
- no fingerprinting;
- no adapter/model code.

Validation:

corepack pnpm run build:repair
node --test test/repair/pr-repair-intake.test.ts

Result:

build:repair: passed
pr-repair-intake.test.ts: 5/5 tests passed

The tests now cover existing cancelled/failed-check behavior, author-wide intake, metadata-only requires_human, and objective failed-check repair job creation.

@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 6:35 AM ET / 10:35 UTC.

Summary
The branch fetches PR labels, classifies metadata-only ClawSweeper signals as requires_human, excludes those cases from repair-job creation, and adds two regression tests for metadata-only and failed-check cases.

Reproducibility: not applicable. this is a feature PR that adds a new intake classification path rather than fixing a reproduced current-main bug. The added unit tests model the intended cases, but no real dry-run proof is supplied.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 2 files changed; +181/-6. The behavior change is concentrated in the PR-repair intake CLI and its focused test file.
  • Merge state: CONFLICTING/DIRTY. The branch cannot merge cleanly after current main changed the same intake surface.

Root-cause cluster
Relationship: canonical
Canonical: #333
Summary: This PR is the current open, narrower continuation of the human-only PR intake classification work; the original intake PR is merged and the earlier same-title attempt is closed unmerged.

Members:

  • superseded: Separate human-only PR intake signals #307 - The earlier same-title PR was closed unmerged after discussion about watch-silence scope and missing coverage; this PR removes that scope and adds focused tests.
  • partial_overlap: Add repair-only PR intake #290 - The merged PR introduced the repair-only PR intake path that this PR refines, but it does not track the same remaining classification work.
  • adjacent_distinct: feat(repair): add safe author-wide PR intake #335 - The merged author-wide intake PR changed the same files and causes the current conflict, but its feature goal is separate from human-only classification.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase onto current main while preserving author-wide PR intake and repository-profile handling.
  • [P1] Add redacted real dry-run output for one metadata-only requires_human case and one repairable failed-check case.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Only build and unit-test output is supplied; the contributor should add redacted real repair:pr-intake --dry-run terminal output or logs for metadata-only and failed-check cases, then update the PR body or ask a maintainer for @clawsweeper re-review if needed.

Risk before merge

  • [P1] The branch is currently CONFLICTING/DIRTY; conflict resolution must preserve current main’s author-wide intake mode, repository-profile filtering, and target-repo threading from feat(repair): add safe author-wide PR intake #335.
  • [P1] The classifier changes which PR states create repair jobs versus human-only output, so a bad boundary could suppress actionable repair work or enqueue automation for workflow metadata.
  • [P1] The PR body supplies build and unit-test results only; it still needs redacted live dry-run output proving both metadata-only requires_human and repairable failed-check cases.

Maintainer options:

  1. Rebase and prove the intake boundary (recommended)
    Preserve current main’s author-wide intake behavior while rebasing, then add redacted real dry-run output showing metadata-only PRs become requires_human and failed-check PRs still enqueue repair jobs.
  2. Accept after maintainer-run proof
    A maintainer could resolve the conflict and run the dry-run proof themselves if they are comfortable owning the automation boundary.
  3. Pause if the branch cannot preserve main
    If the conflict resolution would replace the author-wide intake work, pause or close this PR and ask for a fresh branch based on current main.

Next step before merge

  • [P1] Human review is needed because the PR needs contributor-supplied real behavior proof and a careful conflict rebase; automation should not repair proof-gated external PRs.

Security
Cleared: The diff changes local PR-intake classification and tests only; no concrete security or supply-chain regression was found.

Review findings

  • [P1] Rebase the triage into the current intake flow — src/repair/pr-repair-intake.ts:68-72
Review details

Best possible solution:

Rebase the focused classification change onto current runRepoIntake and author-wide flows, preserve the merged intake behavior from #335, and add redacted live dry-run proof before merge.

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

Not applicable; this is a feature PR that adds a new intake classification path rather than fixing a reproduced current-main bug. The added unit tests model the intended cases, but no real dry-run proof is supplied.

Is this the best way to solve the issue?

Yes in direction, but not in its current mergeable form. The narrower classification-only approach addresses the earlier same-title branch concern, but it must be rebased onto current main and proven with live dry-run output.

Full review comments:

  • [P1] Rebase the triage into the current intake flow — src/repair/pr-repair-intake.ts:68-72
    This hunk is still on the old repo-only top-level flow, while current main now routes repo-scoped and author-wide scans through runRepoIntake; GitHub reports the head as CONFLICTING/DIRTY. Rebase the requires_human filtering into the current flow and preserve the author-wide aggregation from feat(repair): add safe author-wide PR intake #335 before merge.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against be44378572ac.

Label changes

Label justifications:

  • P2: This is a normal-priority repair-automation improvement with limited scope but real proof and mergeability blockers.
  • merge-risk: 🚨 automation: The diff changes which PR states create repair jobs versus human-only output, so a wrong boundary could misroute ClawSweeper automation.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only build and unit-test output is supplied; the contributor should add redacted real repair:pr-intake --dry-run terminal output or logs for metadata-only and failed-check cases, then update the PR body or ask a maintainer for @clawsweeper re-review if needed.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its repair-lane and automation-safety guidance applies because this PR changes src/repair/ intake behavior. (AGENTS.md:1, be44378572ac)
  • Current main lacks the requested classification: Current main maps PRs through candidateResult and filters only by signal count; it has no requires_human result path or label-aware metadata split. (src/repair/pr-repair-intake.ts:110, be44378572ac)
  • PR source change: The PR head adds requiresHuman, filters those results out of repair candidates, and includes requires_human details in output. (src/repair/pr-repair-intake.ts:68, c8dcf7ec2324)
  • PR classification logic: The PR head adds triageDecision, repairable-signal checks, metadata-signal checks, and ClawSweeper label parsing for the new boundary. (src/repair/pr-repair-intake.ts:239, c8dcf7ec2324)
  • PR regression coverage: The PR adds tests for metadata-only labels routing to human review and metadata plus failed check still writing a repair job. (test/repair/pr-repair-intake.test.ts:68, c8dcf7ec2324)
  • Mergeability blocker: GitHub reports the PR head as mergeable=CONFLICTING and mergeStateStatus=DIRTY against current base be44378572ac, so a careful rebase is required before merge. (c8dcf7ec2324)

Likely related people:

  • Jhacarreiro: GitHub PR history for Add repair-only PR intake #290 shows this person authored the original repair-only intake work and its tests, so they are connected to the affected behavior beyond this PR. (role: original repair-intake feature contributor; confidence: high; commits: ff5ada9b0c83, 4d54a576216d, f4a1239a49cc; files: src/repair/pr-repair-intake.ts, test/repair/pr-repair-intake.test.ts)
  • steipete: Local blame and feat(repair): add safe author-wide PR intake #335 tie the current main author-wide intake flow and repository-profile routing to this person’s recent work in the same files. (role: recent area contributor; confidence: high; commits: 59e2d4406ebb, 426b0351b9e1, b58b79937f14; files: src/repair/pr-repair-intake.ts, test/repair/pr-repair-intake.test.ts, README.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • 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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jun 19, 2026
@Jhacarreiro Jhacarreiro force-pushed the fix/human-only-pr-intake branch from ec1f1fd to c8dcf7e Compare June 19, 2026 10:25
@Jhacarreiro

Copy link
Copy Markdown
Contributor Author

Update: focused replacement for #307 close reason

This PR has been amended so it no longer repeats the closed #307 branch unchanged.

What changed versus #307:

- removed watch-silence / fingerprint state entirely;
- kept only the useful human-only classification idea;
- added regression coverage for the metadata-only path;
- added regression coverage proving objective failed checks still create repair jobs.

Validation:

corepack pnpm run build:repair
node --test test/repair/pr-repair-intake.test.ts

Result:

build:repair: passed
pr-repair-intake.test.ts: 4/4 tests passed

The updated patch-id differs from #307, and src/repair/pr-repair-intake.ts no longer contains watch/silence/fingerprint logic.

@steipete

Copy link
Copy Markdown
Contributor

Closing this policy proposal. It does not fix a demonstrated current-main failure, conflicts with the author-wide intake flow landed in #335, and changes the automation boundary in a way that can suppress actionable repair jobs. Unit examples are useful, but there is still no real dry-run proof for both metadata-only and objectively repairable PRs.

The current intake behavior should remain unchanged until production evidence shows a concrete misclassification. Any future version should start from current main, preserve repository-profile and author-wide routing, define the human-only signal contract narrowly, and prove both sides with real dry-run output. Thanks for narrowing the earlier proposal.

@steipete steipete closed this Jun 19, 2026
@Jhacarreiro

Copy link
Copy Markdown
Contributor Author

Update: rebased on current main and kept focused

This PR was rebuilt on current main after the author-wide intake rewrite landed.

It remains intentionally focused:

- human-only / metadata-only classification only;
- no watch-silence state;
- no fingerprinting;
- no adapter/model code.

Validation:

corepack pnpm run build:repair
node --test test/repair/pr-repair-intake.test.ts

Result:

build:repair: passed
pr-repair-intake.test.ts: 5/5 tests passed

The tests now cover existing cancelled/failed-check behavior, author-wide intake, metadata-only requires_human, and objective failed-check repair job creation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants