Skip to content

Separate human-only PR intake signals#339

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

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

Conversation

@Jhacarreiro

@Jhacarreiro Jhacarreiro commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Reintroduce only the focused human-only PR intake classification from #333, rebuilt on current main after #335 landed.

This PR classifies metadata-only ClawSweeper signals as requires_human instead of enqueueing a repair job, while preserving repair jobs when objective repair blockers exist.

Scope

- human-only / metadata-only classification only
- no watch-silence state
- no fingerprinting
- no adapter/model code
- preserves author-wide intake from #335

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

Covered cases:

- cancelled-only checks stay ignored
- failed checks still write repair jobs
- author-wide discovery still works
- metadata-only ClawSweeper labels route to requires_human
- metadata plus failed check still writes a repair job

Real dry-run proof

After rebuilding this branch locally from fix/human-only-pr-intake-v2, I reran the dry-runs requested by review.

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

1. Metadata-only PR becomes requires_human, no repair job

Command:

node dist/repair/pr-repair-intake.js --repo openclaw/clawsweeper --author Jhacarreiro --limit 100 --dry-run --no-comments

Redacted output summary:

{
  "status": "ok",
  "repo": "openclaw/clawsweeper",
  "author": "Jhacarreiro",
  "scanned": 1,
  "candidates": 0,
  "requires_human": [
    {
      "number": 339,
      "url": "https://github.com/openclaw/clawsweeper/pull/339",
      "reason": "No repairable blocker was detected. Remaining signals look like human/review workflow or stale metadata, so do not create an automatic repair job.",
      "repairable_signals": [],
      "metadata_signals": [
        { "kind": "clawsweeper_rating", "detail": "label=rating: unranked krab" },
        { "kind": "clawsweeper_status", "detail": "label=status: needs proof" },
        { "kind": "clawsweeper_merge_risk", "detail": "label=merge-risk: automation" }
      ]
    }
  ],
  "jobs": []
}

2. Objectively repairable PR still creates a repair job

Command:

node dist/repair/pr-repair-intake.js --repo Jhacarreiro/clawsweeper --author Jhacarreiro --limit 100 --dry-run --no-comments

Redacted output summary:

{
  "status": "ok",
  "repo": "Jhacarreiro/clawsweeper",
  "author": "Jhacarreiro",
  "scanned": 1,
  "candidates": 1,
  "requires_human": [],
  "jobs": [
    {
      "status": "planned",
      "job": "jobs/Jhacarreiro/inbox/repair-pr-jhacarreiro-clawsweeper-2.md",
      "number": 2,
      "signals": [
        { "kind": "check_failed", "detail": "pnpm check: conclusion=FAILURE" }
      ]
    }
  ]
}

This covers both sides requested by review: metadata-only PRs are not enqueued as repair jobs, and objectively failed-check PRs remain repair candidates.

@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 19, 2026, 9:22 AM ET / 13:22 UTC.

Summary
The branch adds label-aware requires_human output to PR repair intake and tests metadata-only versus failed-check routing.

Reproducibility: not applicable. as a bug reproduction; this is a PR adding a new intake classification path. Source review and the contributor's dry-run output cover both intended routing paths.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 2 files changed, +170/-6. The behavior change is limited to the repair-intake executable and its focused test file.
  • Proof cases: 2 redacted dry-run outcomes. The supplied terminal proof exercises both the metadata-only human route and the failed-check repair-job route.

Root-cause cluster
Relationship: canonical
Canonical: #339
Summary: This PR is the current focused implementation candidate for separating human-only PR intake signals; older same-direction branches are closed, and the merged author-wide intake PR is adjacent context.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Have a repair-lane owner accept or reject the requires_human signal contract before merge.

Risk before merge

  • [P1] This changes which PR states create durable repair jobs versus human-only output; if maintainers disagree with the metadata boundary, actionable repair work could be suppressed or delayed.
  • [P1] The proof is a narrow redacted dry-run sample rather than a broad maintainer-run production rehearsal, so maintainers may still want a wider sample before changing live intake behavior.

Maintainer options:

  1. Merge After Policy Acceptance (recommended)
    A repair-lane owner can land the branch if they accept routing metadata-only ClawSweeper labels to requires_human instead of durable repair jobs.
  2. Request Wider Dry-Run Coverage
    Maintainers can ask for a larger representative dry-run sample before changing the intake boundary.
  3. Keep Current Intake Behavior
    If the metadata boundary is not accepted, pause or close this PR and leave repair-job gating unchanged.

Next step before merge

  • [P2] Manual review is needed because no narrow code defect remains; a repair-lane owner must decide whether metadata-only ClawSweeper labels should route to requires_human.

Security
Cleared: The diff changes local TypeScript intake classification and focused tests only; it adds no dependencies, workflows, permissions, secret handling, package scripts, or downloaded code.

Review details

Best possible solution:

Land this only if maintainers accept the narrow human-only signal contract, while preserving author-wide intake and keeping failed checks, dirty merge states, and review blockers as repair jobs.

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

Not applicable as a bug reproduction; this is a PR adding a new intake classification path. Source review and the contributor's dry-run output cover both intended routing paths.

Is this the best way to solve the issue?

Yes, conditionally: the implementation is narrow and tested, but final acceptance is a maintainer policy call because it intentionally changes automation routing.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded repair-automation improvement with limited code surface but real maintainer-visible workflow impact.
  • merge-risk: 🚨 automation: The PR changes which open PR signals create repair jobs versus human-only output, so a wrong boundary could misroute ClawSweeper automation.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The contributor supplied redacted terminal dry-run output showing the new requires_human route and the preserved failed-check repair-job route.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor supplied redacted terminal dry-run output showing the new requires_human route and the preserved failed-check repair-job route.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its repair-lane, narrow-test, and automation-safety guidance applies because this PR changes src/repair/ intake behavior. (AGENTS.md:1, 471bf8065dd4)
  • Current main behavior: Current main maps PRs through candidateResult and filters by signal count only; it has no label-aware requires_human split. (src/repair/pr-repair-intake.ts:110, 471bf8065dd4)
  • PR implementation: The PR head computes triageDecision, filters requires_human results out of repair-job candidates, and returns the human-only details in repo-mode output. (src/repair/pr-repair-intake.ts:344, 7faebbc25cf8)
  • Focused tests: The PR adds tests for metadata-only ClawSweeper labels routing to human review and for metadata plus a failed check still writing a repair job. (test/repair/pr-repair-intake.test.ts:124, 7faebbc25cf8)
  • Real behavior proof: The PR body and follow-up comment include redacted terminal dry-run output showing this PR under requires_human with no jobs and a separate failed-check PR producing a planned repair job. (7faebbc25cf8)
  • Prior policy context: The predecessor PR was closed with a request for a current-main rebuild, narrow contract, and real dry-run proof; this PR addresses the proof and rebase parts but still needs maintainer acceptance of the routing policy. (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 related tests that this PR refines. (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 author-wide intake flow and repository-profile routing to this person, and the predecessor PR includes policy direction from the same area. (role: recent area contributor and policy commenter; 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

Copy link
Copy Markdown
Contributor Author

Real dry-run proof added

I rebuilt and ran this branch locally from fix/human-only-pr-intake-v2 after build:repair passed.

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

1. Metadata-only PR becomes requires_human, no repair job

Command:

node dist/repair/pr-repair-intake.js --repo openclaw/clawsweeper --author Jhacarreiro --limit 100 --dry-run --no-comments

Redacted output summary:

{
  "status": "ok",
  "repo": "openclaw/clawsweeper",
  "author": "Jhacarreiro",
  "scanned": 1,
  "candidates": 0,
  "requires_human": [
    {
      "number": 339,
      "url": "https://github.com/openclaw/clawsweeper/pull/339",
      "reason": "No repairable blocker was detected. Remaining signals look like human/review workflow or stale metadata, so do not create an automatic repair job.",
      "repairable_signals": [],
      "metadata_signals": [
        { "kind": "clawsweeper_rating", "detail": "label=rating: unranked krab" },
        { "kind": "clawsweeper_status", "detail": "label=status: needs proof" },
        { "kind": "clawsweeper_merge_risk", "detail": "label=merge-risk: automation" }
      ]
    }
  ],
  "jobs": []
}

2. Objectively repairable PR still creates a repair job

Command:

node dist/repair/pr-repair-intake.js --repo Jhacarreiro/clawsweeper --author Jhacarreiro --limit 100 --dry-run --no-comments

Redacted output summary:

{
  "status": "ok",
  "repo": "Jhacarreiro/clawsweeper",
  "author": "Jhacarreiro",
  "scanned": 1,
  "candidates": 1,
  "requires_human": [],
  "jobs": [
    {
      "status": "planned",
      "job": "jobs/Jhacarreiro/inbox/repair-pr-jhacarreiro-clawsweeper-2.md",
      "number": 2,
      "signals": [
        { "kind": "check_failed", "detail": "pnpm check: conclusion=FAILURE" }
      ]
    }
  ]
}

This covers both sides requested by the prior review: metadata-only PRs are not enqueued as repair jobs, and objectively failed-check PRs remain repair candidates.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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. and removed 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. labels Jun 19, 2026
@steipete

Copy link
Copy Markdown
Contributor

Maintainer deep review found two blocking routing defects.

  1. Metadata labels are added to blockingSignals, which activates comment parsing. A ClawSweeper proof-request comment such as Blocking: real behavior proof is required then becomes comment_actionable and creates a repair job. I reproduced this against the PR head; current main creates no job for the same PR state.

  2. comment_actionable is treated as non-repairable whenever proof: sufficient exists, regardless of comment author. I reproduced a maintainer comment saying Please fix the runtime bug before merge being routed to requires_human with zero repair jobs.

The supplied dry-runs use --no-comments, so they do not exercise the default behavior where both failures occur. Also, current main already ignores metadata-only labels and creates no repair job, so the claimed suppression behavior is not a current-main bug.

Required shape before reconsideration:

  • Keep metadata/human signals separate from repair-job signals; metadata must not activate generic comment routing.
  • Classify automation comments using trusted author/marker ownership, not proof: sufficient.
  • Add default-mode regression tests for a ClawSweeper proof blocker and a real maintainer fix request.

Relevant code: src/repair/pr-repair-intake.ts, especially label insertion around lines 292-295 and comment_actionable classification around lines 362-410.

@steipete

Copy link
Copy Markdown
Contributor

Closing after maintainer deep review. Current main does not read metadata labels during PR repair intake, so the claimed metadata-label enqueue failure is not present. Default-mode executable reproductions also showed this branch can create a false repair job from a ClawSweeper proof comment and suppress a real maintainer fix request when proof: sufficient is present; the submitted --no-comments proof misses both paths.

Detailed findings: #339 (comment)

Thanks for exploring the intake boundary.

@steipete steipete closed this Jun 19, 2026
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. proof: sufficient Contributor real behavior proof is sufficient. 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.

2 participants