Skip to content

ci: add deadcode check#3

Merged
vincentkoc merged 3 commits into
mainfrom
codex/deadcode-20260608
Jun 9, 2026
Merged

ci: add deadcode check#3
vincentkoc merged 3 commits into
mainfrom
codex/deadcode-20260608

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

  • add an enforced Go deadcode CI gate pinned to golang.org/x/tools/cmd/deadcode@v0.45.0
  • remove unreachable helpers reported by deadcode\n- make the workflow fail when deadcode emits findings

Validation

  • git diff --check
  • go run golang.org/x/tools/cmd/deadcode@v0.45.0 -test ./...
  • go test -count=1 ./...
  • go build ./cmd/clawgo
  • autoreview --mode branch --base origin/main --no-web-search --thinking low clean

@clawsweeper

clawsweeper Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 7, 2026, 11:31 PM ET / 03:31 UTC.

Summary
The PR adds a pinned Go deadcode GitHub Actions gate and removes the unused internal Queue.Start helper.

Reproducibility: not applicable. this is a CI cleanup PR, not a bug report. Source inspection does confirm the removed Queue.Start helper has no in-repo call sites on current main.

Review metrics: 3 noteworthy metrics.

  • Files changed: 2 files, +12/-16. The review surface is one workflow change plus one internal dead-code deletion.
  • CI gates added: 1 required analyzer step. The new deadcode step changes the pass/fail behavior of the repository workflow.
  • Internal methods removed: 1 exported internal-package method. The deleted method has no current in-repo call sites, but it is still an internal API surface change.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
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] Confirm the exact PR-head GitHub Actions run is green before merge.

Risk before merge

  • [P1] Merging this intentionally makes deadcode a required CI gate, so future pushes and pull requests can fail on analyzer findings or tool install/execution issues that current main does not enforce.
  • [P1] The workflow installs the analyzer during CI from a pinned module version rather than a repository-tracked tool dependency; this is probably acceptable, but it is still a new CI execution dependency maintainers should intentionally accept.

Maintainer options:

  1. Require a green PR-head workflow
    Keep the implementation as proposed, but merge only after GitHub Actions proves the new deadcode, test, and build steps pass on the exact PR head.
  2. Adopt deadcode as policy
    Maintainers can accept the new required analyzer gate knowing it will block future CI whenever unreachable code appears or the pinned tool cannot run.
  3. Defer the CI gate
    If maintainers do not want a new required analyzer in this small repository yet, pause or close this draft and keep deadcode as an ad hoc local check.

Next step before merge

  • [P2] This member-authored draft changes CI enforcement and needs maintainer acceptance plus a green PR-head workflow, not an automated repair branch.

Security
Cleared: The diff touches CI and adds a pinned official Go analyzer under contents: read permissions, with no new secrets, permission broadening, or concrete supply-chain blocker found.

Review details

Best possible solution:

Land the PR only after maintainers decide they want deadcode enforced in CI and the PR head has a green workflow run proving the new analyzer, tests, and build all pass together.

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

Not applicable; this is a CI cleanup PR, not a bug report. Source inspection does confirm the removed Queue.Start helper has no in-repo call sites on current main.

Is this the best way to solve the issue?

Yes, if maintainers want this enforcement policy: the PR is a narrow way to add the gate and remove the reported dead code. The remaining decision is whether deadcode should become a required CI dependency for the repository.

AGENTS.md: not found in the target repository.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2c95d9596013.

Label changes

Label justifications:

  • P3: This is low-risk CI hygiene and dead-code cleanup rather than a user-facing runtime fix.
  • merge-risk: 🚨 automation: The PR changes GitHub Actions pass/fail behavior by adding a new required analyzer install and run step.
  • 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 PR is member-authored, so the external contributor real-behavior proof gate does not apply; the body lists local validation commands.
Evidence reviewed

What I checked:

  • Target AGENTS policy: No AGENTS.md exists inside the target clawgo checkout; the only discovered parent file described ClawSweeper internals and was not applied as target-repo policy.
  • Current main CI: Current main runs only go test ./... and go build ./cmd/clawgo; there is no existing deadcode gate on the default branch. (.github/workflows/ci.yml:24, 2c95d9596013)
  • PR diff surface: The supplied PR diff adds a go install golang.org/x/tools/cmd/deadcode@v0.45.0 step plus a required deadcode -test ./... output check, and deletes Queue.Start. (.github/workflows/ci.yml:21, f517b0bf111b)
  • Queue helper on main: Queue.Start exists on current main at lines 23-37 and is the method removed by this PR. (internal/routing/queue/queue.go:23, 2c95d9596013)
  • No in-repo queue Start call site: Searching for Start calls found Queue.Start only at its own definition; the other matches are unrelated audio/process start methods. (internal/routing/queue/queue.go:23, 2c95d9596013)
  • Queue history: Blame shows the queue worker method dates to the original queue scaffold commit. (internal/routing/queue/queue.go:23, 28508432940e)

Likely related people:

  • Peter Steinberger: History shows Peter introduced and updated .github/workflows/ci.yml, and recently touched internal/routing/queue/queue.go during the audio replay fix. (role: CI workflow author and recent queue contributor; confidence: high; commits: 3c69a2468abe, 507a5fe5f4fa, a86cdbb8c5ee; files: .github/workflows/ci.yml, internal/routing/queue/queue.go)
  • Mariano Belinky: Blame and git log -S show the removed Queue.Start method was introduced with the original queue scaffold. (role: queue layer introducer; confidence: high; commits: 28508432940e; files: internal/routing/queue/queue.go)
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: 🐚 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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jun 8, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. label Jun 8, 2026
@vincentkoc vincentkoc marked this pull request as ready for review June 9, 2026 02:20
@vincentkoc vincentkoc merged commit cd6c9bd into main Jun 9, 2026
1 check passed
@vincentkoc vincentkoc deleted the codex/deadcode-20260608 branch June 9, 2026 02:20
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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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