Persist subagent registry in SQLite#88260
Conversation
openclaw-barnacle
Bot
added
agents
|
Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 3:06 AM ET / 07:06 UTC. Summary PR surface: Source +318, Tests +134. Total +452 across 3 files. Reproducibility: not applicable. this is a backend persistence replacement PR rather than a standalone bug report. Current-main source verifies the existing JSON-backed behavior, and the PR proof is focused fixture coverage of the new SQLite path. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land a single canonical SQLite-backed subagent registry path only after proving a real JSON-to-SQLite upgrade/restart flow and coordinating the replaced hot-path pull request. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a backend persistence replacement PR rather than a standalone bug report. Current-main source verifies the existing JSON-backed behavior, and the PR proof is focused fixture coverage of the new SQLite path. Is this the best way to solve the issue? Unclear: the SQLite direction matches the existing AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 26bf8f0dc87b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +318, Tests +134. Total +452 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
Real behavior proofBehavior addressed: migrate legacy JSON subagent registry data into the SQLite Real environment tested: AWS Crabbox Exact steps or command run after this patch: node scripts/crabbox-wrapper.mjs run \
--provider aws \
--idle-timeout 90m \
--ttl 240m \
--timing-json \
--env-from-profile <redacted-openai-profile> \
--allow-env OPENAI_API_KEY \
--download /tmp/openclaw-sqlite-migration-proof.json=/tmp/openclaw-sqlite-migration-proof.remote.json \
--script /tmp/openclaw-crabbox-sqlite-migration-install-e2e.shEvidence after fix: baseline pre-SQLite Observed result after fix: SQLite contained two migrated What was not tested: published |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae69064010
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (runs.size > 0) { | ||
| return runs; |
There was a problem hiding this comment.
If subagent_runs is already populated while an old subagents/runs.json still exists (for example, a first post-upgrade write occurs before the migration read, or the process crashes after the SQLite save but before removeLegacySubagentRegistryFile()), this early return leaves the legacy file behind. Once the current SQLite rows are later swept/released, the next loadSubagentRegistryFromSqlite() sees an empty table and imports that stale JSON, resurrecting old subagent runs despite the intended one-way migration.
Useful? React with 👍 / 👎.
* fix(agents): persist subagent registry in sqlite * test(agents): mock sqlite subagent registry in loop guard
* fix(agents): persist subagent registry in sqlite * test(agents): mock sqlite subagent registry in loop guard
* fix(agents): persist subagent registry in sqlite * test(agents): mock sqlite subagent registry in loop guard
* fix(agents): persist subagent registry in sqlite * test(agents): mock sqlite subagent registry in loop guard
1 participant
Summary
subagent_runsreads, upserts, and snapshot deletes.subagents/runs.jsononce when SQLite is empty, then removes the legacy file so stale runs cannot be re-imported.Replaces #88166 with the SQLite-backed approach discussed there.
Verification
node scripts/run-vitest.mjs src/agents/subagent-registry.store.sqlite.test.ts src/agents/subagent-registry.persistence.test.ts src/gateway/session-utils.subagent.test.tspnpm tsgo:corepnpm tsgo:core:testnode scripts/run-oxlint.mjs src/agents/subagent-registry-state.ts src/agents/subagent-registry.store.sqlite.ts src/agents/subagent-registry.store.sqlite.test.ts.agents/skills/autoreview/scripts/autoreview --mode local->autoreview clean: no accepted/actionable findings reportedReal behavior proof
Behavior addressed: Subagent registry runs persist through the shared SQLite state database instead of a standalone JSON registry file, while existing JSON state migrates on first read.
Real environment tested: Local macOS checkout with Node/Vitest/tsgo/oxlint; SQLite state database exercised through temp
OPENCLAW_STATE_DIRtest fixtures.Exact steps or command run after this patch:
node scripts/run-vitest.mjs src/agents/subagent-registry.store.sqlite.test.ts src/agents/subagent-registry.persistence.test.ts src/gateway/session-utils.subagent.test.ts.Evidence after fix: Focused tests covered SQLite persistence, whole-registry snapshot deletion, and one-shot legacy JSON import/removal.
Observed result after fix: 3 test files passed, 48 tests passed;
tsgo:core,tsgo:core:test, targeted oxlint, and autoreview also passed.What was not tested: Full
pnpm check:changed; an earlier Testbox delegation was stopped after it became stale relative to a local cleanup patch.