feat(auto-fix): add code-drift repair path for verify-style failures#69
Merged
Conversation
Currently, when a workflow fails because it has detected drift between
target source code and an external reference (provider docs, schema,
etc.), ricky's auto-fix loop dispatches workforce-persona-repairer to
patch the workflow file — the wrong target. The workflow already worked;
the source code under inspection is what needs fixing.
This commit adds a parallel auto-fix path that:
1. Detects "code drift" failures by scanning for structured drift
reports under <cwd>/.workflow-artifacts/**/*-drift.json with
verdict === 'DRIFT' and at least one blocker- or major-severity
finding. Reports are validated against a generic schema (verdict +
findings shape) and freshness-checked against the run start time.
2. Dispatches a new repairer (repairCodeFromDriftArtifacts) that builds
a code-fix prompt from the report content and sends it via the
existing workforce-persona resolver. The agent edits target source
files in place via its harness; ricky then retries the workflow
normally, which re-validates by re-running the same gates that
previously caught the drift.
3. Falls back to the existing workflow-repair path if the agent
returns applied=false or throws — drift detection is convention-
based, and a false positive should not block workflow repair.
The dispatch is preferred over workflow repair: when both apply (drift
artifacts present AND a workflow artifact resolves), code-drift wins.
This matches the intent: verify-style workflows are designed to surface
findings about target code, not about themselves.
Detection design:
- Convention-based: any workflow that writes drift-shaped JSON gets
auto-repair for free (no per-workflow opt-in needed).
- Schema-validated: file must parse with verdict + findings shape.
- Freshness-checked: artifact mtime must be >= runStartTimeMs to
ignore stale reports from prior runs.
The new file (code-drift-repairer.ts, ~280 lines) mirrors the shape of
workforce-persona-repairer.ts but with a code-fix prompt and a sentinel-
based completion contract (CODE_DRIFT_REPAIR_COMPLETE) — the agent
edits files in place rather than returning a workflow artifact for
ricky to write.
Default severity threshold: blocker + major. Minor findings are skipped
unless trivial to bundle in. (Future: configurable per workflow.)
The auto-fix-loop change is additive: existing workflow-repair behavior
is unchanged when no drift artifacts are present.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds code-drift auto-repair support to the auto-fix loop. When drift artifacts are discovered under ChangesCode Drift Auto-Repair Feature
Sequence Diagram(s)sequenceDiagram
Client->>Runner: start runWithAutoFix
Runner->>Discover: discoverDriftReports(cwd, runStartTimeMs)
Discover-->>Runner: reports (or null)
Runner->>Repairer: invoke codeDriftRepairer(target)
Repairer->>Harness: dispatch repair task
Harness-->>Repairer: output + CODE_DRIFT_REPAIR_COMPLETE
Repairer-->>Runner: CodeDriftRepairResult (applied/summary)
Runner->>Runner: record applied_fix / retry or fall back to workflow repair
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2447282c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| attempt: attempt + 1, | ||
| maxAttempts, | ||
| ...(runId ? { previousRunId: runId, retryOfRunId: retryOfRunId ?? runId } : {}), | ||
| ...(failedStep ? { startFromStep: failedStep } : {}), |
There was a problem hiding this comment.
When a verify-style workflow fails in a final gate that reads *-drift.json, retrying with startFromStep: failedStep skips the earlier step that regenerates the drift report from the now-edited source code. I checked the local runner path in src/local/entrypoint.ts, which forwards this field as startFrom, so the retry can re-read the same fresh drift artifact, fail again, and keep dispatching code-drift repair until max attempts instead of proving the code fix cleared the drift. The code-drift path should rerun the whole workflow, or at least the drift-generation step, rather than resuming at the failing gate.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/local/code-drift-repairer.ts (3)
294-334: ⚡ Quick winUse static import for
statinstead of dynamic import in loop.Line 296 dynamically imports
node:fs/promisesto callstat, butreadFileis already statically imported at the top of the file. This inconsistency causes repeated module resolution overhead on each iteration.♻️ Proposed fix
At the top of the file, update the import:
-import { readFile } from 'node:fs/promises'; +import { readFile, stat } from 'node:fs/promises';Then in the loop:
- const stat = await (await import('node:fs/promises')).stat(filePath); + const fileStat = await stat(filePath); - if (stat.mtimeMs < runStartTimeMs) continue; + if (fileStat.mtimeMs < runStartTimeMs) continue;🤖 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 `@src/local/code-drift-repairer.ts` around lines 294 - 334, The loop currently does a dynamic import of node:fs/promises to call stat on each drift file which is inefficient; change to a static import of stat alongside the existing readFile import at the top of the file, then replace the dynamic import call in the loop (the await (await import('node:fs/promises')).stat(filePath) expression) with a direct call to stat(filePath). Ensure no other code paths rely on the dynamic import and run tests or linting to confirm the module resolution is correct for the code in function(s) handling drift files (the loop that builds DriftReport using isDriftReportShape, actionableFindings, readFile, etc.).
339-362: 💤 Low valueRemove unnecessary dynamic import.
collectDriftJsonFilesdynamically importsnode:fs/promiseswhenreadFile(and ideallyreaddir) should be statically imported at the module level. This is inconsistent with the existing static import.♻️ Proposed fix
Update the static import at top of file:
-import { readFile } from 'node:fs/promises'; +import { readdir, readFile, stat } from 'node:fs/promises';Then update the function:
async function collectDriftJsonFiles(dir: string): Promise<string[]> { - const fs = await import('node:fs/promises'); const out: string[] = []; async function walk(current: string): Promise<void> { let entries: import('node:fs').Dirent[]; try { - entries = await fs.readdir(current, { withFileTypes: true }); + entries = await readdir(current, { withFileTypes: true }); } catch {🤖 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 `@src/local/code-drift-repairer.ts` around lines 339 - 362, collectDriftJsonFiles currently uses a dynamic import of 'node:fs/promises'; remove that dynamic import and use the module-level static import instead so the code is consistent with other file I/O. Modify the top-of-file imports to include the promised fs API (e.g., import { readdir } from 'node:fs/promises' or import * as fsPromises) and then update collectDriftJsonFiles to call readdir (and any other used functions like readFile if applicable) directly (e.g., replace await import(...) and fs.readdir(...) with readdir(current, { withFileTypes: true })) while keeping the walk logic and Dirent checks the same.
322-325: 💤 Low valueType cast for
sourcesdoesn't validate string values.
isPlainRecordonly confirmssourcesValueis a non-null object, not that all values are strings. If a drift report has non-string values insources, they'll be silently accepted but violate theRecord<string, string>type.🛡️ Optional stricter validation
const sourcesValue = parsedRecord.sources; - if (isPlainRecord(sourcesValue)) { - report.sources = sourcesValue as Record<string, string>; + if (isPlainRecord(sourcesValue) && Object.values(sourcesValue).every((v) => typeof v === 'string')) { + report.sources = sourcesValue as Record<string, string>; }🤖 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 `@src/local/code-drift-repairer.ts` around lines 322 - 325, The code is casting parsedRecord.sources to Record<string,string> after only calling isPlainRecord (which only checks for a non-null object); update validation so you confirm every value is a string before assigning to report.sources: inspect parsedRecord.sources (sourcesValue) and check Object.entries/some/all values are typeof "string", only then set report.sources = sourcesValue as Record<string,string>, otherwise handle/skip/throw as appropriate; keep references to parsedRecord, sourcesValue, isPlainRecord, and report.sources when making the change.
🤖 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 `@src/local/code-drift-repairer.ts`:
- Around line 364-378: The isDriftReportShape type guard currently only checks
each finding's severity; update it (the isDriftReportShape function) to also
validate that each finding has axis and description properties of type string
(and not undefined) before returning true, e.g., extend the findings.every
callback to verify typeof (f as { axis?: unknown }).axis === 'string' and typeof
(f as { description?: unknown }).description === 'string' along with the
existing severity checks so malformed JSON won't be cast to DriftFinding[].
---
Nitpick comments:
In `@src/local/code-drift-repairer.ts`:
- Around line 294-334: The loop currently does a dynamic import of
node:fs/promises to call stat on each drift file which is inefficient; change to
a static import of stat alongside the existing readFile import at the top of the
file, then replace the dynamic import call in the loop (the await (await
import('node:fs/promises')).stat(filePath) expression) with a direct call to
stat(filePath). Ensure no other code paths rely on the dynamic import and run
tests or linting to confirm the module resolution is correct for the code in
function(s) handling drift files (the loop that builds DriftReport using
isDriftReportShape, actionableFindings, readFile, etc.).
- Around line 339-362: collectDriftJsonFiles currently uses a dynamic import of
'node:fs/promises'; remove that dynamic import and use the module-level static
import instead so the code is consistent with other file I/O. Modify the
top-of-file imports to include the promised fs API (e.g., import { readdir }
from 'node:fs/promises' or import * as fsPromises) and then update
collectDriftJsonFiles to call readdir (and any other used functions like
readFile if applicable) directly (e.g., replace await import(...) and
fs.readdir(...) with readdir(current, { withFileTypes: true })) while keeping
the walk logic and Dirent checks the same.
- Around line 322-325: The code is casting parsedRecord.sources to
Record<string,string> after only calling isPlainRecord (which only checks for a
non-null object); update validation so you confirm every value is a string
before assigning to report.sources: inspect parsedRecord.sources (sourcesValue)
and check Object.entries/some/all values are typeof "string", only then set
report.sources = sourcesValue as Record<string,string>, otherwise
handle/skip/throw as appropriate; keep references to parsedRecord, sourcesValue,
isPlainRecord, and report.sources when making the change.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e432dca-2d60-49c4-bfd1-75c3ff841b7a
📒 Files selected for processing (2)
src/local/auto-fix-loop.tssrc/local/code-drift-repairer.ts
Default remains ON: when ricky's auto-fix loop is enabled and the
workflow emits drift-shaped artifacts, ricky dispatches the code-fix
repairer.
This commit adds a fail-safe per-report opt-out so monitoring/audit
workflows that want findings surfaced for human triage (not auto-edited)
can declare so. Workflow emits `"autofix": false` in the report JSON;
ricky parses the report, records the opt-out, and skips dispatch.
- DriftReport interface gains `autofix: boolean` (defaulting to true).
- discoverDriftReports treats only the exact boolean `false` as opt-out.
Any other value (true, missing, "no", null) is treated as default ON,
so a workflow that misspells the field still gets repaired — the
default-useful behavior.
- Filter applies after parsing so future telemetry can still see the
opt-out without needing a separate scan.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ry, tighter schema [Codex P2] code-drift retry must rerun drift-producing steps Previously, after a successful code-drift repair, the retry resumed at the failed step (the aggregation gate that reads *-drift.json). That gate just re-reads the SAME pre-fix drift artifacts and fails again, looping until max attempts and never proving the fix cleared the drift. The verify agent steps that PRODUCE the drift reports must re-run against the patched source. Removed `startFromStep` from the code-drift retry — the workflow restarts from the root. Pays the cost of re-running successful steps (verify agents re-fetch external docs), but correctness wins. The progress message and retry reason both reflect the full restart so observers can see why the cost was paid. Workflow-repair retries (workforce-persona path) keep `startFromStep` unchanged — that path patches the workflow file itself, so resuming at the failed step is the right semantic. [CodeRabbit] strengthen DriftFinding schema validation isDriftReportShape only checked `severity`. Malformed JSON missing axis or description would pass validation, then get cast to DriftFinding[], surfacing `undefined` in the repair prompt — degrading the agent's ability to locate the offending code. Added typeof string + non-empty axis check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/local/auto-fix-loop.ts`:
- Line 118: Discovery currently keys artifacts to a single run-level variable
runStartTimeMs, letting stale drift reports from earlier attempts be reused;
update the logic so each attempt has its own attemptStartTimeMs (or attempt
window timestamps) and use that when discovering/filtering artifacts for
codeDriftRepairer and any discovery calls (replace uses of runStartTimeMs in
discovery with the attempt-specific timestamp and/or add a filter that checks
artifact.createdAt or artifact.attemptId falls inside the current attempt
window) so only artifacts produced during the current attempt are considered.
In `@src/local/code-drift-repairer.ts`:
- Around line 394-401: The validation in the findings.every(...) callback allows
empty or whitespace-only descriptions; update the check for description (the
variable named description in the callback) to require a non-empty,
non-whitespace string (e.g., typeof description === 'string' &&
description.trim().length > 0) while keeping the existing checks for
isPlainRecord, sev ('blocker'|'major'|'minor') and axis; adjust the final return
condition in that anonymous function accordingly so empty/whitespace
descriptions are rejected.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fd56e51-ecea-4388-a343-02855b6d19e3
📒 Files selected for processing (2)
src/local/auto-fix-loop.tssrc/local/code-drift-repairer.ts
Two more PR #69 review findings. [CodeRabbit major] Drift artifact freshness keyed to run-level timestamp Previously discoverDriftReports used `runStartTimeMs` (captured once before the attempt loop) as the freshness floor. That means stale drift artifacts produced by attempt N could be re-considered "fresh" on attempt N+1, re-triggering codeDriftRepairer for findings that earlier attempts already addressed (or that belong to a different failure path in the same run). Added `attemptStartTimeMs` per attempt and pass `Math.max(runStartTimeMs, attemptStartTimeMs)` to discoverDriftReports so each attempt only considers artifacts the CURRENT attempt produced. The Math.max guards against backward clock skew between attempts (NTP correction during a long run) which would otherwise cause attemptStartTimeMs to drop below runStartTimeMs. [CodeRabbit minor] description: '' or whitespace passed schema validation isDriftReportShape required `description` to be a string but allowed empty/whitespace values. That weakened the "tightened schema" intent from the previous commit and could feed empty findings into the agent repair prompt. Added trim().length > 0 for both axis and description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 9, 2026
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.
Summary
Adds a parallel auto-fix path so verify-style workflows (those that produce drift reports about target source code, not about themselves) get the right kind of repair — patching the target code, not the workflow file.
Motivation
When a workflow fails because it surfaced drift between target code and a reference (provider docs, schema, etc.), the existing auto-fix dispatches `workforce-persona-repairer` to patch the workflow file. That's the wrong target — the workflow worked correctly; the code under inspection is what needs fixing. Result: the persona repairer either rewrites a working workflow into something subtly different, or escalates because it can't make sense of the failure.
Concrete case: `workflows/044-tier1-verify.ts` in relayfile-adapters fetches HubSpot/Stripe/etc. webhook docs and diffs against our adapter implementation. When it finds the implementation has the wrong header name or algorithm, it writes a drift report and exits non-zero. Today that triggers a useless workflow rewrite. With this change, ricky reads the drift report, dispatches a code-fix agent that patches the offending `webhook-normalizer.ts`, and retries — which now passes.
Design
Detection (`discoverDriftReports`)
Convention-based + schema-validated + freshness-checked:
Dispatch (`auto-fix-loop.ts`)
Repairer (`code-drift-repairer.ts`)
Backwards compatibility
Strictly additive:
What it doesn't do (yet)
Test plan
Risk
Medium. Touches `auto-fix-loop.ts` (1642-line core file), but the change is a single new code path with try/catch and fallback to existing behavior. Real risk is the prompt itself — if the code-fix agent makes wrong edits, it could break source files. Mitigations: severity threshold (blocker+major only), workflow's own gates re-run on retry to catch regressions, max-attempts cap (default 7).
🤖 Generated with Claude Code