chore: drop dead code and neutralize the legacy post-PR gate - #206
Open
outof-place wants to merge 6 commits into
Open
chore: drop dead code and neutralize the legacy post-PR gate#206outof-place wants to merge 6 commits into
outof-place wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Review limit reached
Next review available in: 14 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (99)
📝 WalkthroughWalkthroughThis pull request removes legacy dashboard and worker surfaces, migrates worker call sites to ChangesDashboard legacy cleanup
Worker runtime and compatibility cleanup
Infrastructure, gate, and documentation updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/worker/verify-sequences.mjs (1)
78-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFail when the reset produces an incorrect next value.
This output does not compare an expected value and does not fail the process. A broken reset can therefore report success. After the reset, insert into a known sequence-backed table or call
nextval, then throw unless the value equalsMAX(column) + 1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/worker/verify-sequences.mjs` around lines 78 - 79, Update the post-reset verification in verify-sequences around the restored.query call so it exercises a known sequence-backed table or calls nextval, computes the expected next value as MAX(column) + 1, and throws when the returned value differs; retain the existing diagnostic output as needed but ensure mismatches fail the process.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/worker/post-pr-gate.yaml`:
- Around line 23-24: Replace the disabled sentinel in
apps/worker/post-pr-gate.yaml at lines 23-24 with a Git-invalid ref component
such as __ai-workflow-gate-disabled__.lock, and update the matching assertions
in apps/worker/src/post-pr-gate/config.test.ts at lines 153-167 plus any
documented sentinel values to use the same value. Ensure
checkPostPrGateEligibility recognizes the invalid sentinel consistently.
In `@docs/plans/2026-08-04-legacy-post-pr-gate-neutralization.md`:
- Around line 3-6: Update the legacy YAML gate status section in the document to
describe the pre-neutralization state instead of claiming it is live. Revise the
rollback instructions to include the dependent post-pr-gate test change,
specifically apps/worker/src/post-pr-gate/config.test.ts, so the documented
rollback accurately restores a passing state rather than claiming the YAML file
alone is sufficient.
In `@README.md`:
- Line 57: Update README.md’s broader trigger and integration descriptions,
including the claims near the earlier overview sections and the roadmap list, so
they no longer present messages, alerts, webhooks, or schedules as currently
supported inputs. Reword them to list only supported triggers or explicitly mark
unsupported integrations as planned, while preserving the supported Jira and
PR/MR lifecycle trigger wording.
---
Nitpick comments:
In `@apps/worker/verify-sequences.mjs`:
- Around line 78-79: Update the post-reset verification in verify-sequences
around the restored.query call so it exercises a known sequence-backed table or
calls nextval, computes the expected next value as MAX(column) + 1, and throws
when the returned value differs; retain the existing diagnostic output as needed
but ensure mismatches fail the process.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 53277260-2dea-4335-91b9-ac6e65840fff
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (100)
.github/workflows/ci.ymlREADME.mdSETUP.mdapps/dashboard/app/api/approvals/handler.tsapps/dashboard/app/api/approvals/route.test.tsapps/dashboard/app/api/approvals/route.tsapps/dashboard/app/api/pre-pr-checks/handler.tsapps/dashboard/app/api/pre-pr-checks/route.test.tsapps/dashboard/app/api/pre-pr-checks/route.tsapps/dashboard/app/api/workflow-definitions/[id]/restore/route.tsapps/dashboard/app/api/workflow-definitions/handler.tsapps/dashboard/app/api/workflow-definitions/route.test.tsapps/dashboard/app/api/workflow-definitions/route.tsapps/dashboard/components/charts.tsxapps/dashboard/components/cockpit/chrome.tsxapps/dashboard/components/cockpit/controls.tsxapps/dashboard/components/cockpit/tweaks-panel.tsxapps/dashboard/lib/data/mock.tsapps/dashboard/lib/integrations/env.tsapps/dashboard/lib/integrations/vercel.tsapps/dashboard/lib/rng.tsapps/dashboard/lib/theme.tsapps/dashboard/lib/workflow-editor/clipboard.test.tsapps/dashboard/lib/workflow-editor/clipboard.tsapps/dashboard/lib/workflow-editor/edges.test.tsapps/dashboard/lib/workflow-editor/edges.tsapps/dashboard/lib/workflow-editor/graph-edit.test.tsapps/dashboard/lib/workflow-editor/graph-edit.tsapps/dashboard/lib/workflow-editor/keyboard-actions.test.tsapps/dashboard/lib/workflow-editor/keyboard-actions.tsapps/dashboard/lib/workflow-editor/layout-geometry.test.tsapps/dashboard/lib/workflow-editor/layout-geometry.tsapps/dashboard/lib/workflow-editor/repository-scope.test.tsapps/dashboard/lib/workflow-editor/repository-scope.tsapps/worker/package.jsonapps/worker/post-pr-gate.yamlapps/worker/src/clarifications/store.tsapps/worker/src/db/test-db.tsapps/worker/src/lib/auth/roles.tsapps/worker/src/lib/branch-prefix.tsapps/worker/src/lib/logger.tsapps/worker/src/lib/overview/collect-cost.test.tsapps/worker/src/lib/overview/collect-cost.tsapps/worker/src/lib/overview/collect-kpis.test.tsapps/worker/src/lib/overview/collect-kpis.tsapps/worker/src/lib/step-adapters.tsapps/worker/src/lib/trigger-delivery-store.tsapps/worker/src/post-pr-gate/config.test.tsapps/worker/src/post-pr-gate/types.tsapps/worker/src/pre-sandbox/types.tsapps/worker/src/routes/api/v1/approvals.test.tsapps/worker/src/routes/api/v1/approvals/[id]/approve.post.tsapps/worker/src/routes/api/v1/approvals/[id]/reject.post.tsapps/worker/src/routes/api/v1/clarifications.test.tsapps/worker/src/routes/api/v1/clarifications/[id]/answer.post.tsapps/worker/src/sandbox/agents/pricing.tsapps/worker/src/sandbox/agents/protocol.tsapps/worker/src/sandbox/agents/shared.tsapps/worker/src/sandbox/disposable-review-workspace.test.tsapps/worker/src/sandbox/disposable-review-workspace.tsapps/worker/src/sandbox/trusted-workspace-publisher.test.tsapps/worker/src/sandbox/trusted-workspace-publisher.tsapps/worker/src/workflow-definition/bindings.test.tsapps/worker/src/workflow-definition/bindings.tsapps/worker/src/workflow-definition/harness-profile-runtime.tsapps/worker/src/workflow-definition/resolve-agent.test.tsapps/worker/src/workflow-definition/resolve-agent.tsapps/worker/src/workflow-definition/schema.tsapps/worker/src/workflow-definition/store.test.tsapps/worker/src/workflow-definition/store.tsapps/worker/src/workflow-definition/v2-migration.tsapps/worker/src/workflows/agent-input.test.tsapps/worker/src/workflows/agent-input.tsapps/worker/src/workflows/agent-provider-fences.test.tsapps/worker/src/workflows/agent.tsapps/worker/src/workflows/blocks/agent-sandbox.test.tsapps/worker/src/workflows/blocks/agent-sandbox.tsapps/worker/src/workflows/blocks/io-blocks.edge.test.tsapps/worker/src/workflows/blocks/post-ticket-comment.test.tsapps/worker/src/workflows/blocks/post-ticket-comment.tsapps/worker/src/workflows/blocks/prepare-workspace.test.tsapps/worker/src/workflows/blocks/prepare-workspace.tsapps/worker/src/workflows/blocks/send-plan-approval.test.tsapps/worker/src/workflows/blocks/send-plan-approval.tsapps/worker/src/workflows/clarification-snapshot-steps.test.tsapps/worker/src/workflows/clarification-snapshot-steps.tsapps/worker/src/workflows/definition-step.test.tsapps/worker/src/workflows/definition-step.tsapps/worker/src/workflows/loader-triggers.edge.test.tsapps/worker/src/workflows/pr-external-resources.test.tsapps/worker/src/workflows/pr-external-resources.tsapps/worker/src/workflows/run-ownership-steps.test.tsapps/worker/src/workflows/run-ownership-steps.tsapps/worker/src/workflows/ticket-transition-step.test.tsapps/worker/src/workflows/ticket-transition-step.tsapps/worker/src/workflows/workflow-ticket.test.tsapps/worker/src/workflows/workflow-ticket.tsapps/worker/verify-sequences.mjsdocs/plans/2026-08-04-legacy-post-pr-gate-neutralization.mdpackage.json
💤 Files with no reviewable changes (51)
- apps/worker/src/lib/branch-prefix.ts
- apps/dashboard/lib/integrations/env.ts
- apps/worker/src/workflow-definition/store.ts
- apps/worker/src/lib/trigger-delivery-store.ts
- apps/dashboard/lib/workflow-editor/graph-edit.ts
- apps/dashboard/lib/workflow-editor/repository-scope.ts
- apps/dashboard/lib/rng.ts
- apps/dashboard/lib/workflow-editor/clipboard.test.ts
- apps/dashboard/lib/integrations/vercel.ts
- apps/dashboard/components/charts.tsx
- apps/worker/src/lib/logger.ts
- apps/worker/src/lib/overview/collect-cost.test.ts
- apps/dashboard/lib/workflow-editor/edges.test.ts
- apps/worker/src/workflows/pr-external-resources.test.ts
- apps/worker/src/lib/overview/collect-kpis.test.ts
- apps/worker/src/post-pr-gate/types.ts
- apps/worker/src/pre-sandbox/types.ts
- apps/dashboard/lib/workflow-editor/keyboard-actions.ts
- apps/worker/src/workflow-definition/v2-migration.ts
- apps/dashboard/app/api/approvals/handler.ts
- apps/worker/src/workflows/agent-input.ts
- apps/dashboard/lib/workflow-editor/keyboard-actions.test.ts
- apps/dashboard/components/cockpit/chrome.tsx
- apps/dashboard/lib/workflow-editor/layout-geometry.test.ts
- apps/dashboard/lib/theme.ts
- apps/worker/src/workflows/pr-external-resources.ts
- apps/worker/src/lib/overview/collect-cost.ts
- apps/worker/src/lib/overview/collect-kpis.ts
- apps/worker/src/sandbox/agents/protocol.ts
- apps/worker/src/lib/step-adapters.ts
- apps/worker/src/sandbox/agents/shared.ts
- apps/dashboard/lib/data/mock.ts
- apps/worker/src/sandbox/agents/pricing.ts
- apps/dashboard/app/api/approvals/route.test.ts
- apps/worker/src/workflow-definition/resolve-agent.ts
- apps/worker/src/clarifications/store.ts
- apps/dashboard/lib/workflow-editor/repository-scope.test.ts
- apps/dashboard/lib/workflow-editor/edges.ts
- apps/dashboard/lib/workflow-editor/layout-geometry.ts
- apps/worker/src/workflow-definition/schema.ts
- apps/worker/src/workflow-definition/harness-profile-runtime.ts
- apps/dashboard/lib/workflow-editor/clipboard.ts
- apps/dashboard/app/api/workflow-definitions/[id]/restore/route.ts
- apps/dashboard/app/api/pre-pr-checks/handler.ts
- apps/dashboard/app/api/workflow-definitions/handler.ts
- apps/dashboard/components/cockpit/tweaks-panel.tsx
- apps/dashboard/app/api/approvals/route.ts
- apps/worker/src/lib/auth/roles.ts
- apps/worker/src/workflow-definition/store.test.ts
- apps/dashboard/app/api/workflow-definitions/route.test.ts
- apps/worker/src/workflow-definition/bindings.ts
outof-place
force-pushed
the
chore/dead-code-and-ci-cleanup
branch
from
August 5, 2026 15:31
8916eb7 to
1ed0e61
Compare
outof-place
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two things, kept in separate commits so either can be reverted alone: the legacy post-PR gate goes off, and confirmed-dead code gets deleted.
Net: 99 files, +383 / -17506.
The legacy post-PR gate is off (
88708b8)apps/worker/post-pr-gate.yamlwas committed and enabling the gate, so it publishedAI Workflow / code-hygienealongside the editable Post-PR review workflow'sAI Workflow / Reviewon the same head SHA, and spent one Claude Haiku call per changed file on every bot pull request.docs/plans/2026-08-04-legacy-post-pr-gate-neutralization.mddocumented this and its own runbook, but the runbook had never been applied. This applies section 2 of it verbatim.The gate machinery under
src/post-pr-gate/, thecode-hygienestep, and thegate_locks/gate_dedupe/gate_currenttables are all left in place on purpose, sogit revert 88708b8is a complete rollback. Deleting the file would not have worked: onENOENTthe built-in default inconfig.ts:10takes over and the gate is live again.Three tests in
config.test.tsnow read the shipped yaml and fail if anyone re-enables the gate. Mutation-checked: restoring the old config fails exactly those assertions.Two consequences worth knowing, both recorded in the plan document:
repositoryScopepin previously gotcode-hygieneas its only coverage.cancelPreviousRunbecomes unreachable, so a gate check left in progress by an earlier crashed run cannot self-heal. Verified empty here at the time of writing: one open pull request, on a non-managed branch, carrying noAI Workflow /check. Worth re-checking on a tenant before syncing.This propagates to the Arthur tenant on the next release sync. That was a deliberate decision, not an oversight.
Worker test CPU: split out, not included here
An earlier revision of this PR cached the test database schema as a PGlite data-directory snapshot instead of replaying all 39 migrations on every
createTestDb()call. It was 34% faster and used 38% less CPU, and it broke CI twice: once killed by SIGTERM at 107s, once still running when the job hit its 20 minute timeout.Controlled measurement, same machine, 4 forks on both sides to match the runner:
The snapshot trades memory for speed, and memory is exactly what the runner is short of: 4 vCPU, 16 GB, with the dashboard suite running concurrently. Dropping the commit puts peak RSS back at 5.32 GB, level with main.
The underlying problem is older than that commit: no test ever closes the database it creates, so instances accumulate inside each fork. Restoring a data directory allocates more per instance than replaying migrations does, which is why the snapshot pushed an already-heavy suite over the edge. Closing databases via
onTestFinishedwould cut peak memory on both paths and is the prerequisite for the snapshot being worth having. That touches all 63 database test files and wants its own PR.The commit is preserved on
perf/test-db-snapshot, including the sequence-reset fix described below.Dead code removed (
1ed0e61,ae104ee)Every deletion was verified with a word-boundary search showing the symbol appears only in its own definition and its own test.
Worker, 2035 lines:
lib/overview/collect-cost.tsandcollect-kpis.tsplus tests. The live routes usecostAggandrunKpisfromdb/queries/runs-read.ts; these were superseded and left behind.lib/step-adapters.tsmerged intolib/adapters.ts. The two differed only in an unusedvcsTargetparameter and memoization of avcsgetter that none of the 29 call sites read.lib/branch-prefix.ts, a re-export shim with zero references repo-wide.normalizeDefinitionForExecutionwhose docstring described behavior that no longer exists.Dashboard, 1373 lines:
components/cockpit/tweaks-panel.tsx(691 lines, zero importers). TheuseTweakshook is live and stays; only the panel that nobody ever mounted goes.lib/integrations/vercel.tsandenv.ts, referenced only from a planning document.SearchBox(superseded byspotlight-search.tsx),BarRow,Histogram,CkTopbar(an empty div), and 9 helpers whose only consumer was their own test.Also
package-lock.json, 14064 lines, tracked and 4 months stale on a project that runs entirely on pnpm.Suites CI never ran (
3a06d0f)scripts/release-noteshas nopackage.json, so it is not a workspace member andpnpm -r testskipped its 43 tests. Now wired in, 2.4s.workflow-sdk-testssits besidesrc/and falls outsidevitest.config.ts's include glob. Now wired in, 11 tests, 9.5s. Its script was also missing thebuild:shared &&prefix every sibling has, so it failed cold on a fresh checkout.check:prompt-driftwas left alone deliberately. It opens a real Neon connection to compare shipped constants against production rows, so it is meaningless in CI.Verification
pnpm run typecheckDeliberately not done
recordWorkflowBlockAttemptObservationand thesaveWorkflowDefinitionVersion/restoreWorkflowDefinitionVersionpair have no production callers, but their tests interleave with live functions in shared integration files. Deleting them means rewriting real coverage of live code, which does not belong in a cleanup pass.e2e/tier2/us22-gate-skips-non-bot.test.tsandus26-gate-runon-filters.test.tsassert the absence of gate checks and now pass unconditionally. They regain meaning if the gate is reverted, so they stay, noted in the plan document.Follow-ups
onTestFinished, then revisitperf/test-db-snapshot. That branch also carries a fix worth keeping either way:loadDataDirresumes sequences from the WAL high-water mark rather than their true value (Postgres logs them in blocks of 32), so an id that should be 4 comes back as 34. It breaks 22 hardcoded id assertions across 12 files.README.mdclaimed Linear triggers and Slack, alert, webhook and schedule inputs that do not exist, while its own Roadmap listed the same items as future work. Corrected here (903513f), along with documenting fiveENABLE_*flags that existed only inenv.ts.🤖 Generated with Claude Code