fix(server): log server-side agent stream failures

[kwakayama]

Problem

Every agent run against a preview environment on staging fails with:

{"title":"Initialization failed","status":500,"category":"GENERAL"}

That is the entire diagnostic. There is no detail in the response, nothing in Loki, and nothing in Sentry — I checked all four Sentry projects over the failure window and found zero events.

The cause is agent-stream.handler.ts:

if (isVeryfrontError(error)) {
  const response = errorToResponse(error, new URL(req.url).pathname);
  return this.respond(applyBuilderHeaders(response, builder.headers));   // never logged
}

Only the generic non-VeryfrontError fallback below it logs. Meanwhile errorToResponse strips detail from 5xx bodies (http-error.ts:111) so internals never reach the caller — correct — but the handler keeps no record either, so the detail is destroyed on both sides at once.

It matters because the runtime has three distinct throws behind that one title, in cloud-agent-config.ts alone:

• "Agent service context has not been initialized."
• "Agent service Skill parser has not been initialized."
• the default-agent-id equivalent

From the outside they are indistinguishable. I ruled out the skill-parser branch by running an agent that declares no skills, and ruled out two further hypotheses (missing veryfront.config.*, multi-agent ambiguity) the same way — by elimination against production, because there was no other way to tell.

Fix

Log slug, category, detail and cause for 5xx on that branch.

The response body is unchanged, so nothing new is exposed to the caller. 4xx stays quiet — those are client errors and already carry their own detail in the body.

Evidence this is the right branch

When I tried to drive this path in a test, my injected error took the fallback branch instead, which emitted two log lines. Production shows neither of those lines in Loki for the failing runs — so production really is taking the silent isVeryfrontError branch. That mismatch is what confirmed the diagnosis.

Verification

deno check, deno lint, deno fmt --check clean. agent-stream.handler.test.ts — 41 steps, 0 failed.

Not included

No test. I attempted one and could not get the injected error onto this branch — it kept landing on the fallback. I would rather ship the fix without a test than ship a test that passes for the wrong reason on a path it never reaches. Worth adding once someone can construct a VeryfrontError that survives to this catch.

Follow-up

This is a diagnostic unblock, not the underlying fix. Once deployed, the next failing preview run names its own cause in one log line, and the real "Initialization failed" bug can be fixed directly.

Refs veryfront-issue-inbox#356

Summary by CodeRabbit

• Bug Fixes

  • Improved diagnostics for server errors, helping support teams investigate failed requests more effectively.
  • Error details remain protected from exposure in responses when service failures occur.

• Tests

  • Added coverage to verify error logging and sensitive information handling for server failures.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

[coderabbitai]

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: 8948aa82-2fda-4a2f-a8e7-ae8cfc52704d

📥 Commits

Reviewing files that changed from the base of the PR and between 8a2bd85 and 6a58860.

📒 Files selected for processing (1)

• src/server/handlers/request/agent-stream.handler.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/server/handlers/request/agent-stream.handler.test.ts

---

📝 Walkthrough

Walkthrough

The agent stream handler now logs project context and error metadata for 5xx Veryfront errors. A helper normalizes unknown causes. A regression test verifies logged details and confirms that the HTTP response omits sensitive error detail.

Changes

Veryfront error observability

Layer / File(s)	Summary
Error cause normalization and structured logging src/server/handlers/request/agent-stream.handler.ts	A helper converts unknown causes to optional strings. The handler logs project context and error metadata for 5xx Veryfront responses before returning the existing response.
Test for 5xx error logging src/server/handlers/request/agent-stream.handler.test.ts	A regression test triggers SERVICE_OVERLOADED, checks the 5xx response, and verifies the log contains the detail, cause, slug, and category.

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

Suggested reviewers: kojiwakayama, copilot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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 server-side agent stream logging change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/log-server-side-agent-stream-errors

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 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/server/handlers/request/agent-stream.handler.ts`:
- Around line 943-961: Add a focused test for the catch path in handle that
throws a registered VeryfrontError with status >= 500, ensuring execution
reaches the isVeryfrontError branch. Mock or spy on logger.error and assert it
receives the error’s slug, category, detail, and cause, while also asserting the
returned response body remains unchanged.

🪄 Autofix

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 613d87b1-a0c8-4ba0-bada-f7a44bca2117

📥 Commits

Reviewing files that changed from the base of the PR and between f20f0d9 and 9d76e66.

📒 Files selected for processing (1)

• src/server/handlers/request/agent-stream.handler.ts

[Copilot]

Pull request overview

This PR addresses missing diagnostics when AgentStreamHandler catches a VeryfrontError and converts it to an HTTP response via errorToResponse, which intentionally strips detail for 5xx responses. The change adds a server-side log entry for 5xx errors on that previously silent branch so the underlying failure can be identified from logs without exposing new details to the caller.

Changes:

• Add a logger.error(...) call when isVeryfrontError(error) and the derived response.status is >= 500.
• Include structured fields (project identifiers, HTTP status, error slug/category/detail, and cause) to support debugging preview/staging failures.

Verification:

• Not run in this review environment. PR description reports deno check, deno lint, deno fmt --check, and agent-stream.handler.test.ts passing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kwakayama]

Both review findings addressed

Copilot found a real bug in the fix — cause was being dropped

VeryfrontError.cause is typed as unknown and is frequently a string … string causes become undefined and you lose the provenance this PR is trying to surface.

Correct, and it defeated the point of the change. describeErrorCause now handles Error, string, and anything else. Proven in the test, which asserts a string cause survives:

status=503 slug=service-overloaded category=SERVER
detail="Agent service context has not been initialized."
cause="adapter boot failed"

Test added

My earlier attempt landed on the fallback branch, which is why I shipped without one. Injecting through ensureProjectDiscovery reaches the isVeryfrontError branch. The test asserts the detail, the string cause and the slug are logged, and that the response body still omits the detail.

Confirmed it actually tests the change: flipping the condition to >= 600 makes it fail with "handler did not log"; restoring it passes.

Gates

deno check, deno lint, deno fmt --check clean. agent-stream.handler.test.ts — 2 tests, 42 steps, 0 failed. lint:test-typecheck holds at 51 grandfathered, 0 new.

Also trimmed the inline comment down to the one non-obvious fact — why the branch has to log at all.

[coderabbitai]

Actionable comments posted: 1

🤖 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/server/handlers/request/agent-stream.handler.test.ts`:
- Around line 3460-3464: Update the test around the logged error in the relevant
request handler test to store the created error in a local variable, then assert
that its category matches the structured category field on logged. Keep the
existing serialized assertions for the cause and slug, while adding focused
coverage for the logger’s error.category behavior.

🪄 Autofix

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c7c47d73-3a40-44a1-8633-7fce2b84b964

📥 Commits

Reviewing files that changed from the base of the PR and between 9d76e66 and 8a2bd85.

📒 Files selected for processing (2)

• src/server/handlers/request/agent-stream.handler.test.ts
• src/server/handlers/request/agent-stream.handler.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/server/handlers/request/agent-stream.handler.ts

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/server/handlers/request/agent-stream.handler.ts:118

• describeErrorCause can throw when String(cause) hits a hostile object/proxy (or a custom toString that throws). This is in the error-handling path, so a secondary throw here can mask the original VeryfrontError and potentially change the response/logging behavior. Wrap the stringification in a try/catch (or use the existing #veryfront/errors getErrorMessage helper) so logging never throws.

function describeErrorCause(cause: unknown): string | undefined {
  if (cause === undefined || cause === null) return undefined;
  if (cause instanceof Error) return cause.message;
  return typeof cause === "string" ? cause : String(cause);
}

src/server/handlers/request/agent-stream.handler.test.ts:3464

• This test claims it verifies slug, category, detail, and cause, but it only asserts slug/detail/cause via JSON.stringify. Prefer asserting the structured fields directly (including category) so the test fails if the log context shape regresses.

      const serialized = JSON.stringify(logged);
      assertStringIncludes(serialized, detail);
      // cause is typed `unknown` and is frequently a string, not an Error.
      assertStringIncludes(serialized, "adapter boot failed");
      assertStringIncludes(serialized, "service-overloaded");

[kwakayama]

Addressed: the test now holds the thrown error in a local and asserts both slug and category against it, rather than matching a hardcoded slug string. deno check clean; handler suite 2 tests / 42 steps green.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/server/handlers/request/agent-stream.handler.ts:118

• describeErrorCause can throw if cause is an object with a throwing toString() (or a Proxy that throws on coercion). Since this runs inside the catch handler, a throw here could mask the original error and potentially prevent sending the intended error response. Wrap the stringification in a try/catch and include the error name when cause is an Error for more useful diagnostics.

function describeErrorCause(cause: unknown): string | undefined {
  if (cause === undefined || cause === null) return undefined;
  if (cause instanceof Error) return cause.message;
  return typeof cause === "string" ? cause : String(cause);
}