Skip to content

fix(automations): read Hermes's state.db off the main thread, one batch per page - #24650

Closed
Jinwoo-H wants to merge 1 commit into
sta-9122-gokey-codex-off-mainfrom
sta-9122-hermes-off-main
Closed

Jinwoo-H wants to merge 1 commit into
sta-9122-gokey-codex-off-mainfrom
sta-9122-hermes-off-main

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​240 $\color{#cf222e}{\Huge{\mathbf{−}}}$​28 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​212
Prod 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​282 $\color{#cf222e}{\Huge{\mathbf{−}}}$​88 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​194

ELI5

When you open Orca's Automations page, Orca reads Hermes's database to list when each scheduled job ran. That read ran on the thread that draws Orca's window, once per run. This PR moves it to a background thread and reads a whole page of runs in one go.

What Changed

  • Before: hermes-cron-output.ts opened Hermes's state.db synchronously on the main thread, and hydrateHermesRunRef opened it once per run through Promise.all.
  • After:
    • The two raw SQLite reads are readers in foreign-sqlite-readers/readers/hermes-session-runs.ts.
    • A page of runs is a single request carrying every id, so the database opens once per page.
    • Turning rows into runs, formatting, and the run-count cache stay on the main thread.
  • Failure behaviour is unchanged: a failed run-list read gives [], and a run the batch can't read falls back to its output file or ref.

Why

Same approach as #24603: reuse the shared worker and never fall back to the main thread. The src/relay/ Hermes reader runs on remote hosts and shares none of these functions, so it's untouched.

Linked Issue

Related to #24360. Stacked on #24649.

Visual Proof

N/A. No UI change; the Automations page shows the same data.

Testing

  • Automated tests added/updated: the readers, the batched read (one request per page, with dedupe checked by counting worker requests), and the failure fallbacks.
  • Gates:
    • pnpm tc passes.
    • 619 test files / 6,206 tests pass, including src/relay.
    • oxlint and check:code-quality:changed pass.
    • The electron-vite build and pnpm build:orcad pass.

Review

Agent skill upstream boundary

  • Not applicable

Notes

A page currently uses two short-lived reader threads, one per read kind. Sharing one Hermes thread would need the reader class to handle more than one result type, which wasn't worth it here.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • N/A with reason for visual proof
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path impact considered
  • typecheck, tests, lint, and build pass locally

… worker (STA-9122)

Before: opening the Automations page read Hermes's state.db with node:sqlite
on the main thread: once per Hermes job for the run-count badge, and once per
run when a page of runs was shown, each run opening the database again.

After: the two raw SQLite reads are pure readers in
foreign-sqlite-readers/readers/hermes-session-runs.ts, run only on the worker.
Mapping rows into runs, merging with output files, and the run-count cache
stay on the main thread.

- A page of runs is one hermesSessionRuns request carrying every session id,
  so state.db opens once per page instead of once per run.
- Failure behaviour per call site is unchanged: the run-ref read resolves to
  [] (count and list fall back to output files), and a run the batch could not
  read hydrates from its output file or ref, as a null per-run read did. The
  existsSync guard stays on the main thread, so no thread starts without
  state.db.
- Each kind has its own lane with the default 30 s idle teardown.

Not changed: src/relay/ keeps its own Hermes reader (hermes-session-run-database.ts)
and shares none of these functions. orcad does not bundle hermes-cron-output.

Tests: readers against a real state.db (job filter with an escaped LIKE,
newest first, message order, missing runs left out, missing/corrupt throw),
dispatch routing, client (one request per page, failure values, dedupe per id
set), and the page reader (one batched call, no call without state.db or for
counts, output-file fallback on worker failure).

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

ℹ️ Minor suggestions only — one weak test assertion.

Reviewed changes

  • Hermes's state.db reads moved onto the shared worker: hermes-cron-output.ts now calls readHermesSessionRunRefRows / readHermesSessionRuns through foreign-sqlite-reader-spawn.ts instead of opening the DB synchronously; the raw SQL lives in readers/hermes-session-runs.ts, with parsers/failure values in hermes-session-runs-result.ts and two new request kinds wired through the protocol, dispatch, and client.
  • One state.db open per page: readHermesCronOutputRunsPage collects the page's session ids and reads them in a single batched request, hydrating from a Map; a missing or failed run still falls back to its output file or ref, preserving the prior per-run failure behaviour.
  • Tests: reader SQL/throws, dispatch routing, client batching + failure + in-flight sharing, and page-level failure fallbacks.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

const { readHermesCronOutputRunsPage } = await loadReader()
const withRefs = await readHermesCronOutputRunsPage('job-1', { page: 1, pageSize: 25 })
expect(withRefs.runs.length).toBeGreaterThan(0)
expect(withRefs.runs.every((run) => run !== null)).toBe(true)

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.

This assertion can never fail: hydrateHermesRunRef always falls back to ref, so no entry is ever null, and the length > 0 above only proves the array is non-empty. Pin the concrete run the failure path should still produce, otherwise this half of the test cannot catch a regression in the fallback.

Suggested change
expect(withRefs.runs.every((run) => run !== null)).toBe(true)
expect(withRefs.runs).toMatchObject([{ id: 'job-1:2026-05-15_09-02-00.md' }])

@Jinwoo-H

Jinwoo-H commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Closing: low value now that the scope is narrowed. It only affects Hermes users when they open the Automations page. See #24649.

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.

1 participant