Skip to content

[devintern-public-devintern-h] Fix Sentry error - #366

Open
danii1 wants to merge 2 commits into
mainfrom
feature/devintern-public-devintern-h-attempt-2
Open

danii1 wants to merge 2 commits into
mainfrom
feature/devintern-public-devintern-h-attempt-2

Conversation

@danii1

@danii1 danii1 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Why

Task devintern-public-devintern-h: Fix Sentry error

The Sentry group records Agent exited with code 1 from runAgentHarness with no explanation. The known infrastructure cause — a nested nono sandbox failing before the agent starts — was already guarded in the sandbox resolver, but the failure path still had two gaps: some nono session/capability startup errors were not recognized, and any remaining non-zero exit discarded stdout/stderr entirely, leaving every distinct cause collapsed into the same unattributable Sentry event.

Reproducing a nested nono run in this environment produced nono: Snapshot error: Failed to create session directory ... Permission denied (os error 13). The classifier now handles that failure shape regardless of the OS error wording, and ordinary agent failures carry their actual reason instead of only an exit code.

What changed

  • packages/code/src/lib/agent/harness-startup.ts
    • Added startup patterns for nested nono snapshot/session/capability errors and landlock ruleset/namespace creation failures.
    • Added describeAgentExitFailure, which selects the last failure-shaped stderr line (falling back to the last stderr/stdout line), strips ANSI codes, and caps the reason at 300 characters.
  • packages/code/src/lib/agent/run-harness.ts
    • The ordinary non-zero-exit branch now logs the reason and throws Agent exited with code N: <reason> instead of the bare exit code.
  • packages/code/tests/harness-startup.test.ts
    • Covers the nested nono snapshot failure with a duration beyond the fast-exit heuristic, plus reason selection, fallback, ANSI stripping, truncation, and empty output.
  • packages/code/tests/run-harness.test.ts
    • Asserts a non-zero agent exit includes the last failure line in the rejected error.

Scope

Included: startup-failure classification and non-zero exit diagnostics for implementation agent runs. Excluded: sandbox nesting policy, usage-limit detection, max-turns handling, delivery/PR behavior, and review-agent failure reporting (which is a separate path).

Tradeoffs

Appending the failure reason changes the Sentry event title/grouping for ordinary agent exits, which is intentional because it separates distinct causes. Only the most relevant single line is included rather than the full transcript, keeping events bounded and avoiding a large unredacted payload; Sentry’s existing redaction still applies. The added nono patterns are scoped to wrapper-prefixed startup errors and do not reclassify ordinary tool failures after the agent has run.

Blast Radius

runAgentHarness is used by every implementation pipeline run, so the improved error reaches ticket failure comments, run records, CLI output, and Sentry capture. detectHarnessStartupFailure consumers now recognize more nested-sandbox startup failures as HarnessUnavailableError, allowing worker failover/defer instead of spending a task attempt. Review/CI-fix runs, sandbox provider selection, and successful runs are unchanged.

Verification

  • Nested nono run reproduction in this environment returned nono: Snapshot error: Failed to create session directory ... Permission denied (os error 13).
  • bun run test harness-startup.test.ts run-harness.test.ts in packages/code — 65 passed, 0 failed.
  • bun run test in packages/code — 2531 passed, 0 failed across 168 files.
  • bun run typecheck in packages/code — clean.
  • bunx oxfmt --check and bunx oxlint on all four changed files — clean.
  • git diff --check — clean.
  • Not verified: a live Sentry event or remediation run after deployment, and the exact nested-sandbox wording from older nono releases.

This PR was automatically created by @devintern/code

- Tighten Landlock startup detection so compiler diagnostics mentioning
  landlock_ruleset stay ordinary task failures; exclude path:line compiler
  lines from startup pattern matching.
- Restrict nono startup detection to session/snapshot creation phrases and
  gate it on a fast exit or empty stdout so mid-run nono errors are not
  misclassified as harness unavailability.
- Redact credentials in agent exit reasons and startup-failure reasons before
  they reach ticket comments, Sentry, or runs.outcome_reason.
- Add regression tests for the compiler line, mid-run nono failure, genuine
  landlock failure, and token redaction.

Iteration 2 fixes:
- Redact agent-derived reasons with redactText + redactSecrets (worker-logs)
  plus GitLab/npm/Google token shapes, so tracker comments and
  runs.outcome_reason never carry AWS, Slack, JSON password, or PEM secrets.
- Make the compiler-diagnostic matcher space-tolerant so paths such as
  "/Users/John Smith/..." do not turn a failed compile into a startup failure.
- Remove the broad landlock pattern and gate landlock detection on a fast exit
  or empty stdout, so mid-run test/build output mentioning landlock stays an
  ordinary task failure.
- Report the line that matched a startup pattern instead of the first stdout
  line, so the failure reason and Sentry event name the real cause.
- Add regression tests for spaced paths, mid-run landlock output, reason-line
  selection, and AWS/Slack/JSON credential redaction.

Iteration 3 fixes:
- Anchor extra token redaction patterns with lookarounds instead of \b so
  GitLab/Google tokens ending in "-" or "_" are still masked before reaching
  ticket comments and runs.outcome_reason.
- Base the landlock/nono startup gate on combined stdout+stderr output and a
  30s exit window, so stderr-only mid-run diagnostics are not misclassified
  while a banner-prefixed startup failure after 2s still is.
- Narrow the "pat" secret-key alternative so path/patch/compatible are no
  longer redacted, keeping nono path_not_granted reasons intact.
- Rewrite compiler-diagnostic tests to exercise the diagnostic filter with an
  open startup gate, and add regressions for trailing "-"/"_" tokens,
  stderr-only mid-run landlock output, banner-prefixed slow startup failures,
  and ordinary "pat" words.

Iteration 4 fixes:
- Remove the lookbehind from the "pat" secret-key alternative so camelCase and
  all-caps keys (clientPat, userPat, MYPAT) are masked again, while path/patch/
  compatible stay untouched because the lookahead already rejects those.
- Drop the unreachable !hasAgentOutput gate clause and document that
  landlock/nono signals are gated on the 30s exit window; a stderr-only mention
  past the window is an ordinary task failure by design.
- Add regression tests for camelCase/uppercase "pat" keys and the inclusive
  30s startup-window boundary.

Iteration 5 fixes:
- Gate mise/EACCES/ENOENT/"Permission denied (os error 13)" startup patterns on
  the same 30s window as landlock/nono, so a long agent run that merely prints
  an npm/node OS error is no longer reclassified as "harness unavailable"
  (which suppressed the failure comment, retry state, and the real failure).
- Apply the compiler-diagnostic filter to the <=2s no-output heuristic and to
  the reason it reports, so tsc/eslint-style output cannot become a startup
  failure with its diagnostic line as the reason.
- Reject whole lines of bare base64/PEM key material and mask inline
  MII-prefixed DER bodies before they reach runs.outcome_reason or a tracker
  comment; prose lines containing spaces stay selectable.
- Add regressions for 20-minute runs printing ENOENT/EACCES/os-error-13/mise
  denials, a 1.2s compiler-diagnostic failure, and lone/inline PEM key bodies.
@danii1 danii1 added the devintern-authored Pull request written by DevIntern working its own backlog label Nov 3, 2026
@danii1

danii1 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Artifacts & verification

Run: 26 · Commit: ab95cc3f021d914997ccaa06deeb30d72357d2de

Change explanation

Produced by pr-explainer-narrative, execution 1 at ab95cc3f021d914997ccaa06deeb30d72357d2de.

No PR template exists under .github/ in this repository, so the structure below follows the requested sections rather than a template.

Explain: Sentry DEVINTERN-H, "Agent exited with code 1"

Why this change is needed

The task provides only the Sentry grouping: an Error: Agent exited with code 1 raised from lib/agent/run-harness.ts, seen 11 times. The task text does not state why the error occurs; it asks for a reproduction, a minimal fix, and tests for the failure path. Any cause statement below therefore comes from the pipeline evidence, not from the task description.

Recorded reproduction (verification step): running nono run -- true inside the current sandbox produced nono: Snapshot error: Failed to create session directory ... Permission denied (os error 13), i.e. a nested sandbox failed before the agent started, while the emitted error was still the generic "exited with code 1". The same step notes nested sandboxing is already prevented by activeSandboxName in the harness resolver, so this classifier is defense-in-depth rather than the only guard.

What changed

1. Make non-zero exits attributable

A new describeAgentExitFailure helper picks the last failure-shaped stderr line (falling back to last stderr, then last stdout), strips ANSI codes, redacts, and truncates to 300 characters. run-harness.ts now appends that detail to the thrown error and logs a Reason: line. Sentry, runs.outcome_reason, and the tracker failure comment therefore receive a cause instead of only an exit code.

2. Make startup classification narrower and more exact

detectHarnessStartupFailure now separates three groups:

  • Always-valid sandbox/harness signals: path_not_granted, sandbox initialization/setup failures, seatbelt errors.
  • Generic OS wordings (mise, EACCES, ENOENT, Permission denied (os error 13)) plus landlock and nested-nono session patterns, which only count when the child exits within 30 seconds.
  • The existing fast-exit heuristic, now suppressed when all output consists of compiler/linter diagnostics.

Compiler diagnostic lines (path:[redacted] error|warning|...) are filtered before pattern matching, so agent build output that mentions landlock_ruleset is no longer read as a sandbox failure. When a startup failure is confirmed, the reason is taken from the line that matched the startup pattern rather than the first stdout line.

3. Redact agent-derived text before it leaves the process

redactAgentOutput composes the existing Sentry-oriented redactText with redactSecrets, then adds masks for GitLab (glpat-), npm, Google API, and bare base64 MII... key material, using lookarounds so tokens ending in - or _ are still caught. Both the startup reason and the new exit reason pass through it. Lines consisting only of bare key material are never selected as a reason.

4. Reduce a shared redaction false positive

SECRET_KEY_PATTERN in worker-logs.ts now requires pat to be followed by a non-alphanumeric character. path, patch, and compatible are no longer treated as secret keys, while pat, GITHUB_PAT, clientPat, and MYPAT still match.

5. Tests

New and extended tests cover: nested-nono startup classification, compiler diagnostics naming landlock (including paths with spaces), mid-run landlock/nono output, the 30-second gate boundaries, selection of the matched startup line, redaction of credentials in reasons, ANSI stripping and truncation, bare-key-material rejection, the enriched exit error, and the pat regex cases.

Validation actually performed

Observed, from the pipeline evidence:

  • Full @getdevintern/code suite: 2531 tests across 168 files, 0 failures.
  • Targeted harness tests passed; the auto-review separately reports 47 tests in harness-startup/worker-logs and 54 in run-harness.
  • Typecheck and oxlint passed on the changed files.
  • Root cause reproduced with a nested nono run -- true invocation as quoted above.

Not observed:

  • No end-to-end replay of a real failing agent run, no live Sentry event check, and no live tracker-comment check.
  • Auto-review did not converge: it reports success: false after 5 iterations, with medium-severity findings still recorded.

Material risks

  • Agent output now reaches the tracker. Redaction is regex-based and best-effort. Credentials outside the covered shapes, or unusual encodings, can still reach a posted failure comment. This was the medium finding in verification; the final diff does route the new reason through redactAgentOutput, but that is a reading of the diff, not a re-run of the review.
  • The 30-second gate is a tradeoff. A genuine sandbox startup failure that takes longer than 30 seconds, or prints only generic OS wording, becomes an ordinary task failure: no deferral, retry state recorded, failure comment posted, and the real cause may be misreported. Conversely, genuine agent work printing landlock/nono text within 30 seconds would still be misclassified.
  • Compiler-diagnostic suppression can over-filter. A real harness error formatted like file.ext:12:5: error: ... is treated as agent output and loses startup classification.
  • Shared regex change has wider reach. redactSecrets is used by dashboard logs, artifact storage, harness logs, and bug reports, so the pat change affects those sinks too. The recorded concern that camelCase keys such as clientPat leak appears addressed by the lookahead form in the final diff, and tests assert it; that is again diff inspection, not a fresh review result.
  • Classification changes control flow. Startup-classified exits take the harness-deferral path (HARNESS_UNAVAILABLE_EXIT_CODE) instead of the task-failure path, so any classification change alters retry/deferral behavior.
  • Auto-review is still open. Earlier feedback also flagged the ungated EACCES/ENOENT/mise patterns; the final diff shows these moved into the windowed list, which appears to resolve that specific item, but reviewers should confirm by re-running the review rather than treating it as verified.

Where reviewers should focus

  1. packages/code/src/lib/agent/harness-startup.ts — the windowed versus always-valid pattern split, the compiler-diagnostic filter, and the redaction composition. This is where misclassification and leakage risk both live.
  2. packages/code/src/lib/agent/run-harness.ts — the new thrown error text; confirm it is the only new consumer path and that it runs after timeout, usage-limit, startup, and max-turns handling.
  3. packages/code/src/lib/observability/worker-logs.ts — the SECRET_KEY_PATTERN change, because it affects redaction outside the agent path.
  4. packages/code/tests/harness-startup.test.ts — verify the tests assert behavior rather than merely matching the current regexes, especially the 30-second boundary and bare-key-material cases.

Suggested checks not yet recorded: replay a long agent run that prints code: 'ENOENT' or npm ERR! EACCES and confirm it is reported as a task failure; replay a genuine slow nested-nono startup failure and confirm it is still deferred; inspect the rendered tracker comment for an error containing a realistic token.

Visual explanation

Produced by pr-explainer-visual, execution 1 at ab95cc3f021d914997ccaa06deeb30d72357d2de.

Show artifact

Summary

The task is to reproduce and fix the root cause behind Sentry group DEVINTERN-H, Error: Agent exited with code 1 in run-harness.ts, which recorded 11 events, and to cover that failure path with tests. This change classifies nested sandbox startup failures that were previously missed and attaches the agent's last failure line to ordinary non-zero exits, so distinct causes no longer collapse into the same bare exit code. It also redacts agent-derived text before the reason is logged, recorded, or posted.

Evidence

child exits non-zero
├─ detectHarnessStartupFailure()
│  ├─ always:  path_not_granted · sandbox init/setup · seatbelt
│  ├─ <=30s:   mise · EACCES · ENOENT · "Permission denied (os error 13)"
│  │            landlock · nested-nono session/snapshot errors
│  ├─ <=2s and empty stdout: fast-exit heuristic
│  └─ compiler-diagnostic lines filtered out before all checks
└─ otherwise: describeAgentExitFailure()
   ├─ last failure-shaped stderr line -> last stderr -> last stdout
   ├─ redactAgentOutput(redactText -> redactSecrets -> extra token masks)
   └─ throw "Agent exited with code N: <reason>" (300-char cap)

Recorded in the pipeline evidence (not re-run here):

  • Root cause reproduction: nono run -- true inside the sandbox produced nono: Snapshot error: Failed to create session directory ... Permission denied (os error 13) — a nested sandbox failing before the agent starts.
  • Verification step: full @getdevintern/code suite, 2531 tests across 168 files, 0 failures; targeted harness tests passed; typecheck and oxlint clean on changed files.
  • Auto-review separately reports 47 tests in harness-startup/worker-logs and 54 in run-harness, with typecheck and oxlint clean.

Before/after execution results were not captured in the evidence. There is no end-to-end replay of a real failing agent run, no live Sentry event check, and no rendered tracker-comment check. Auto-review did not converge: success: false after 5 iterations, with medium-severity findings still recorded.

Merge Danger

Revert is mechanically simple: no schema or migration changes, and the enriched error text is only consumed as opaque text at four sinks — console, runs.outcome_reason, the tracker failure comment, and Sentry. Reverting restores the bare Agent exited with code N and the older pattern set; nothing parses the string, so no downstream format breaks.

Blast radius is control flow, not just messaging. detectHarnessStartupFailure decides between the harness-deferral path (HARNESS_UNAVAILABLE_EXIT_CODE, worker failover) and the task-failure path (retry state plus failure comment), so it affects every implementation run. Two concrete edges: a genuine startup failure taking longer than 30 seconds now reports as an ordinary task failure, and a real harness error formatted like file.ext:12:5: error: ... is filtered as compiler output and loses startup classification. The SECRET_KEY_PATTERN edit in worker-logs.ts also reaches dashboard logs, artifact storage, harness logs, and bug reports, not only agent output.

Safe to ship without a staged rollout because it is confined to failure handling and nested sandboxing is already blocked earlier by activeSandboxName, making this classifier defense-in-depth. The caveat is redaction: agent stderr that was never previously surfaced now reaches the tracker comment and outcome_reason, and the masking is regex best-effort, which was the verification step's remaining medium finding.

Verification report

Produced by __verify-1, execution 1 at 291a8a4aacfbd0064362787aa29bf8a18b9ac1fe.

Show artifact
{
  "summary": "Implementation satisfies the task: the nested nono root cause was reproduced (running `nono run -- true` inside the current sandbox emits `nono: Snapshot error: Failed to create session directory ... Permission denied (os error 13)`), and the change adds OS-wording-independent startup classification plus a non-zero-exit reason that is carried into logs, Sentry, and failure feedback. Verification: full `@getdevintern/code` suite ran 2531 tests across 168 files with 0 failures; targeted harness tests passed; typecheck and oxlint on changed files passed. No critical or high-severity defects found. The main residual concern is that the newly surfaced stderr fragment is not redacted before being posted to the tracker. Blast radius is contained because the new helper is called only in the generic non-zero-exit branch after timeout, usage-limit, startup, and max-turns handling, so it cannot change control flow; the added startup patterns are additive regex checks evaluated only on non-zero exit. The change is safe because nested sandboxing is already prevented by `activeSandboxName` in the harness resolver, making this classifier a defense-in-depth layer rather than the sole guard.",
  "items": [
    {
      "priority": "medium",
      "confidence": "medium",
      "category": "security",
      "file": "packages/code/src/lib/agent/run-harness.ts",
      "line": "523",
      "issue": "The new code appends a raw agent stderr fragment to the thrown Error message. That message is later posted to the task tracker, so stderr that was previously never posted can now leak tokens, credentials, or sensitive paths into a Jira/Linear comment. Sentry redacts exception values, but the tracker failure-comment path does not.",
      "evidence": "run-harness.ts:516-523 embeds `failureDetail` in the Error; packages/code/src/lib/task/pipeline.ts:444 passes `err.message` to reportProcessingFailure; packages/code/src/lib/task/failure-feedback.ts:59-61 posts it via formatProcessingFailureMarkdown. `redactText` exists at packages/utils/src/redact.ts:37 but is wired only into packages/utils/src/sentry.ts:60, not this tracker path.",
      "suggestion": "Run `redactText` from `@devintern/utils` over `failureDetail` before embedding it, or redact centrally in the failure-comment formatter."
    },
    {
      "priority": "low",
      "confidence": "high",
      "category": "testing",
      "file": "packages/code/src/lib/agent/harness-startup.ts",
      "line": "52",
      "issue": "The new landlock startup pattern is untested, and its failure wording is not tied to the DEVINTERN-H evidence. It is an unexercised behavior change in a task that asked for a minimal fix.",
      "evidence": "grep for `landlock` under packages/code/tests returns no test matches; the only occurrence is the source line. The full suite still passes, so there is no regression, but the pattern itself is unverified.",
      "suggestion": "Add a `detectHarnessStartupFailure` case for a message such as `failed to apply landlock ruleset`, or drop the pattern to keep the change scoped to the reproduced nono failure."
    },
    {
      "priority": "low",
      "confidence": "medium",
      "category": "code-quality",
      "file": "packages/code/src/lib/agent/harness-startup.ts",
      "line": "57",
      "issue": "The nono regex requires `snapshot|session|capabilit` to appear before `error|fail|denied`. A plausible variant such as `nono: error: failed to create session ...` would not match, even though the currently reproduced nono v0.69 wording does match.",
      "evidence": "Ran `nono run -- true` inside this session and got `nono: Snapshot error: Failed to create session directory /home/daniil/.local/state/nono/audit/...: Permission denied (os error 13)`, which matches the regex. The ordering dependency remains brittle to future wording changes.",
      "suggestion": "Match either ordering, for example with two alternatives or by requiring `nono:` plus a state word and a failure word without fixing their order."
    },
    {
      "priority": "low",
      "confidence": "high",
      "category": "testing",
      "file": "packages/code/tests/run-harness.test.ts",
      "line": "380",
      "issue": "There is no integration test proving `runAgentHarness` converts the reproduced nested-nono stderr into `HarnessUnavailableError`; coverage is split between a unit regex test and the existing mise sandbox integration test.",
      "evidence": "harness-startup.test.ts:31-44 exercises `detectHarnessStartupFailure` only; run-harness.test.ts:380 covers a mise sandbox failure, and no test in run-harness.test.ts uses the nono stderr string.",
      "suggestion": "Add a `runAgentHarness` scenario with the reproduced nono stderr and assert it rejects with `HarnessUnavailableError`."
    },
    {
      "priority": "low",
      "confidence": "high",
      "category": "code-quality",
      "file": "packages/code/src/lib/webhook/agent-review.ts",
      "line": "203",
      "issue": "A sibling consumer of the same error shape still emits the bare `Agent exited with code N` message and does not use the new helper, so review-agent failures remain unattributable in Sentry and feedback.",
      "evidence": "grep for `Agent exited with code` finds agent-review.ts:203, while `describeAgentExitFailure` is imported only by run-harness.ts:46.",
      "suggestion": "If review-agent runs share this Sentry issue class, reuse `describeAgentExitFailure` there; otherwise document that scope intentionally covers only the implementation harness."
    },
    {
      "priority": "low",
      "confidence": "high",
      "category": "code-quality",
      "file": "packages/code/src/lib/agent/harness-startup.ts",
      "line": "122",
      "issue": "`maxChars` is documented as the maximum reason length, but truncation returns `maxChars + 1` characters because the ellipsis is appended after slicing to `maxChars`. The test enshrines this off-by-one behavior.",
      "evidence": "harness-startup.ts:122 slices to `maxChars` then appends `…`; harness-startup.test.ts:108 asserts `reason.length` is 21 for `maxChars` 20.",
      "suggestion": "Slice to `maxChars - 1` before appending the ellipsis, or reword the documentation to say the limit is approximate."
    },
    {
      "priority": "info",
      "confidence": "high",
      "category": "documentation",
      "file": "packages/agent-harness/src/sandbox/resolver.ts",
      "line": "143",
      "issue": "This change is diagnostic/classification rather than prevention: the nested-nono cause was already prevented by `activeSandboxName` and `skipNesting`, which is why the new classifier is a safe defense-in-depth layer.",
      "evidence": "`git merge-base --is-ancestor 8679b74 HEAD` exited 0; resolver.ts:143 defines `activeSandboxName`, and resolver.ts:188/253 skip nesting; the nested nono failure was reproduced with `nono run -- true`.",
      "suggestion": "No action needed; retain both layers so a future wrapper that hides the marker still produces an attributable failure."
    }
  ],
  "approved": true
}

Further artifacts omitted because they exceed the comment preview budget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devintern-authored Pull request written by DevIntern working its own backlog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant