Skip to content

fix(#12274): fallback-slop sweep — app plugins B (wallet/vision/music/agent-skills/browser/workflow/training) - #12825

Merged
lalalune merged 1 commit into
developfrom
fallback/12274-sweep
Jul 4, 2026
Merged

lalalune merged 1 commit into
developfrom
fallback/12274-sweep

Conversation

@lalalune

@lalalune lalalune commented Jul 4, 2026

Copy link
Copy Markdown
Member

Fallback-slop sweep (#12182): app plugins B — wallet / vision / music / agent-skills / browser / workflow / training

Executes the #12182 rubric against the app-plugins-B slice. This is behavior-changing
work, so it is deliberately a small, precise PR: high-confidence slop converted to
fast-fail, plausibly-justified handlers annotated, everything ambiguous left untouched
and counted. Precision over completeness — a reckless large sweep here would fake wallet
success or drop a designed browser/workflow degrade.

Foundation (#12263) is used as-is: ElizaError code/context/cause, and the diff-scoped
bun run audit:error-policy-ratchet.

Re-derived suspect counts (this slice @ base 36be3c8655)

pattern count
empty catch (non-test, TS) 4 (2 real; 2 inside a PowerShell string — exempt per issue)
promise swallow .catch(()=>…) 33
return-default catch 133
?? <lit> 699
|| <lit> 316
try blocks 968

Site table (verdict per touched site)

site pattern verdict action
plugin-wallet/src/wallet/local-eoa-backend.ts:83-87 catch → return null SLOP convert → let typed SolanaPrivateKeyInvalidError propagate
plugin-browser/src/workspace/browser-capture.ts:244 empty catch (teardown) J6 keep + logger.debug + annotate
plugin-browser/src/workspace/browser-capture.ts:250 empty catch (teardown) J6 keep + logger.debug + annotate
plugin-browser/src/bridge-policy.ts:41 new URL() catch → null J3 keep + annotate (typed-invalid, callers fail-closed)
plugin-browser/src/routes/workspace.ts:176 decodeURIComponent catch → null J3 keep + annotate (typed-invalid tab id → 404)

Per-category tally

  • Converted (slop → fast-fail): 1local-eoa-backend.ts (money path).
  • Annotated (justified J-category): 4 — 2× J6 (browser teardown), 2× J3 (browser URL/tab-id parse).
  • Left (ambiguous / designed degrade / legitimate absence): ~large — e.g. the kamino
    APY service (Math.random()-fabricated data — a larp rewrite, out of a fallback sweep's
    scope), workflow-service generation/deploy degrades (getRuntimeContext/fixWorkflowErrors/
    activateWorkflow log-and-continue — nuanced product-degrade judgment, not money paths),
    agent-skills load paths (already logger.warn per failure — not silent), and the wallet
    ?? "0" transaction/LP defaults (ERC20 value=0, optional second LP token — legitimate
    defaults, not catch-swallow). Precision beats completeness; these are candidates for a
    follow-up with the product owner, not high-confidence one-shot conversions.

Exemplar before → after (the money-path conversion)

resolveSolanaKeypair swallowed a carefully-typed error, conflating "invalid configured key"
with "no key configured":

// BEFORE — a malformed SOLANA_PRIVATE_KEY reads as WalletBackendNotConfiguredError("NO_WALLET_CONFIGURED")
if (!raw) return null;
try {
  return keypairFromSolanaSecret(raw);   // throws typed SolanaPrivateKeyInvalidError
} catch {
  return null;                            // swallowed
}

// AFTER — absence stays null; a configured-but-malformed key surfaces the typed error
if (!raw) return null;                    // legitimate absence
return keypairFromSolanaSecret(raw);      // invalid key propagates

Test (real error path, no mocked dependency)

plugin-wallet/src/wallet/local-eoa-backend.test.ts drives the real key-derivation path
(real @solana/web3.js Keypair, real base58):

  • valid base58 secret → usable signer (getAddresses().solana matches, canSign('solana'));
  • wrong-length configured key → rejects.toBeInstanceOf(SolanaPrivateKeyInvalidError);
  • non-base58 configured key → SolanaPrivateKeyInvalidError;
  • genuinely no key set → still WalletBackendNotConfiguredError (absence path preserved).

Verification

  • bun run --cwd plugins/plugin-wallet test126 passed, 1 skipped (incl. the 4 new tests).
  • bun run --cwd plugins/plugin-browser test147 passed.
  • bun run --cwd plugins/plugin-wallet check → clean; bun run --cwd plugins/plugin-browser typecheck → clean.
  • bun run audit:error-policy-ratchetpass; browser-capture emptyCatch 2→0, no new slop in any touched file.

Refs #12274 · parent #12182 · foundation #12263.

🤖 Generated with Claude Code

@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: 9852dc33-0b93-4304-966e-9e73be07c07d

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 fallback/12274-sweep

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.

@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.

@lalalune

lalalune commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

Reviewed (#12274 slop sweep, app plugins B): browser-capture stop-path errors now log at debug with the error message (non-fatal but observable); the wallet LocalEoaBackend gets tests proving invalid Solana keys / missing config fail loud with typed SolanaPrivateKeyInvalidError/WalletBackendNotConfiguredError instead of fabricating a default (security-correct fail-closed for a wallet key). Net-additive, well-tested. Queuing auto-merge on green.

…, annotate justified handlers

App plugins B slice (wallet/vision/music/agent-skills/browser/workflow/training).
High-confidence conversions only; large sitesLeft by design (precision over
completeness).

- wallet/local-eoa-backend: resolveSolanaKeypair swallowed the typed
  SolanaPrivateKeyInvalidError into null, so a configured-but-malformed
  SOLANA_PRIVATE_KEY read identically to "no wallet configured"
  (WalletBackendNotConfiguredError). Let the typed invalid-key error propagate;
  a real test asserts the malformed-key path now surfaces and the genuine
  no-key path still reports NO_WALLET_CONFIGURED.
- browser/workspace/browser-capture: the two real empty catches in
  stopBrowserCapture are teardown — annotated error-policy:J6 with logger.debug.
- browser/bridge-policy + routes/workspace: URL/tab-id parse helpers returning
  null on malformed input are explicit typed-invalid signals — annotated
  error-policy:J3.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lalalune
lalalune force-pushed the fallback/12274-sweep branch from eea44a6 to 5f4d4cc Compare July 4, 2026 04:46

@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.

@lalalune
lalalune merged commit d1a9523 into develop Jul 4, 2026
17 of 38 checks passed
@lalalune
lalalune deleted the fallback/12274-sweep branch July 4, 2026 04:46
lalalune added a commit that referenced this pull request Jul 4, 2026
…/fabricated-default/promise-swallow to fail-fast (#13270)

Residual deep sweep over the app-plugins-B slice (wallet/vision/music/
agent-skills/browser/workflow/training) after the primary #12274 sweep
(#12825). Converts the clear-slop that survived, keeps and annotates
genuine J-categories, defers the ambiguous return-null/false + ??-literal
tail for a judgment pass.

Fabricated-healthy (data-loss) — agent-skills skill preference/acknowledgment
loaders returned `{}` on a cache *read failure*, conflating "DB read failed"
with "nothing persisted". Callers read-modify-write these maps and save them
back (`prefs[id]=true; save(prefs)`), so the fabricated empty overwrote every
other skill's saved preference on a transient DB error. Now the read failure
propagates to the route boundary (a genuinely-empty cache still returns `{}`
via `?? {}`). Fixed in both the exported helpers and the duplicated local
copies in skills-routes.ts.

Empty catch — browser-service target-resolution failover scan swallowed an
unhealthy candidate's availability error with an empty `catch {}`. Kept as
designed failover (other targets still resolve, unlike the pinned path which
throws) but annotated `// error-policy:J4` and now logs the exclusion so a
systemically broken target is observable.

Tests — new skill-discovery-helpers.test.ts asserts a cache read failure now
throws (would have resolved `{}` before); browser-service.test.ts gains a
failover-scan case asserting the broken candidate is excluded, a healthy one
still resolves, and the exclusion is logged.

Refs #12274

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lalalune pushed a commit that referenced this pull request Jul 4, 2026
…/fabricated-default/promise-swallow to fail-fast

Residual deep sweep over the app-plugins-B slice (wallet/vision/music/
agent-skills/browser/workflow/training) after the primary #12274 sweep
(#12825). Converts the clear-slop that survived, keeps and annotates
genuine J-categories, defers the ambiguous return-null/false + ??-literal
tail for a judgment pass.

Fabricated-healthy (data-loss) — agent-skills skill preference/acknowledgment
loaders returned `{}` on a cache *read failure*, conflating "DB read failed"
with "nothing persisted". Callers read-modify-write these maps and save them
back (`prefs[id]=true; save(prefs)`), so the fabricated empty overwrote every
other skill's saved preference on a transient DB error. Now the read failure
propagates to the route boundary (a genuinely-empty cache still returns `{}`
via `?? {}`). Fixed in both the exported helpers and the duplicated local
copies in skills-routes.ts.

Empty catch — browser-service target-resolution failover scan swallowed an
unhealthy candidate's availability error with an empty `catch {}`. Kept as
designed failover (other targets still resolve, unlike the pinned path which
throws) but annotated `// error-policy:J4` and now logs the exclusion so a
systemically broken target is observable.

Tests — new skill-discovery-helpers.test.ts asserts a cache read failure now
throws (would have resolved `{}` before); browser-service.test.ts gains a
failover-scan case asserting the broken candidate is excluded, a healthy one
still resolves, and the exclusion is logged.

Refs #12274

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants