Skip to content

fix(agent): escalate mirror flush retry log to error once per failure streak - #4330

Merged
kwakayama merged 2 commits into
mainfrom
issue/821-mirror-flush-retry
Aug 31, 2026
Merged

fix(agent): escalate mirror flush retry log to error once per failure streak#4330
kwakayama merged 2 commits into
mainfrom
issue/821-mirror-flush-retry

Conversation

@kwakayama

@kwakayama kwakayama commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

The hosted chunk mirror's retry-scheduled log ("Durable run mirror flush failed; queued for retry") escalated to error level with a >= comparison against HOSTED_CHUNK_MIRROR_RETRY_ERROR_THRESHOLD, so during a persistent outage every retry at/after the 5th attempt logged at error. Since run-mirror.ts retries indefinitely with backoff capped at 5s, one outage produced a captured Sentry error roughly every 5 seconds per active run (the unresolved VERYFRONT-AGENT-3 group). This change switches the escalation to strict equality (=== HOSTED_CHUNK_MIRROR_RETRY_ERROR_THRESHOLD): consecutiveFailures increments by exactly 1 per scheduled retry and resets to 0 on a successful flush (src/agent/conversation/durable.ts), so the error fires exactly once per failure streak and re-arms after recovery. Later attempts in the same streak stay at warn, and terminal stop/disable paths (auth_rejected, payload_too_large, cursor_resyncs_exhausted) keep reporting at error level. One error per persistent-outage streak remains expected signal for the Sentry group, not suppressed. Note: veryfront-agent consumes this code via the veryfront npm package and needs a dependency bump after release before VERYFRONT-AGENT-3 goes quiet.

Fixes veryfront/veryfront-issue-inbox#821

Red

deno task test:file src/agent/conversation/run-chunk-mirror.test.ts
agent/conversation-run-chunk-mirror ...
  escalates a persistent retry streak to error once, not on every attempt ... FAILED (2ms)

error: AssertionError: Values are not equal: a failure streak must escalate to error exactly once at the threshold; later attempts in the same streak stay at warn, and a recovered flush resets the streak so the next persistent outage escalates again

    [diff, expected (+) vs actual (-)]
      { consecutiveFailures: 1..4, level: "warn" }   (unchanged)
      { consecutiveFailures: 5, level: "error" }     (unchanged)
      { consecutiveFailures: 6,
-       level: "error",
+       level: "warn" }
      { consecutiveFailures: 7,
-       level: "error",
+       level: "warn" }
      { consecutiveFailures: 8,
-       level: "error",
+       level: "warn" }
      [second streak after recovery: warn 1..4, error at 5]  (unchanged)

    at src/agent/conversation/run-chunk-mirror.test.ts:690

FAILED | 0 passed (14 steps) | 1 failed (1 step) (14ms)

Green

agent/conversation-run-chunk-mirror ...
  escalates a persistent retry streak to error once, not on every attempt ... ok (2ms)
agent/conversation-run-chunk-mirror ... ok (11ms)
ok | 1 passed (15 steps) | 0 failed (12ms)

Suite results:

  • deno task test:file src/agent/conversation: ok | 21 passed (247 steps) | 0 failed (2s)
  • deno task test:file src/agent/hosted: ok | 498 passed (417 steps) | 0 failed (11s)
  • deno fmt --check / deno lint / deno check on src/agent/conversation/run-chunk-mirror.ts: all clean

Revert check

Recorded HEAD 3a22b25. git revert --no-commit 3a22b254c (clean revert, only src/agent/conversation/run-chunk-mirror.ts modified) -> deno task test:file src/agent/conversation/run-chunk-mirror.test.ts FAILED: "escalates a persistent retry streak to error once, not on every attempt ... FAILED" with the AssertionError at run-chunk-mirror.test.ts:690 (attempts 6-8 logged at error instead of warn) — "FAILED | 0 passed (14 steps) | 1 failed (1 step)". Restored with git reset --hard 3a22b254c -> same command PASSED — "ok | 1 passed (15 steps) | 0 failed (13ms)".

Acceptance criteria

  • With a persistently failing append endpoint (HTTP 503), the retry-scheduled log "Durable run mirror flush failed; queued for retry" is emitted at warn level for attempts 1-4 of a failure streak.
  • The escalation to error level fires exactly once per failure streak, at the attempt where consecutiveFailures first reaches the threshold (5); attempts 6+ in the same streak stay at warn.
  • A successful flush resets the streak, and a subsequent persistent outage in the same run escalates to error again (once, at its own threshold crossing).
  • Existing contracts intact: terminal stop/disable paths (auth_rejected, payload_too_large, cursor_resyncs_exhausted) still report at error level, run_terminal still warns, and the pre-existing "warns on early retry attempts and escalates to error at the failure threshold" test stays green.
  • Lint, typecheck (deno fmt/lint/check), and deno task test:file src/agent/conversation/run-chunk-mirror.test.ts pass after the fix.

The issue is filed in veryfront-issue-inbox against veryfront-agent's Sentry group, but the culprit code lives here: veryfront-agent consumes recordHostedChunkMirrorRetryScheduled from this repo via the veryfront npm package. No overlap with open PR #4326, which touches only src/agent/hosted/durable-run-event-sink.{ts,test.ts}; the two are compatible in either merge order.

Summary by CodeRabbit

  • Bug Fixes

    • Retry failures now escalate to an error exactly once at the configured threshold, while subsequent attempts remain warnings.
    • Retry escalation resets after a successful flush, improving log clarity for new failure streaks.
  • Tests

    • Added coverage verifying retry logging behavior across repeated failures and successful recovery.

Kentaro Wakayama added 2 commits August 31, 2026 00:41
… streak

The retry-scheduled log compared consecutiveFailures against the
threshold with >=, so every attempt at or past the fifth logged at error
level. With retry backoff capped at ~5s, one persistent append outage
emitted a Sentry error every few seconds per active run
(VERYFRONT-AGENT-3).

Compare with strict equality instead: the streak counter increments by
one per scheduled retry and resets to zero on a successful flush, so the
escalation fires exactly once per failure streak and re-arms after
recovery. Attempts 1-4 and 6+ of a streak stay at warn; terminal
stop/disable paths still report at error level.
@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 06f5725f-5196-4bed-87f8-a2931423cc37

📥 Commits

Reviewing files that changed from the base of the PR and between a75d7fd and 3a22b25.

📒 Files selected for processing (2)
  • src/agent/conversation/run-chunk-mirror.test.ts
  • src/agent/conversation/run-chunk-mirror.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The hosted chunk mirror now logs one error when retries reach the failure threshold. Later retries remain warnings. A successful flush resets escalation. Tests cover re-escalation during a later failure streak.

Changes

Hosted mirror retry escalation

Layer / File(s) Summary
Retry escalation behavior
src/agent/conversation/run-chunk-mirror.ts, src/agent/conversation/run-chunk-mirror.test.ts
The retry logger uses strict threshold equality for error escalation. Regression tests verify warning retries, single error escalation, recovery reset, and later re-escalation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 3a22b

This change limits repeated error reporting during persistent mirror outages while preserving retry, recovery, and terminal-failure behavior. No actionable merge-blocking risk remains after the reported checks pass.

Suggested reviewers: kojiwakayama

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: mirror flush retry logs escalate to error once per failure streak.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue/821-mirror-flush-retry

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 288 2232 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@gitar-bot

gitar-bot Bot commented Aug 30, 2026

Copy link
Copy Markdown

Note

Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime.
Learn more

Code Review ✅ Approved

Changes the mirror flush retry log escalation from >= to === comparison, so error-level logging fires exactly once per failure streak at the threshold instead of on every retry attempt thereafter. Fixes excessive Sentry noise during persistent outages while preserving one error per streak as expected signal. All tests pass, lint/typecheck clean, and terminal paths (auth_rejected, payload_too_large, cursor_resyncs_exhausted) retain error-level reporting. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud

Copy link
Copy Markdown

@kwakayama

Copy link
Copy Markdown
Contributor Author

Reviewer: Codex
Reviewed SHA: 3a22b254c3533a056b8752cb8bb3e318aa7c43c8
Base: a75d7fdc5bb87b458cde1ec979de5b786cafec19

Findings, ordered by severity

  • No critical, high, medium, or low actionable findings.
  • The production diff is limited to the retry-level comparison at run-chunk-mirror.ts#L296. The queue counter increments once for each scheduled retry at durable.ts#L553-L560 and resets after a successful flush at durable.ts#L814-L816. The separate terminal stop paths remain error-level where required at run-chunk-mirror.ts#L325-L399.
  • The added regression test at run-chunk-mirror.test.ts#L632-L710 covers attempts 1-8, successful recovery, and a second failure streak through threshold 5. Existing neighboring tests cover terminal auth, terminal-run, and oversized-event outcomes.

Score breakdown: correctness/completeness 40/40, regression tests/verification 19/20, reliability/security 15/15, repository standards/maintainability 15/15, scope/docs/rollout clarity 9/10. Score: 98/100.

Verification and current PR state

  • git diff --check, deno fmt --check on both changed files, and deno lint on both changed files pass.
  • The local deno task test:file src/agent/conversation/run-chunk-mirror.test.ts could not execute the tests because this checkout pins Deno 2.7.7 while the available binary is Deno 2.9.4, which trips the repository runtime compatibility guard. The local source check also reports Deno-version-dependent errors outside this diff. GitHub exact-head CI reports 47 passed checks, including format, lint, typecheck, unit, integration, coverage, CodeQL, and Sonar; 0 failed; 1 pending check, Automated review.
  • Eleven checks are skipped: dispatch release, prerelease, release, quality gate (registry), update-homebrew, invalidate unverified review proof, close unresolved merge group review gate, reuse exact-head review for merge group, build-binaries, tests (split mode), and version-check.
  • Review metadata is complete: 5 bot issue comments, 0 inline review comments, 0 submitted reviews, and 0 unresolved threads. No other reviewer conclusions were used.

Verdict: APPROVE for the code change. Merge readiness remains blocked by the draft state and pending Automated review check.

Review-Gate:
Reviewer: Codex
Reviewed-SHA: 3a22b25
Score: 98/100
Actionable-Findings: 0
Verdict: APPROVE

@kwakayama

Copy link
Copy Markdown
Contributor Author

Reviewer: Claude
Reviewed SHA: 3a22b254c3533a056b8752cb8bb3e318aa7c43c8
Base: a75d7fdc5bb87b458cde1ec979de5b786cafec19

Findings, ordered by severity

  • No critical, high, medium, or low actionable findings.
  • The exact production diff changes the retry escalation comparison at src/agent/conversation/run-chunk-mirror.ts:296 from >= to ===. Static cross-module inspection confirmed consecutiveFailures increments by one for each retry-scheduled outcome in durable.ts:558, resets on a successful flush in durable.ts:815 and durable.ts:948, and starts at zero in durable.ts:870.
  • The regression test in src/agent/conversation/run-chunk-mirror.test.ts:632 proves warnings at attempts 1-4, one error at attempt 5, warnings at attempts 6-8, recovery, and a second streak that re-escalates once at its own fifth failure. Existing terminal error paths remain separate and unchanged.

Verification and gaps

  • Reviewed the complete pinned BASE...HEAD diff and surrounding counter/reset implementation.
  • The Claude advisor sandbox denied its gh and test commands, so it could not independently execute tests or inspect remote threads/checks. The full raw advisor output and prompt are preserved in .omx/artifacts/claude-you-are-an-independent-fresh-context-code-reviewer-review-on-2026-08-31T05-52-04-720Z.md.

Score breakdown: correctness/completeness 38/40, regression tests/verification 17/20, reliability/security 14/15, repository standards/maintainability 13/15, scope/docs/rollout clarity 8/10. Score: 90/100.

Verdict: APPROVE.

Review-Gate:
Reviewer: Claude
Reviewed-SHA: 3a22b25
Score: 90/100
Actionable-Findings: 0
Verdict: APPROVE

@kwakayama
kwakayama marked this pull request as ready for review August 31, 2026 05:54

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-08-31T05:54:33.778136Z 3a22b25 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 3a22b254c3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@kwakayama
kwakayama added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit b4fc9ed Aug 31, 2026
74 checks passed
@kwakayama
kwakayama deleted the issue/821-mirror-flush-retry branch August 31, 2026 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant