Skip to content

fix: surface full error chain on task failure; fix index-task failure banner label - #617

Merged
leghadjeu-christian merged 3 commits into
mainfrom
fix/surface-full-error-chain-on-task-failure
Aug 18, 2026
Merged

fix: surface full error chain on task failure; fix index-task failure banner label#617
leghadjeu-christian merged 3 commits into
mainfrom
fix/surface-full-error-chain-on-task-failure

Conversation

@leghadjeu-christian

@leghadjeu-christian leghadjeu-christian commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

1. Summary

This PR changes:

  • services/agent-runner/src/run.rs: the terminal task-failure branch now formats the reported
    error with format!("{error:#}") instead of error.to_string().
  • apps/web/lib/domain/tasks.ts / apps/web/app/dashboard/runs/[id]/page.tsx: the run-detail
    failure banner no longer says "Review did not post: ..." for a bare index task (which never
    attempts a review) — it now says "Indexing failed: ..." for target_type: "repository" tasks,
    and keeps the existing "Review did not post: ..." wording for pull_request/issue tasks.

It solves two related but independent problems found while tracing a real production failure:

  • Task failures were reported to the control plane (and shown on the LCI dashboard) with only the
    outermost anyhow context string — e.g. "embedding batch 27" — silently dropping the actual
    root cause chained underneath.
  • The same failure's dashboard banner read "Review did not post: embedding batch 27", which is
    misleading for an index-only task: there was never a review attempt to "not post" in the first
    place, only a failed re-index.

2. Intent

Error chain: anyhow::Error's plain Display (.to_string() / {error}) prints only the
outermost .context(...) message. index_checkout wraps embeddings failures with
.with_context(|| format!("embedding batch {batch_idx}")), and the embeddings client itself
(services/agent-clients/src/embeddings.rs) already builds a detailed cause one level down (HTTP
status + response body snippet for a rejected request). None of that ever reached a log line,
error_detail, or the dashboard — it was generated and then thrown away at the top of
run_once_body. This is inconsistent with the review-failure path in the very same file
(run.rs:631), which already uses format!("review run failed: {error:#}") for exactly this
reason.

Banner label: the run-detail page's error banner is a fixed string regardless of what kind of
task failed. An index task (command: "index", target_type: "repository", e.g. triggered by a
GitLab push with no PR involved) never invokes the review agent at all, so telling the operator
"Review did not post" when indexing itself failed is actively confusing — it points troubleshooting
in the wrong direction.


3. Scope

In Scope

  • run.rs's terminal RunOnceOutcome::Ran(Err(error)) branch: error.to_string()
    format!("{error:#}").
  • A failureNoticePrefix(task) helper in tasks.ts that picks "Indexing failed" for
    target_type: "repository" and "Review did not post" otherwise, used in both failure-banner
    spots on the run-detail page.

Out of Scope

  • Three earlier call sites in the same Rust function (run.rs:86,98,113, the config-resolution
    failure branches) have the identical error.to_string() pattern. Flagging, not fixing here, to
    keep this diff minimal and reviewable — happy to follow up separately.
  • The runs LIST page (as opposed to the run-detail page) — this PR only touches the detail page's
    banner, which is where the misleading text was reported.

4. Verification

I verified this change by:

  • Running automated tests
  • Checking logs

Commands run:

cargo build --workspace
cargo test -p agent-runner --lib run::
cd apps/web && node_modules/.bin/tsc --noEmit -p tsconfig.json
npx biome check apps/web/lib/domain/tasks.ts "apps/web/app/dashboard/runs/[id]/page.tsx"

Results:

$ cargo build --workspace
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 14.71s

$ cargo test -p agent-runner --lib run::
running 5 tests
test run::tests::model_override_replaces_the_preset_model_and_nothing_else ... ok
test run::tests::blank_override_is_treated_as_no_override ... ok
test run::tests::no_override_leaves_the_preset_model_unchanged ... ok
test run::tests::exhausted_pr_open_entry_point_gets_fast_banner_even_under_a_custom_preset_name ... ok
test run::tests::exhausted_non_pr_open_entry_point_gets_truncation_note ... ok
test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 77 filtered out

$ tsc --noEmit -p tsconfig.json
(no output — clean)

$ biome check ...
Checked 2 files in 67ms. No fixes applied.

No new automated test was added for either change: the Rust branch (run_once_body) isn't
unit-testable without mocking the entire task pipeline (checkout, indexing, review), and the
{error:#} chain behavior it relies on is already exercised by agent-clients' own
embeddings.rs test suite. failureNoticePrefix is a trivial pure string-picker with no branching
logic worth a dedicated test beyond the typecheck/lint above.


5. Screenshots / Evidence

Traced from a real production failure: task 407bfd11-99ba-48de-9632-e91b194ac483
(adorsys/xs2a/qwac-assessor, re-index triggered by a GitLab push, target_type: "repository")
failed twice, byte-identically, reporting only error_detail: "embedding batch 27", surfaced on
the dashboard as "Review did not post: embedding batch 27" — misleading, since this was a bare
index task with no review ever attempted.

Cross-referencing the Envoy AI Gateway access logs in Loki
({service_name="envoy-ai-gateway"} |= "407bfd11-99ba-48de-9632-e91b194ac483") showed the real
cause the runner discarded: both attempts got HTTP 400 from api.fireworks.ai for that batch,
with a request body of 244,907 bytes (~2x any successful batch in the same run) — consistent with
one oversized chunk exceeding qwen3-embedding-8b's 32,768-token input limit. With the error-chain
fix, error_detail would have included that HTTP status + body directly, no gateway-log
spelunking required; with the banner fix, the dashboard would have correctly read
"Indexing failed: ..." instead of pointing at a nonexistent review.


6. Risk Assessment

Risk level:

  • Low

Potential risks:

  • error_detail strings get longer for multi-level errors.
  • Banner copy changes for index-task failures — purely cosmetic, no behavior/data change.

Mitigation:

  • No known consumer parses error_detail structurally — it's rendered as free text on the
    dashboard and logged as-is. Both changes are additive/cosmetic, no control-flow change.

7. AI Usage Declaration

AI was used for:

  • Understanding existing code
  • Generating code
  • Reviewing the diff

Human verification:

  • I understand every meaningful change in this PR
  • I checked generated code manually
  • I removed unsupported AI assumptions
  • I accept responsibility for this PR

8. Reviewer Focus

Please focus your review on:

  • Correctness
  • Maintainability
  • Product intent

`error.to_string()` on an anyhow::Error only prints the outermost
`.context(...)` message and drops everything chained underneath. A
task failing mid-indexing reported only "embedding batch 27" with no
indication of *why* — the embeddings client already builds a detailed
cause (HTTP status + response body) one level down, but it was
silently discarded before ever reaching a log line, error_detail, or
the dashboard.

Use `{error:#}` instead, mirroring the review-failure path a few
lines below which already does this correctly.

Traced via task 407bfd11-99ba-48de-9632-e91b194ac483: the runner
reported only "embedding batch 27", but the Envoy AI Gateway access
logs showed the real cause was an HTTP 400 from api.fireworks.ai on
an oversized batch (244,907 bytes, ~2x any successful batch in the
same run). This fix means the next such failure self-reports that
detail instead of requiring a manual gateway-log trace.
@github-actions

Copy link
Copy Markdown
Contributor

❌ AI Governance check failed

This PR description is missing the following governance requirements:

  • a source-of-truth reference in the summary/intent — a URL or a #123 issue/PR reference (an evidence-only or boilerplate governance link does not count).

Please update the PR description using the pull request template and edit the PR body to re-run the check.

AI may accelerate the work, but it must not launder ignorance into polished artifacts.
See: https://adorsys-gis.github.io/ai-governance/

A bare index task (target_type "repository") never attempts a
review, so its failure banner shouldn't say "Review did not post: ...".
Added failureNoticePrefix() to pick "Indexing failed" for those and
kept "Review did not post" for pull_request/issue tasks. Also trims
the previous commit's comment down to one line per review feedback.
@leghadjeu-christian leghadjeu-christian changed the title fix(agent-runner): surface full error chain on task failure fix: surface full error chain on task failure; fix index-task failure banner label Aug 18, 2026
Reword both comments to describe what the code does rather than the
history of the change that introduced it.
@leghadjeu-christian
leghadjeu-christian merged commit 94f2a6c into main Aug 18, 2026
9 of 10 checks passed
@leghadjeu-christian
leghadjeu-christian deleted the fix/surface-full-error-chain-on-task-failure branch August 18, 2026 09:52
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