[PR #5818] [MERGED] feat(sqlite): make worker deadlines adjustable with recovery guidance #7080

Closed
opened 2026-11-01 21:45:15 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5818
Author: @gensokyo-dev
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @gensokyo-dev

Base: main ← Head: feat/20261021-sqlite-timeout-recovery


📝 Commits (4)

  • f1fbd1d feat(sqlite): make worker deadlines adjustable with recovery guidance
  • 200f493 fix(sqlite): require known budgets for fresh observations
  • b58ca53 fix(sqlite): retain typed timeout and one output owner through cleanup
  • 3e5743b test(sqlite): hand fixture ownership to supervision immediately

📊 Changes

15 files changed (+496 additions, -49 deletions)

View changed files

📝 docs/sqlite-databases.md (+23 -2)
📝 src/application/sqlite/SqliteDatabaseService.test.ts (+70 -0)
📝 src/application/sqlite/SqliteDatabaseService.ts (+5 -1)
📝 src/cli/sqliteCommands.ts (+5 -0)
📝 src/contracts/sqlite/sqliteToolContracts.ts (+1 -1)
📝 src/domain/sqlite/sqliteDatabase.ts (+17 -0)
📝 src/sqlite/SqliteDatabaseFailures.ts (+91 -27)
📝 src/sqlite/SqliteDatabaseInspection.ts (+1 -1)
📝 src/sqlite/SqliteDatabaseLimits.ts (+0 -1)
📝 src/sqlite/SqliteDatabaseProvider.ts (+12 -6)
📝 src/sqlite/SqliteDatabaseWorkerExecution.ts (+6 -3)
📝 tests/acceptance/sqliteDatabaseCli.test.ts (+21 -3)
📝 tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts (+10 -1)
➕ tests/boundary/sqlite/sqliteDatabaseFailures.test.ts (+88 -0)
📝 tests/boundary/sqlite/sqliteDatabaseProvider.test.ts (+146 -3)

📄 Description

Summary

Fixes #5812. Follow-up to #5808 and the RepoPrompt comparison in #5598.

SQLite's fixed 30-second worker deadline offered no actual public repair. Agents can now select worker_timeout_ms / --worker-timeout-ms and receive precise timeout versus fixed-limit guidance, without changing the database, requested facts or safety bounds.

Problem and expected behavior

Raising the MCP client's deadline did not change the SQLite worker budget. Timeout plus cleanup failure also lost the typed primary timeout facts. Expose the real per-call control and retain its actual scope/value in Evidence; do not invent adjustable byte/heap ceilings or repeat deterministic refusals.

Change and scope

The existing parent provider resolves one budget and passes it to the owned supervisor, result and failure mapper. Default remains 30000 ms; accepted timer range is 1..2147483647. Worker wire replies cannot claim parent-owned execution metadata. Historical missing metadata stays unknown; fresh service results must report selected-or-default settings. One shared SQLite cleanup composer preserves typed primaries and delivers captured output once, including after cleanup resolves. Fixed value/reply recovery suggests exact columns/sample/schema selection only when the question permits it. Existing read-only authority and cleanup ownership remain unchanged.

Contract and boundary impact

  • Semantic owner and earliest changed stage: canonical SQLite input/result contract; parent provider owns deadline selection.
  • Contribution scope fit and linked maintainer decision: existing local-only SQLite workflow; new capability/provider/target-execution decision not applicable.
  • CLI and MCP/tool-catalog contract: additive optional timeout input and optional historical-compatible result metadata; shared service enforces caller selections.
  • Provider, bridge, target-format, or platform compatibility: existing native SQLite runtime requirement unchanged; locally verified Linux / Node 24.18.0.
  • Evidence, artifact, provenance, or reconstruction contract: actual budget in current results and completed partial observations; explicit parameters preserved, omission remains omission; DB/WAL identity retained.
  • Target selection, authority, upstream behavior, and cleanup details: same private snapshots and owned worker; no arbitrary SQL, target execution, source mutation or new installation. Cleanup blocks retry; nested timeout diagnostics avoid a second output copy.
  • Generated metadata/package/installation impact: ignored catalogs regenerated and validated; no generated outputs, binaries or dependencies committed.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process. (Existing owned Node SQLite worker only; no new attachment.)
  • Inject instrumentation or run scripts/code. (Existing owned worker script only; no injection or target code.)
  • Change process memory, registers, files, or configuration. (Existing private snapshot/request/reply files only; no source/configuration mutation.)
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Service guards for invalid/mismatched budgets, actual owned-worker timeout/corrected retry, typed timeout plus cleanup, max-timer guidance, CLI/MCP custom budget, fixed admission despite larger deadline, cancellation/cleanup partial Evidence.

  • Six focused guards failed before implementation and pass after; not every test was run fail-before.

  • Representative output: provider_timeout, details.timeout_ms: 1, remediation names worker_timeout_ms / --worker-timeout-ms; retry at 30000 returns the same requested Chinook rows with actual budget and original SHA-256. Oversized input remains non-retryable resource_constraint even at 60000.

  • Detailed evidence and whole-agent measurements: initial ten-session record, final review fixes and 13-session verification.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:sqlite — 155 passed, 3 skipped, 12 files (final run 69.39 s). New fail-before guards also reproduce fresh default omission, duplicate output on the actual retained-cleanup path, and missing typed timeout restoration.
  • npm run check — passed; existing lint warnings remain.
  • npm run docs:check, npm run verify:test-discovery — passed after final test additions (5 README checks; all 1900 test files discovered exactly once).
  • Independent public CLI/MCP verifier — six pairs passed; SDK-converted AJV 2020 schemas, timer bounds, full retained Evidence readback, independent Python SQLite row oracle and source identity.
  • Actual Codex CLI 0.161.0 / gpt-6.1-sol — 13 recorded sessions: nine completed answers, one legitimate fixed-input blocker, three pre-REA host-discovery blockers kept as failures. Four 1 ms timeout recoveries (including final cleanup code), invalid-input correction, fixed-value column selection, oversized honest stop and before/after controls. No unchanged failed retries or surviving sampled owned processes.
  • Whole 13-session cost: 1292760 input tokens (1022720 cached), 6559 output, 420.43 s wall, 135.04 s CPU, maximum sampled process-family RSS 1121.2 MiB (500 ms sampling). Fresh processes; uncontrolled OS/model caches. This is not a speedup claim or kernel/native-memory bound.
  • Separate initial read-only Codex source review found no issues; GitHub Codex/Greptile subsequently found three contract/cleanup issues, fixed with stronger regressions. Greptile's fourth finding was a test-only readiness handoff leak: the unnecessary wait was removed, so ownership transfers immediately; 27 provider tests passed again (17.05 s). Final head 3e5743ba6 has a clean Codex re-review, successful Greptile review and all four threads resolved. None establishes human review.
  • CI on prior head exposed a pre-existing WABT CLI requiredness-test failure, reproduced at e9cdf873c58bd32e8f43b69f8a9a6d1ceb50d645 before SQLite; separately tracked in #5845, not mixed into this PR. Final CI status is reported below, not assumed green.

Compatibility, safety, and release

  • Breaking changes or migration: none; defaults and historical Evidence accepted without inventing missing settings.
  • Real provider/OS coverage: Linux native SQLite only. Natural 30-second slow databases, other OS/Node versions, other providers and native memory exhaustion unverified.
  • Package/release impact: additive documented SQLite controls; no dependency or installer changes.
  • Safety: no raised byte/reply/heap limits, source writes, SQL authority or process ownership relaxation. Native SQLITE_NOMEM classification remains separate follow-up.

Final CI status — 3e5743ba6

Build, source tests, static checks, docs, Windows-controls build and all four Linux SQLite runtime jobs (Node 22.x, 24.15.0, 24.x, 26.x) passed. Seven portable test shards passed; shard 4 has only the pre-existing WABT CLI requiredness oracle failure (log, #5845), reproduced before this SQLite change. CI is not all green; dependent rollups inherit that failure. The human-declaration check also fails for the intentionally unfilled attestations below. Merge uses the user's explicit standing override authorization, not fabricated attestations. Main advanced with unrelated Rust #5815; a scoped diff found no changes to the touched SQLite paths.

Author readiness

Human owner: @gensokyo-dev

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

The user explicitly instructed ready-for-review submission and authorized continuation/merge overrides. The first two human-review attestations remain unchecked because the agent cannot truthfully claim those human acts occurred. Automated checks/source review are not substitutes for them; no all-green readiness claim is made.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5818 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `feat/20261021-sqlite-timeout-recovery` --- ### 📝 Commits (4) - [`f1fbd1d`](https://github.com/morluto/rea/commit/f1fbd1d624f01527afc10c6fc7e2a134fbb33fb9) feat(sqlite): make worker deadlines adjustable with recovery guidance - [`200f493`](https://github.com/morluto/rea/commit/200f493d99c5e38ef6ee24a51492672e27ad216c) fix(sqlite): require known budgets for fresh observations - [`b58ca53`](https://github.com/morluto/rea/commit/b58ca531f1f978a5813d3fe5b8e054171fe0466b) fix(sqlite): retain typed timeout and one output owner through cleanup - [`3e5743b`](https://github.com/morluto/rea/commit/3e5743ba6d9291bdb33cd47336a0e6ae50c20319) test(sqlite): hand fixture ownership to supervision immediately ### 📊 Changes **15 files changed** (+496 additions, -49 deletions) <details> <summary>View changed files</summary> 📝 `docs/sqlite-databases.md` (+23 -2) 📝 `src/application/sqlite/SqliteDatabaseService.test.ts` (+70 -0) 📝 `src/application/sqlite/SqliteDatabaseService.ts` (+5 -1) 📝 `src/cli/sqliteCommands.ts` (+5 -0) 📝 `src/contracts/sqlite/sqliteToolContracts.ts` (+1 -1) 📝 `src/domain/sqlite/sqliteDatabase.ts` (+17 -0) 📝 `src/sqlite/SqliteDatabaseFailures.ts` (+91 -27) 📝 `src/sqlite/SqliteDatabaseInspection.ts` (+1 -1) 📝 `src/sqlite/SqliteDatabaseLimits.ts` (+0 -1) 📝 `src/sqlite/SqliteDatabaseProvider.ts` (+12 -6) 📝 `src/sqlite/SqliteDatabaseWorkerExecution.ts` (+6 -3) 📝 `tests/acceptance/sqliteDatabaseCli.test.ts` (+21 -3) 📝 `tests/boundary/sqlite/sqliteDatabaseDescriptorCleanup.test.ts` (+10 -1) ➕ `tests/boundary/sqlite/sqliteDatabaseFailures.test.ts` (+88 -0) 📝 `tests/boundary/sqlite/sqliteDatabaseProvider.test.ts` (+146 -3) </details> ### 📄 Description ## Summary Fixes #5812. Follow-up to #5808 and the RepoPrompt comparison in #5598. SQLite's fixed 30-second worker deadline offered no actual public repair. Agents can now select `worker_timeout_ms` / `--worker-timeout-ms` and receive precise timeout versus fixed-limit guidance, without changing the database, requested facts or safety bounds. ## Problem and expected behavior Raising the MCP client's deadline did not change the SQLite worker budget. Timeout plus cleanup failure also lost the typed primary timeout facts. Expose the real per-call control and retain its actual scope/value in Evidence; do not invent adjustable byte/heap ceilings or repeat deterministic refusals. ## Change and scope The existing parent provider resolves one budget and passes it to the owned supervisor, result and failure mapper. Default remains 30000 ms; accepted timer range is 1..2147483647. Worker wire replies cannot claim parent-owned execution metadata. Historical missing metadata stays unknown; fresh service results must report selected-or-default settings. One shared SQLite cleanup composer preserves typed primaries and delivers captured output once, including after cleanup resolves. Fixed value/reply recovery suggests exact columns/sample/schema selection only when the question permits it. Existing read-only authority and cleanup ownership remain unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: canonical SQLite input/result contract; parent provider owns deadline selection. - Contribution scope fit and linked maintainer decision: existing local-only SQLite workflow; new capability/provider/target-execution decision not applicable. - CLI and MCP/tool-catalog contract: additive optional timeout input and optional historical-compatible result metadata; shared service enforces caller selections. - Provider, bridge, target-format, or platform compatibility: existing native SQLite runtime requirement unchanged; locally verified Linux / Node 24.18.0. - Evidence, artifact, provenance, or reconstruction contract: actual budget in current results and completed partial observations; explicit parameters preserved, omission remains omission; DB/WAL identity retained. - Target selection, authority, upstream behavior, and cleanup details: same private snapshots and owned worker; no arbitrary SQL, target execution, source mutation or new installation. Cleanup blocks retry; nested timeout diagnostics avoid a second output copy. - Generated metadata/package/installation impact: ignored catalogs regenerated and validated; no generated outputs, binaries or dependencies committed. Expected effects: - [x] Read existing files or captured data. - [x] Launch or attach to a process. (Existing owned Node SQLite worker only; no new attachment.) - [x] Inject instrumentation or run scripts/code. (Existing owned worker script only; no injection or target code.) - [x] Change process memory, registers, files, or configuration. (Existing private snapshot/request/reply files only; no source/configuration mutation.) - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Service guards for invalid/mismatched budgets, actual owned-worker timeout/corrected retry, typed timeout plus cleanup, max-timer guidance, CLI/MCP custom budget, fixed admission despite larger deadline, cancellation/cleanup partial Evidence. - Six focused guards failed before implementation and pass after; not every test was run fail-before. - Representative output: `provider_timeout`, `details.timeout_ms: 1`, remediation names `worker_timeout_ms` / `--worker-timeout-ms`; retry at 30000 returns the same requested Chinook rows with actual budget and original SHA-256. Oversized input remains non-retryable `resource_constraint` even at 60000. - Detailed evidence and whole-agent measurements: [initial ten-session record](https://github.com/morluto/rea/issues/5812#issuecomment-6311080292), [final review fixes and 13-session verification](https://github.com/morluto/rea/issues/5812#issuecomment-6311506440). - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:sqlite` — **155 passed, 3 skipped**, 12 files (final run **69.39 s**). New fail-before guards also reproduce fresh default omission, duplicate output on the actual retained-cleanup path, and missing typed timeout restoration. - `npm run check` — passed; existing lint warnings remain. - `npm run docs:check`, `npm run verify:test-discovery` — passed after final test additions (5 README checks; all 1900 test files discovered exactly once). - Independent public CLI/MCP verifier — six pairs passed; SDK-converted AJV 2020 schemas, timer bounds, full retained Evidence readback, independent Python SQLite row oracle and source identity. - Actual Codex CLI 0.161.0 / `gpt-6.1-sol` — **13 recorded sessions**: nine completed answers, one legitimate fixed-input blocker, three pre-REA host-discovery blockers kept as failures. **Four** 1 ms timeout recoveries (including final cleanup code), invalid-input correction, fixed-value column selection, oversized honest stop and before/after controls. No unchanged failed retries or surviving sampled owned processes. - Whole 13-session cost: **1292760 input tokens (1022720 cached), 6559 output, 420.43 s wall, 135.04 s CPU**, maximum sampled process-family RSS **1121.2 MiB** (500 ms sampling). Fresh processes; uncontrolled OS/model caches. This is not a speedup claim or kernel/native-memory bound. - Separate initial read-only Codex source review found no issues; GitHub Codex/Greptile subsequently found three contract/cleanup issues, fixed with stronger regressions. Greptile's fourth finding was a test-only readiness handoff leak: the unnecessary wait was removed, so ownership transfers immediately; 27 provider tests passed again (17.05 s). Final head `3e5743ba6` has a clean Codex re-review, successful Greptile review and all four threads resolved. None establishes human review. - CI on prior head exposed a pre-existing WABT CLI requiredness-test failure, reproduced at `e9cdf873c58bd32e8f43b69f8a9a6d1ceb50d645` before SQLite; separately tracked in #5845, not mixed into this PR. Final CI status is reported below, not assumed green. ## Compatibility, safety, and release - Breaking changes or migration: none; defaults and historical Evidence accepted without inventing missing settings. - Real provider/OS coverage: Linux native SQLite only. Natural 30-second slow databases, other OS/Node versions, other providers and native memory exhaustion unverified. - Package/release impact: additive documented SQLite controls; no dependency or installer changes. - Safety: no raised byte/reply/heap limits, source writes, SQL authority or process ownership relaxation. Native `SQLITE_NOMEM` classification remains separate follow-up. ## Final CI status — `3e5743ba6` Build, source tests, static checks, docs, Windows-controls build and all four Linux SQLite runtime jobs (Node 22.x, 24.15.0, 24.x, 26.x) passed. Seven portable test shards passed; shard 4 has only the pre-existing WABT CLI requiredness oracle failure ([log](https://github.com/morluto/rea/actions/runs/39569747788/job/119121570706), #5845), reproduced before this SQLite change. CI is **not all green**; dependent rollups inherit that failure. The human-declaration check also fails for the intentionally unfilled attestations below. Merge uses the user's explicit standing override authorization, not fabricated attestations. Main advanced with unrelated Rust #5815; a scoped diff found no changes to the touched SQLite paths. ## Author readiness Human owner: @gensokyo-dev - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. The user explicitly instructed ready-for-review submission and authorized continuation/merge overrides. The first two human-review attestations remain unchecked because the agent cannot truthfully claim those human acts occurred. Automated checks/source review are not substitutes for them; no all-green readiness claim is made. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:15 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7080
No description provided.