Skip to content

fix(import): allow issue-only duplicate closes - #309

Draft
vincentkoc wants to merge 1 commit into
mainfrom
fix/issue-only-dedupe-close
Draft

fix(import): allow issue-only duplicate closes#309
vincentkoc wants to merge 1 commit into
mainfrom
fix/issue-only-dedupe-close

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Summary

  • stop requiring a code fix before closing duplicates in issue-only clusters
  • preserve fix-first behavior for clusters containing pull requests
  • keep issue-only clusters quarantined when any member has a security signal

Validation

  • node --test test/import-gitcrawl-clusters.test.mjs (8/8)
  • npm run validate (6,699 jobs)

@clawsweeper clawsweeper Bot added 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. 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 Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 5, 2026, 5:19 PM ET / 21:19 UTC.

ClawSweeper review

What this changes

The PR changes Gitcrawl cluster-job generation so issue-only clusters can close duplicates without first requiring a fix or merge, with tests for mixed and security-signaled clusters.

Merge readiness

Blocked by patch quality or review findings - 3 items remain

Keep open: the branch disables the fix-first guard for all issue-only close classifications, including superseded closures; current main already exempts duplicates at the downstream action gate without weakening superseded protection.

Priority: P1
Reviewed head: 62039ae65b18eeff123b6781792d0745d11a47a9

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The scope is focused, but the unchanged P1 closure-policy defect makes the patch not ready to merge.
Proof confidence 🌊 off-meta tidepool Not applicable: This MEMBER-authored automation-policy PR is not subject to the external-contributor real-behavior-proof gate; its reported test validation remains supplemental.
Patch quality 🦪 silver shellfish (2/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This MEMBER-authored automation-policy PR is not subject to the external-contributor real-behavior-proof gate; its reported test validation remains supplemental.
Evidence reviewed 5 items PR broadens the shared close prerequisite: The changed expression emits require_fix_before_close: false for every issue-only cluster without distinguishing duplicate from superseded actions.
Current main already permits duplicate closes: The applicator bypasses the fix-first prerequisite only when the planned action is classified as duplicate.
Superseded closures still require a completed fix path: After the duplicate-only exception, current main requires an executed merge, merged candidate, or opened/pushed fix before other eligible closures proceed.
Findings 1 actionable finding [P1] Keep superseded closures behind the fix-first guard
Security None None.

How this fits together

Gitcrawl cluster import converts related GitHub reports into guarded ProjectClownfish jobs. The worker plans close actions from those jobs and the applicator enforces the final duplicate, superseded, and fix-first safety rules.

flowchart LR
  A[Gitcrawl cluster data] --> B[Cluster importer]
  B --> C[Generated guarded job]
  C --> D[Worker action plan]
  D --> E[Applicator safety gate]
  E --> F[Duplicate or superseded close]
Loading

Before merge

  • Keep superseded closures behind the fix-first guard (P1) - This sets the shared job flag false for an issue-only cluster, so close_superseded bypasses the current applicator guard as well as close_duplicate. Current main already exempts only duplicate; preserve the flag or add an action-specific distinction, and cover both classifications. This blocker was already visible on the unchanged reviewed head.
  • Resolve merge risk (P1) - Merging this change would allow an issue-only cluster to execute a superseded close without the fix, merged-candidate, or merge evidence that current main requires.
  • Improve patch quality - Amend the shared prerequisite so superseded actions retain fix-first protection, then add focused duplicate-versus-superseded coverage.

Findings

  • [P1] Keep superseded closures behind the fix-first guard — scripts/import-gitcrawl-clusters.mjs:277-282
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production vs test delta production +6/-1, tests +77/-3 A small importer policy change controls automated closure eligibility, so the tests need to cover the downstream action classifications.
Files affected 2 files The branch is narrowly scoped to job generation and its fixture coverage.

Merge-risk options

Maintainer options:

  1. Preserve the action-specific guard (recommended)
    Keep the generated fix-first prerequisite and test that only duplicate classifications bypass it while superseded closures remain blocked.
  2. Pause the branch
    Leave the draft unmerged if its intended behavior requires relaxing the superseded-close policy rather than only permitting duplicate closure.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Retain require_fix_before_close for clusters that allow a fix or merge; add regression coverage proving duplicate closes bypass the downstream guard while superseded closes remain fix-first gated.

Technical review

Best possible solution:

Keep the shared fix-first setting for clusters that allow fixes or merges, and rely on current main’s duplicate-only applicator exception; add regression coverage for both duplicate and superseded classifications.

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

Yes. Current source shows the importer disables a shared prerequisite, while the applicator’s existing exception is limited to duplicate classifications.

Is this the best way to solve the issue?

No. Current main already supplies the narrow duplicate exception; changing the job-wide prerequisite also admits superseded closures before their required fix path.

Full review comments:

  • [P1] Keep superseded closures behind the fix-first guard — scripts/import-gitcrawl-clusters.mjs:277-282
    This sets the shared job flag false for an issue-only cluster, so close_superseded bypasses the current applicator guard as well as close_duplicate. Current main already exempts only duplicate; preserve the flag or add an action-specific distinction, and cover both classifications. This blocker was already visible on the unchanged reviewed head.
    Confidence: 0.99
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P1: The branch can weaken an automated close safeguard for live issue clusters.
  • merge-risk: 🚨 automation: The changed field is consumed by ProjectClownfish’s automated planning and applicator path.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This MEMBER-authored automation-policy PR is not subject to the external-contributor real-behavior-proof gate; its reported test validation remains supplemental.

Evidence

Acceptance criteria:

  • [P1] node --test test/import-gitcrawl-clusters.test.mjs.
  • [P1] node --test test/apply-result.test.mjs.
  • [P1] npm run validate.

What I checked:

  • PR broadens the shared close prerequisite: The changed expression emits require_fix_before_close: false for every issue-only cluster without distinguishing duplicate from superseded actions. (scripts/import-gitcrawl-clusters.mjs:277, 62039ae65b18)
  • Current main already permits duplicate closes: The applicator bypasses the fix-first prerequisite only when the planned action is classified as duplicate. (scripts/apply-result.mjs:1158, b96e7e5700a9)
  • Superseded closures still require a completed fix path: After the duplicate-only exception, current main requires an executed merge, merged candidate, or opened/pushed fix before other eligible closures proceed. (scripts/apply-result.mjs:1183, b96e7e5700a9)
  • Current guard provenance: Blame attributes the current importer and applicator safety lines to the recent Clownfish routing commit, confirming they are part of the current-main policy surface. (scripts/apply-result.mjs:1158, 77399382d6ed)
  • Prior blocker remains on the reviewed head: The current head is identical to the earlier reviewed SHA for both changed files, so the same guard issue remains unresolved. (62039ae65b18)

Likely related people:

  • github-actions[bot]: Current-main blame ties both the importer policy and downstream fix-first exception to its recent Clownfish routing commit. (role: recent area contributor; confidence: medium; commits: 77399382d6ed; files: scripts/import-gitcrawl-clusters.mjs, scripts/apply-result.mjs)

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 (52 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-05T06:40:26.991Z sha 62039ae :: found issues before merge. :: [P1] Keep superseded closes behind the fix-first guard
  • reviewed 2026-08-05T09:35:44.538Z sha 62039ae :: needs changes before merge. :: [P1] Keep superseded closes behind the fix-first guard
  • reviewed 2026-08-05T10:41:04.804Z sha 62039ae :: needs changes before merge. :: [P1] Keep superseded closes behind the fix-first guard
  • reviewed 2026-08-05T14:23:15.978Z sha 62039ae :: needs changes before merge. :: [P1] Keep superseded closes behind the fix-first guard
  • reviewed 2026-08-05T16:03:46.373Z sha 62039ae :: found issues before merge. :: [P1] Keep superseded closes behind the fix-first guard
  • reviewed 2026-08-05T17:11:46.475Z sha 62039ae :: needs changes before merge. :: [P1] Keep superseded closures behind the fix-first guard
  • reviewed 2026-08-05T19:01:42.668Z sha 62039ae :: found issues before merge. :: [P1] Keep superseded closures behind the fix-first guard
  • reviewed 2026-08-05T20:16:01.253Z sha 62039ae :: needs changes before merge. :: [P1] Keep superseded closures behind the fix-first guard

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 14, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. 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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 2, 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. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant