Skip to content

fix(db): throw on corrupt commands JSON instead of silent empty fallback#1033

Merged
Wirasm merged 2 commits intocoleam00:devfrom
kagura-agent:fix/codebase-commands-corrupt-json
Apr 20, 2026
Merged

fix(db): throw on corrupt commands JSON instead of silent empty fallback#1033
Wirasm merged 2 commits intocoleam00:devfrom
kagura-agent:fix/codebase-commands-corrupt-json

Conversation

@kagura-agent
Copy link
Copy Markdown
Contributor

@kagura-agent kagura-agent commented Apr 10, 2026

Fixes #967

Problem

getCodebaseCommands() silently returned {} when the codebases.commands column contained corrupt JSON. The error was logged but never surfaced to the caller, so a client receiving {} had no way to distinguish "no commands configured" from "commands exist but are unreadable."

Fix

  • Throw a descriptive Error instead of silently returning {}
  • Error message includes the codebase ID and a SQL recovery hint for operators
  • The corruption is still logged via the structured logger before throwing

Changes

  • packages/core/src/db/codebases.ts — replace return {} with throw new Error(...) in the JSON parse catch block
  • packages/core/src/db/codebases.test.ts — add two test cases:
    • Corrupt JSON string throws with descriptive message
    • Valid JSON string from SQLite TEXT column parses correctly

Validation

  • bun test — 31 pass, 0 fail
  • bun run type-check — all packages clean
  • bun run lint — no errors

Summary by CodeRabbit

  • Bug Fixes
    • Corrupted serialized command data now surfaces a clear, descriptive error naming the affected codebase and advising how to reset the stored value, replacing prior silent failures.
  • Tests
    • Added tests to verify behavior for both corrupted and valid serialized command data, ensuring parsing errors are detected and valid payloads are returned.

…ack (coleam00#967)

getCodebaseCommands() silently returned {} when the commands column
contained corrupt JSON. Callers had no way to distinguish 'no commands'
from 'unreadable data', violating fail-fast principles.

Now throws a descriptive error with the codebase ID and a recovery hint.
The error is still logged for observability before throwing.

Adds two test cases: corrupt JSON throws, valid JSON string parses.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d92e1e80-1d23-40be-8938-a4af19d70de5

📥 Commits

Reviewing files that changed from the base of the PR and between beaacdc and f0892b2.

📒 Files selected for processing (2)
  • packages/core/src/db/codebases.test.ts
  • packages/core/src/db/codebases.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/core/src/db/codebases.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/src/db/codebases.ts

📝 Walkthrough

Walkthrough

getCodebaseCommands now surfaces corrupt JSON in the commands field by throwing a descriptive error (including an UPDATE reset suggestion) instead of silently returning an empty object; tests were added for corrupt and valid JSON string cases.

Changes

Cohort / File(s) Summary
DB logic
packages/core/src/db/codebases.ts
On string commands parse failure, log the caught error and throw a new Error indicating corrupt stored JSON with an UPDATE reset hint (replaces prior behavior that returned {}).
Tests
packages/core/src/db/codebases.test.ts
Added tests: one stubbing a corrupt JSON string and asserting rejection with the corrupt-JSON error message, another stubbing a valid JSON string and asserting correct parsing and return.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 In a patch of code where strings once slept,
A scrambled command gave the burrow a fret.
Now I sniff, I shout — errors hop into view,
"Reset the JSON" I whisper, and clarity grew. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the core change: replacing silent empty fallback with error throwing for corrupt JSON commands.
Description check ✅ Passed The description covers the problem, fix, changes made, and validation results. Most required template sections are addressed, though some like UX Journey, Architecture Diagram, and Rollback Plan are omitted.
Linked Issues check ✅ Passed The PR fully addresses issue #967 requirements: corrupt JSON now throws a descriptive error instead of silently returning {}, allowing clients to distinguish unreadable commands from absent ones.
Out of Scope Changes check ✅ Passed All changes are scoped to the codebase commands JSON parsing: one test file with new test cases and one implementation file with error handling logic, both directly addressing issue #967.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/db/codebases.ts`:
- Around line 64-69: The catch block that currently swallows the parse error
should log the original error object before throwing the user-facing Error:
update the catch to include the caught error (_err) in the logging call (e.g.,
getLog().error({ codebaseId: id, raw, err: _err },
'db.codebase_commands_json_parse_failed')) so the raw parse exception is
preserved in logs; keep the existing thrown Error message for users but do not
remove or discard _err.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 668a785b-77d0-4e1c-8f37-516a6b5b9f12

📥 Commits

Reviewing files that changed from the base of the PR and between 95679fa and 9b65350.

📒 Files selected for processing (2)
  • packages/core/src/db/codebases.test.ts
  • packages/core/src/db/codebases.ts

Comment thread packages/core/src/db/codebases.ts Outdated
@kagura-agent
Copy link
Copy Markdown
Contributor Author

Good catch @coderabbitai — included the original parse error in the log entry (beaacdc). Now err is logged alongside codebaseId and raw for full root-cause diagnostics.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 10, 2026

Tip

For best results, initiate chat on the files or code changes.

@kagura-agent: That's exactly what was needed — having err, codebaseId, and raw all in the same structured log entry gives operators everything they need to diagnose corruption without having to cross-reference multiple log lines. The fix aligns well with the principle of logging the raw error alongside a clear user-facing message. Thanks for the quick update!


🧠 Learnings used
Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T16:37:41.390Z
Learning: Applies to packages/*/src/**/*.ts : When database operations fail, always log the raw error for debugging and raise or re-throw with a clear user-friendly message; for UPDATE operations, verify rowCount to catch missing records

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@kagura-agent
Copy link
Copy Markdown
Contributor Author

Gentle ping — any thoughts on this one? Happy to address feedback if needed. Will close in ~7 days if no response to keep things tidy. 🙂

@Wirasm Wirasm force-pushed the fix/codebase-commands-corrupt-json branch from beaacdc to f0892b2 Compare April 20, 2026 11:18
@Wirasm Wirasm merged commit 39a05b7 into coleam00:dev Apr 20, 2026
4 checks passed
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.

Corrupt JSON in codebases.commands silently returns empty object

2 participants