refactor: delete meta-task auto-archive and automated recovery follow-ups - #2461
Merged
Conversation
…-ups Removes two coupled reliability layers whose cost exceeded their value. Meta-task auto-archive: `classifyMetaTask` decided a card was a "meta-task" via regex over title+description (/recover|unblock|finalize|meta/i), so ordinary feature work qualified, and `resolveMetaTargetTaskId`'s fallback then bound an unmatched card to an unrelated task by creation order. The failure mode was auto-archiving live work. Both sweeps, all five helpers, the two skip-audit memo maps, and settings `metaTaskStallAutoCloseMs` / `metaTaskActiveExecutionGraceMs` are gone. Automated recovery follow-ups: `verification-followup-dedup.ts` filed cards that mostly restated state already durable on the parent. The verification and merge-conflict call sites are deleted (parents still park `failed` with a descriptive error, or log the auto-merge-gave-up entry). The dead `findActiveRecoveryFollowUp` goes with them. Preserved deliberately: the autostash-orphan path is the one caller carrying information found nowhere else -- the parent may be `done` and merged, so nothing else would ever mention a stash holding stranded uncommitted work. It becomes a logEntry + addTaskComment retaining sha, stash label, detectedByTaskId and sourcePhase, plus a truthful new run-audit event `task:autostash-orphan-live-detected`. Unchanged in behavior: eval and PR-comment follow-ups are product features, not recovery plumbing. They borrowed this engine only for dedup, which is now inlined per site on suggestionId / prNumber. Run-audit types removed: task:auto-archived-meta-resolved, task:auto-archived-meta-stalled, task:auto-archive-meta-resolved-skipped, task:auto-archive-meta-stalled-skipped, verification:followup-created, verification:followup-deduped. Verified: engine + core `tsc --noEmit` exit 0, `pnpm lint` exit 0, `pnpm build` exit 0, `pnpm test:gate` exit 0, `pnpm smoke:boot` PASS, touched engine tests 38/38. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis change removes meta-task auto-archiving and shared recovery follow-up creation. Failures remain on originating tasks with logs and comments, autostash orphans emit a dedicated audit event, and evaluation and PR-comment follow-ups retain inline open-task deduplication. ChangesTask lifecycle cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectEngine
participant TaskStore
participant RunAuditor
ProjectEngine->>TaskStore: Mark originating task failed
ProjectEngine->>TaskStore: Write log entry and parent task comment
ProjectEngine->>RunAuditor: Emit task:autostash-orphan-live-detected
sequenceDiagram
participant FollowUpHandler
participant TaskStore
participant ExistingTask
FollowUpHandler->>TaskStore: List candidate tasks
TaskStore-->>FollowUpHandler: Open matching follow-up or no match
FollowUpHandler->>ExistingTask: Reuse matching task
FollowUpHandler->>TaskStore: Create triage follow-up when no match exists
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Greptile SummaryThis PR removes unsafe meta-task auto-archival and redundant recovery follow-up machinery while preserving durable failure and autostash-orphan signals.
Confidence Score: 5/5The PR appears safe to merge with no actionable defects identified in the changed behavior. The removed archival and recovery-card paths are consistently detached from maintenance, settings, audit types, and callers, while surviving eval, PR-feedback, failure-parking, and autostash recovery behavior remains represented.
|
| Filename | Overview |
|---|---|
| packages/engine/src/project-engine.ts | Removes redundant recovery-card creation while retaining parent-task failure details and durable autostash recovery notices. |
| packages/engine/src/self-healing.ts | Deletes heuristic meta-task classification and destructive auto-archive maintenance passes. |
| packages/engine/src/eval-followups.ts | Preserves eval follow-up creation and open-card reuse through an inlined metadata scan. |
| packages/engine/src/pr-comment-handler.ts | Preserves PR-feedback follow-up behavior with an inlined parent-and-PR deduplication check. |
| packages/engine/src/run-audit.ts | Removes obsolete follow-up and meta-archive event types and adds a truthful live-autostash detection event. |
| packages/core/src/settings-schema.ts | Removes defaults for settings whose corresponding maintenance machinery no longer exists. |
Reviews (1): Last reviewed commit: "refactor: delete meta-task auto-archive ..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/engine/src/eval-followups.ts (2)
11-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
CLOSED_FOLLOWUP_COLUMNSdedup-column set across two files. Both files independently define the same "closed follow-up columns" concept as a byproduct of deleting the shared follow-up dedup engine; extracting a single shared constant (e.g. a smallfollowup-dedup.tsutil) would prevent the two from drifting out of sync if a terminal column is ever added or renamed.
packages/engine/src/eval-followups.ts#L11-L16: moveCLOSED_FOLLOWUP_COLUMNS(and its explanatory comment) into a small shared module exported for both call sites.packages/engine/src/pr-comment-handler.ts#L4-L9: import the sharedCLOSED_FOLLOWUP_COLUMNSconstant instead of redefining it locally.🤖 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 `@packages/engine/src/eval-followups.ts` around lines 11 - 16, Extract CLOSED_FOLLOWUP_COLUMNS and its explanatory comment from packages/engine/src/eval-followups.ts lines 11-16 into a shared follow-up dedup utility, exporting the constant for reuse. Update packages/engine/src/pr-comment-handler.ts lines 4-9 to import the shared constant and remove its local duplicate, preserving the existing closed-column deduplication behavior in both call sites.
188-210: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist
listTasksout of the per-followUp loop to avoid N+1 store calls.
findOpenEvalFollowUpTaskIdissues its ownstore.listTasks({ slim: true })call, andmaterializeEvalFollowUpsinvokes it once per creatablefollowUp— so a run with several qualifying suggestions makes that many full task-list round trips.normalizeEvalFollowUpsabove already establishes the pattern of listing once and reusing the result across all drafts (Line 97); this new path should do the same.♻️ Proposed fix: list once, filter in memory
-async function findOpenEvalFollowUpTaskId( - store: TaskStore, - parentTaskId: string, - suggestionId: string, -): Promise<string | undefined> { - const tasks = await store.listTasks({ slim: true }).catch(() => []); - const match = tasks.find( +function findOpenEvalFollowUpTaskId( + tasks: readonly Pick<Task, "id" | "column" | "sourceParentTaskId" | "sourceMetadata">[], + parentTaskId: string, + suggestionId: string, +): string | undefined { + const match = tasks.find( (task) => task.id !== parentTaskId && !CLOSED_FOLLOWUP_COLUMNS.has(task.column) && task.sourceParentTaskId === parentTaskId && task.sourceMetadata?.suggestionId === suggestionId, ); return match?.id; }export async function materializeEvalFollowUps(input: MaterializeEvalFollowUpsInput): Promise<EvalFollowUpSuggestion[]> { const { parentTaskId, runId, policyMode, overallScore, followUps, store } = input; const created: EvalFollowUpSuggestion[] = []; + const existingTasks = await store.listTasks({ slim: true }).catch(() => []); for (const followUp of followUps) { if (!followUp.recommendation.shouldCreate || followUp.state !== "suggested") { created.push(followUp); continue; } - const existingTaskId = await findOpenEvalFollowUpTaskId(store, parentTaskId, followUp.suggestionId); + const existingTaskId = findOpenEvalFollowUpTaskId(existingTasks, parentTaskId, followUp.suggestionId);Also applies to: 221-256
🤖 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 `@packages/engine/src/eval-followups.ts` around lines 188 - 210, Hoist the task listing out of materializeEvalFollowUps and reuse one store.listTasks({ slim: true }) result for all follow-ups. Update findOpenEvalFollowUpTaskId to accept the preloaded task collection and only perform the existing in-memory filtering, preserving its fail-open behavior by handling listing errors once at the caller.
🤖 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.
Nitpick comments:
In `@packages/engine/src/eval-followups.ts`:
- Around line 11-16: Extract CLOSED_FOLLOWUP_COLUMNS and its explanatory comment
from packages/engine/src/eval-followups.ts lines 11-16 into a shared follow-up
dedup utility, exporting the constant for reuse. Update
packages/engine/src/pr-comment-handler.ts lines 4-9 to import the shared
constant and remove its local duplicate, preserving the existing closed-column
deduplication behavior in both call sites.
- Around line 188-210: Hoist the task listing out of materializeEvalFollowUps
and reuse one store.listTasks({ slim: true }) result for all follow-ups. Update
findOpenEvalFollowUpTaskId to accept the preloaded task collection and only
perform the existing in-memory filtering, preserving its fail-open behavior by
handling listing errors once at the caller.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b8cd6e5c-e801-4239-ae75-6b039c6e78bb
📒 Files selected for processing (20)
.changeset/delete-meta-task-and-recovery-followups.mddocs/architecture.mddocs/settings-reference.mdpackages/core/src/builtin-workflow-settings.tspackages/core/src/settings-schema.tspackages/core/src/types/settings-scope.tspackages/dashboard/app/components/settings/sections/__tests__/settings-default-descriptions.test.tsxpackages/engine/src/__tests__/eval-followups.test.tspackages/engine/src/__tests__/pr-comment-handler.test.tspackages/engine/src/__tests__/reliability-interactions/_helpers.tspackages/engine/src/__tests__/reliability-interactions/meta-archive-guard-composition.test.tspackages/engine/src/__tests__/reliability-interactions/meta-chain-auto-close.test.tspackages/engine/src/__tests__/self-healing-db-corruption.test.tspackages/engine/src/__tests__/self-healing-meta-archive-guards.test.tspackages/engine/src/eval-followups.tspackages/engine/src/pr-comment-handler.tspackages/engine/src/project-engine.tspackages/engine/src/run-audit.tspackages/engine/src/self-healing.tspackages/engine/src/verification-followup-dedup.ts
💤 Files with no reviewable changes (9)
- packages/engine/src/tests/reliability-interactions/meta-archive-guard-composition.test.ts
- packages/engine/src/tests/reliability-interactions/meta-chain-auto-close.test.ts
- docs/settings-reference.md
- packages/engine/src/tests/self-healing-meta-archive-guards.test.ts
- packages/dashboard/app/components/settings/sections/tests/settings-default-descriptions.test.tsx
- packages/engine/src/verification-followup-dedup.ts
- packages/core/src/settings-schema.ts
- packages/engine/src/tests/reliability-interactions/_helpers.ts
- packages/engine/src/tests/self-healing-db-corruption.test.ts
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.
Deletes two pieces of automated "meta" machinery that filed and garbage-collected cards restating state already on the task that failed. Net -1015 lines.
Why
Automated recovery follow-ups.
createAutomatedFollowupand its dedup engine (289 lines of signature matching, 1h recurrence rate-limiting, 24h supersedes windows) existed to file recovery cards for verification-cap and merge-conflict give-ups. In both cases the parent is already parkedfailedwith a descriptiveerrorand a log entry carrying the failing command, branch, and output — the card was a second copy of that.Meta-task auto-archive. The sweeps that garbage-collected those cards were worse than redundant: the regex classifier matched ordinary feature work, and its positional fallback bound cards to unrelated tasks, so live work could be archived.
They are removed together, because the auto-archive sweeps only existed to clean up after the follow-up engine.
What changed
Deleted
packages/engine/src/verification-followup-dedup.tsin full —createAutomatedFollowup,decideAutomatedFollowup,AutomatedFollowupKind,computeVerificationFailureSignature,extractFailingTestFiles.findActiveRecoveryFollowUp— dead code, defined and never called (tscindependently flagged it6133 declared but its value is never read).autoArchiveResolvedMetaTasks/autoArchiveStalledMetaTasksand helpersclassifyMetaTask/resolveMetaTargetTaskId/computeMetaChainDepth/archiveMetaTask/evaluateMetaAutoArchiveGuards, plus settingsmetaTaskStallAutoCloseMsandmetaTaskActiveExecutionGraceMs.task:auto-archived-meta-resolved,task:auto-archived-meta-stalled,task:auto-archive-meta-resolved-skipped,task:auto-archive-meta-stalled-skipped,verification:followup-created,verification:followup-deduped.The two signature helpers were deleted rather than relocated — once the three call sites went they were provably unreachable:
buildVerificationFailureSignaturehad exactly one caller, and it was the only caller ofextractFailingTestFiles.Call sites 1 and 2 — park kept, card dropped
Verification-cap and merge-conflict give-ups keep their park, audit event, operator comment, and log entry. Site 1's
errorstring was reworded off"See follow-up task for investigation."(no follow-up will exist) to carry the guidance itself.autoResolveDisabledwas kept — it still drives the outer park guard and thereasonstring; only the inner branch that guarded card creation is gone.Call site 3 — autostash orphan, replaced not deleted
This one is a genuine data-loss guard, so it keeps a durable trail. A
live-classified orphan is a merger stash holding real uncommitted work, and unlike sites 1–2 there is no parked parent — the parent may already bedoneand merged, so nothing else on the board would ever mention the stash.The card is replaced by a
logEntryand anaddTaskCommenton the parent, preserving every fact the old description carried: the sha,record.label(the handlegit stashrecovery needs),record.detectedByTaskId, andsourcePhase. New truthful run-audit eventtask:autostash-orphan-live-detectedreplaces the borrowedverification:followup-*name, with ids/outcomes-only metadata per AGENTS.md.Kept unchanged: the two real product features
Eval follow-ups (
eval-followups.ts) and PR-comment follow-ups (pr-comment-handler.ts) only borrowed the shared engine for its dedup pass. Both keep their exact behavior, column, priority,sourceType, and log lines, with dedup inlined as alistTasksscan onsuggestionId/prNumberrespectively. Both fail open (create) if the listing throws, matching the old engine.Test changes — read this one
Two tests asserted the deleted engine's rate-limited
"[verification recurrence]"logEntry. Those assertions were removed, not loosened: both tests still assert no duplicate card is created, and the eval test still asserts the existing id is reported back. No coverage of surviving behavior was weakened. The threemeta-*test files were deleted along with the sweeps they covered.Verification
Plus a file-scoped run over the touched surfaces (
eval-followups,pr-comment-handler,merger-autostash-orphan-surface,merger-autostash-cleanup,run-audit,run-audit-secret-taxonomy,project-engine,project-engine-manager): 213/213 passed.A repo-wide grep confirms no surviving references to any deleted symbol, module, or audit event.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Changes
Documentation