Skip to content

fix(#12265): annotate the last agent empty catches per error-policy rubric - #12921

Merged
lalalune merged 1 commit into
developfrom
docs/12265-agent-empty-catch-annotations
Jul 4, 2026
Merged

lalalune merged 1 commit into
developfrom
docs/12265-agent-empty-catch-annotations

Conversation

@lalalune

@lalalune lalalune commented Jul 4, 2026

Copy link
Copy Markdown
Member

Summary

Part of the fallback-slop sweep for packages/agent (#12265, parent #12182). The 3 remaining unannotated empty catches in the agent API layer are justified handlers; this adds the grep-able // error-policy:J<N> annotations the binding rubric requires, with no behavior change (comments added inside the catch blocks only).

  • api/server.ts — the WS-upgrade socket.destroy() cleanup → J6 best-effort teardown (the socket may already be destroyed after a failed upgrade).
  • api/wallet-capability.ts — the two EVM service-registry probes → J3: a service lookup that throws means that provider isn't usable here, so it is treated as absent and the next candidate is tried. The function returns an honest false (capability absent), never a fabricated success.

This clears the last truly-empty catch sites in packages/agent/src (3 → 0), so noEmptyBlockStatements holds for them.

Scope note: this is a bounded, safe slice of #12265 — it does not close the batch, which still covers the broader ?? <literal> / .catch(()=>) judgment calls across the agent runtime (best done as small, tested, evidence-backed slices per DoD).

Validation

  • node packages/scripts/error-policy-ratchet.mjsno new fallback-slop in touched files (the enforced diff-scoped gate).
  • bunx @biomejs/biome check on both touched files → clean, no fixes.
  • Truly-empty catches in packages/agent/src: 3 on develop → 0 on this branch.
  • Annotations are grep-able: git grep 'error-policy:J[36]'.

Evidence rows

  • Real-LLM trajectory / logs / screenshots — N/A: comment-only annotations inside existing catch blocks; zero runtime behavior change (the caught paths and their outcomes are unchanged).

🤖 Generated with Claude Code

Fallback-slop sweep (packages/agent, #12182): the 3 remaining unannotated
empty catches in the agent API layer are justified handlers, now carrying
grep-able `// error-policy:J<N>` annotations per the binding rubric:

- server.ts WS-upgrade `socket.destroy()` → J6 best-effort teardown (the
  socket may already be destroyed after a failed upgrade).
- wallet-capability.ts EVM service-registry probes (×2) → J3: a service
  lookup that throws means that provider isn't usable here, so it is treated
  as absent and the next candidate is tried. The function returns an honest
  `false` (capability absent), never a fabricated success.

No behavior change — comments added inside the catch blocks only. Clears the
last empty-catch sites in packages/agent/src so `noEmptyBlockStatements` holds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f88bf7ee-fb08-472e-92ca-9f5e297bf03c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/12265-agent-empty-catch-annotations

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.

@lalalune
lalalune merged commit 1c63528 into develop Jul 4, 2026
15 of 51 checks passed
@lalalune
lalalune deleted the docs/12265-agent-empty-catch-annotations branch July 4, 2026 05:07
lalalune pushed a commit that referenced this pull request Jul 4, 2026
…/fabricated-default/promise-swallow to fail-fast

Deep re-sweep of the remaining fallback-slop in packages/agent after the
headline files were handled by #12766 / #12921. Empty-catch count in this slice
is already 0; this pass targets the log-and-return-empty catches that make a
broken pipeline read as a legitimately-empty result, plus fail-closed swallows.

Converted (behavior-changing, fail-fast; each with a real error-path test):
- providers/recent-conversations.ts + relevant-conversations.ts: on a recall
  failure both providers returned the IDENTICAL empty context as a legit-empty
  recall (the banned "not loaded reads as empty" conflation). Now
  runtime.reportError surfaces the broken pipeline via RECENT_ERRORS while the
  provider still degrades to empty (annotated error-policy:J4). Removed the
  now-unused logger imports (logger-only rule still holds; reportError logs).
- security/access.ts hasPrivateAccess: a throw from the core private-access
  check was silently swallowed to `return false` — fail-closed is correct, but
  a broken role/world-resolution pipeline would deny forever with no signal.
  Now reports via runtime.reportError and stays fail-closed (error-policy:J4).

Annotated (evaluated, kept — not slop):
- api/views-registry.ts ×2 indexView `.catch(() => {})`: indexView self-degrades
  (its own catch logs and falls back to keyword search); the call-site catch only
  suppresses a stray pre-embed rejection so a background task cannot crash the
  loop (error-policy:J5).

Deferred (ambiguous absence-vs-failure, per this wave's scope): bare
`return null/[]/false` catches and `?? <lit>` defaults where legitimate-absence
cannot be cleanly distinguished from masking-a-failure.

Tests: 6 new fast-fail assertions across 3 files (recent-conversations,
relevant-conversations.faildast, access) — all green; each asserts reportError
fires on the real failure AND that a legit-absence path does NOT report.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

❌ PR title does not match the required pattern. Please use one of these formats:

  • 'type: description' (e.g., 'feat: add new feature')
  • 'type(scope): description' (e.g., 'chore(core): update dependencies')
    Valid types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert, release

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.

2 participants