fix(auto-fix): harden sentinel-guarded artifact rehydration#45
Conversation
Deterministic gates that lazily seed an artifact via
`if [ ! -f '<path>' ]; then cat > '<path>' <<'EOF' ... <marker> EOF fi`
followed later by `tail -n 1 '<path>' | tr -d '[:space:]' | grep -E '^<marker>$'`
were unrecoverable when a leftover artifact from a prior run had trailing
junk on its last line: the `if [ ! -f ]` guard short-circuited, the
heredoc never re-ran, and the auto-fix loop replayed the same
guaranteed-to-fail command up to maxAttempts.
`repairWorkflowDeterministically` now detects this shape and rewrites
the guard to also re-rehydrate when the sentinel check fails:
if [ ! -f '<path>' ] || ! tail -n 1 '<path>' \
| tr -d '[:space:]' | grep -qE '^<marker>$'; then
cat > '<path>' <<'EOF'
...
EOF
fi
The repair only fires when a matching `tail -n 1 ... grep '^<marker>$'`
check exists elsewhere in the same workflow source, so unrelated
`if [ ! -f ]` seed blocks are left alone.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a deterministic repair pass that detects workflow shell blocks which rehydrate files with embedded sentinel markers and hardens them by inserting a tail/grep sentinel check; also added tests and helpers validating applied and non-applicable repair cases. ChangesSentinel-Guarded Rehydration Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/local/auto-fix-loop.ts (1)
547-549: ⚡ Quick winRemove duplicate helper; reuse existing
escapeRegExpat line 842.This
escapeRegexfunction is identical to the existingescapeRegExpfunction defined later in this file. Use the existing function to avoid duplication.♻️ Suggested fix
-function escapeRegex(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -}Then update the call site at line 536:
const sentinelCheckPattern = new RegExp( - `tail -n 1 '${escapeRegex(path)}'[^\\n]*\\| grep[^\\n]*'\\^${escapeRegex(marker)}\\$'`, + `tail -n 1 '${escapeRegExp(path)}'[^\\n]*\\| grep[^\\n]*'\\^${escapeRegExp(marker)}\\$'`, );🤖 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/auto-fix-loop.ts` around lines 547 - 549, Remove the duplicate escapeRegex helper and reuse the existing escapeRegExp function: delete the escapeRegex function declaration and update any call-sites that currently call escapeRegex (e.g., the call near where it was used) to call escapeRegExp instead, ensuring imports/exports (if any) and TypeScript types remain consistent; run a quick search in the file for "escapeRegex(" to replace all occurrences and keep only the single escapeRegExp implementation.
🤖 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 `@src/local/auto-fix-loop.ts`:
- Around line 547-549: Remove the duplicate escapeRegex helper and reuse the
existing escapeRegExp function: delete the escapeRegex function declaration and
update any call-sites that currently call escapeRegex (e.g., the call near where
it was used) to call escapeRegExp instead, ensuring imports/exports (if any) and
TypeScript types remain consistent; run a quick search in the file for
"escapeRegex(" to replace all occurrences and keep only the single escapeRegExp
implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dad0ec9a-ea5a-49f7-9b39-a21f25a359fc
📒 Files selected for processing (2)
src/local/auto-fix-loop.test.tssrc/local/auto-fix-loop.ts
![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: 785f25552b
ℹ️ 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".
|
|
||
| const next = content.replace(guardPattern, (match, path: string, indent: string, body: string, marker: string) => { | ||
| const sentinelCheckPattern = new RegExp( | ||
| `tail -n 1 '${escapeRegex(path)}'[^\\n]*\\| grep[^\\n]*'\\^${escapeRegex(marker)}\\$'`, |
There was a problem hiding this comment.
repairSentinelGuardedRehydration only recognizes later sentinel checks where the regex is single-quoted ('^…$'), because sentinelCheckPattern hardcodes that quote style. If a workflow uses the equally valid grep -Eq "^MARKER$" form, the if [ ! -f ... ]; then cat ... fi block is left unchanged, so retries can still loop on a corrupt leftover artifact instead of regenerating it. This makes the new recovery logic miss a common shell variant and leaves the original failure mode in place for those workflows.
Useful? React with 👍 / 👎.
1 participant
Summary
Closes the auto-fix gap that left a sage workflow stuck on
INVALID_ARTIFACT at final-review-pass-gatefor all 7 retries (the recovery on https://github.com/AgentWorkforce/sage/pull/199 had to be done by hand).Deterministic gates that lazily seed an artifact via
if [ ! -f '<path>' ]; then cat > '<path>' <<'EOF' ... <marker> EOF fifollowed later bytail -n 1 '<path>' | tr -d '[:space:]' | grep -E '^<marker>$'were unrecoverable when a leftover artifact from a prior run had trailing junk on its last line: the existence guard short-circuited, the heredoc never re-ran, and the auto-fix loop replayed the same guaranteed-to-fail command up tomaxAttempts. None of the existing deterministic repairs (repairAgentStepTimeouts,missingFileRepair*,repairOutputContainsEchoMismatches,repairUnknownStepTemplateRefs,repairBareGitDiffManifestGates) inspect artifact content, andworkforce-persona-repairerrewrites workflow definitions, not artifact bodies — so the loop made no progress.Fix
repairWorkflowDeterministicallynow detects this gate shape and rewrites the existence-only guard into a content-aware guard:if [ ! -f '<path>' ] || ! tail -n 1 '<path>' | tr -d '[:space:]' | grep -qE '^<marker>$'; then— which lets the next attempt regenerate the corrupt artifact and clear the gate.The repair only fires when a matching
tail -n 1 '<path>' | ... grep '^<marker>$'check exists elsewhere in the same workflow source. Unrelatedif [ ! -f ]seed blocks (e.g. one-shot README seeders) are left alone; a skip-test in the suite asserts this.End-to-end smoke
With a corrupted leftover artifact whose last line is the heredoc-bleed garbage seen in the original sage failure:
Test plan
npx tsc --noEmitcleannpx vitest run src/local/auto-fix-loop.test.ts— 15/15 (13 existing + 2 new)npx vitest run— 826/826 across 43 filesOut of scope (follow-up)
The corruption itself — a newline between
EOFand the trailing tail-check getting joined to&&somewhere in the pipeline — is a separate bug, almost certainly in command-string assembly when a heredoc-containing multi-line shell command is flattened. This PR only closes the recovery gap; happy to file a follow-up once the joiner is identified.🤖 Generated with Claude Code