-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(hermes): use PRAGMA temp_store=MEMORY in SessionDB to fix sessions delete (#8301) #8308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
d56bd65
fix(hermes): use PRAGMA temp_store=MEMORY in SessionDB to fix FK dele…
yanyunl1991 c1ff4ed
fix(hermes): update image-build-probes hash and test fixtures for sql…
yanyunl1991 6c31cf5
Merge remote-tracking branch 'origin/main' into fix/hermes-session-de…
yanyunl1991 4d35fb4
fix(test): add missing sqlite-temp-store fixture to three test suites…
yanyunl1991 6d3fd37
merge: resolve conflicts with main
github-actions[bot] 5b03565
fix(hermes): add sha256 gate and tighten already-patched check for sq…
yanyunl1991 5f2d0e0
Merge remote-tracking branch 'origin/fix/hermes-session-delete-sqlite…
yanyunl1991 48594fd
fix(test): register sqlite temp-store patcher SHA256 ARG as integrity…
yanyunl1991 a22e3f7
test(hermes): exercise session deletion through host CLI
apurvvkumaria 2fc714e
merge: refresh PR branch from main
apurvvkumaria f17464f
fix(hermes): reject ambiguous SQLite temp-store patches
cv 9f46b7e
merge(main): refresh PR #8308 after gateway teardown repair
cv 044567e
merge(main): refresh PR #8308 after Shields status repair
cv a89b9be
docs(hermes): describe SQLite patch states
cv 7494b29
merge: resolve conflicts with main
github-actions[bot] f8dd26b
merge(main): refresh PR #8308 after Hermes cron restore
cv 169d578
merge: preserve concurrent PR #8308 refresh
cv e4a19fa
docs(hermes): use controlled product names
cv 7b797fb
merge(main): refresh PR #8308 after llama.cpp runtime
cv 7e85eff
merge(main): refresh PR #8308
apurvvkumaria 64e403b
merge(main): refresh PR #8308
cjagwani 3b633c3
test(hermes): surface config hash replay failures
cjagwani b41d428
merge: resolve conflicts with main
github-actions[bot] 63b3428
merge(main): refresh Hermes session deletion fix
apurvvkumaria 78e095c
merge(main): refresh Hermes session deletion fix
apurvvkumaria 6f044ef
test(hermes): cover SQLite patcher integrity
apurvvkumaria 3c5bdc4
merge(main): refresh Hermes session deletion fix
apurvvkumaria 13ce1e7
merge(main): refresh Hermes session deletion fix
apurvvkumaria 7e9a8d2
merge(main): refresh Hermes session deletion fix
apurvvkumaria 0178597
merge(main): refresh Hermes session deletion fix
apurvvkumaria 588d7c0
Merge branch 'main' into fix/hermes-session-delete-sqlite-cantopen-8301
cv 3738371
Merge branch 'main' into fix/hermes-session-delete-sqlite-cantopen-8301
cv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| #!/usr/bin/env python3 | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| """Patch SessionDB.__init__ to use in-memory temp store for SQLite FK processing. | ||
|
|
||
| Source-of-truth note for this localized Hermes runtime patch: | ||
| - Invalid state: Hermes v0.19.0 SessionDB does not set PRAGMA temp_store=MEMORY, | ||
| so SQLite falls back to file-based temp storage when processing FK constraints | ||
| (for example, the ON DELETE CASCADE on session_model_usage -> sessions). When | ||
| `hermes sessions delete` is invoked through OpenShell sandbox execution — | ||
| the code path used by `nemohermes <sandbox> sessions delete <id>` — the | ||
| process runs in a restricted environment where SQLite's temp-file creation | ||
| syscalls fail with | ||
| SQLITE_CANTOPEN, causing every `DELETE FROM sessions` with FK enforcement | ||
| enabled to raise `sqlite3.OperationalError: unable to open database file` | ||
| (#8301). The same command succeeds through Docker execution because that | ||
| context allows the file-based temp store. | ||
| - Value being patched: pinned/prebuilt `/opt/hermes/hermes_state.py` | ||
| `SessionDB.__init__` connection setup block; specifically, the statement | ||
| immediately following `apply_wal_with_fallback()` that enables FK enforcement. | ||
| `PRAGMA temp_store=MEMORY` is inserted before `PRAGMA foreign_keys=ON` so the | ||
| in-memory store is active before any FK-constrained write. | ||
| - Source-fix constraint: NemoClaw layers a sandbox image on top of the | ||
| published Hermes runtime; the source fix belongs upstream in Hermes, not in | ||
| NemoClaw's TypeScript or wrapper code. | ||
| - Regression evidence: on first application, this patcher accepts exactly one | ||
| unpatched connection setup block and no temp-store statement. A later | ||
| application accepts exactly one complete patched block with one temp-store | ||
| statement. Every other source shape fails without writing. The Dockerfile | ||
| checks for the inserted PRAGMA after patching. The image-build | ||
| `session-delete` behavior test creates a SessionDB, inserts a session with | ||
| messages, and calls `delete_session()` to confirm that SQLite does not raise | ||
| OperationalError. | ||
| - Removal condition: delete this patch when the pinned Hermes runtime natively | ||
| sets `PRAGMA temp_store=MEMORY` (or equivalent) in `SessionDB.__init__`. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| from pathlib import Path | ||
|
|
||
| OLD = ( | ||
| 'apply_wal_with_fallback(self._conn, db_label="state.db")\n' | ||
| ' self._conn.execute("PRAGMA foreign_keys=ON")' | ||
| ) | ||
| NEW = ( | ||
| 'apply_wal_with_fallback(self._conn, db_label="state.db")\n' | ||
| ' self._conn.execute("PRAGMA temp_store=MEMORY")\n' | ||
| ' self._conn.execute("PRAGMA foreign_keys=ON")' | ||
| ) | ||
| EXPECTED_OCCURRENCES = 1 | ||
|
|
||
|
|
||
| def patch_file(path: Path) -> None: | ||
| source = path.read_text(encoding="utf-8") | ||
| old_count = source.count(OLD) | ||
| new_count = source.count('self._conn.execute("PRAGMA temp_store=MEMORY")') | ||
| patched_count = source.count(NEW) | ||
| if ( | ||
| old_count == 0 | ||
| and new_count == EXPECTED_OCCURRENCES | ||
| and patched_count == EXPECTED_OCCURRENCES | ||
| ): | ||
| return | ||
| if old_count != EXPECTED_OCCURRENCES or new_count != 0: | ||
| raise SystemExit( | ||
| "ERROR: Hermes SessionDB.__init__ connection setup shape changed; " | ||
| f"expected {EXPECTED_OCCURRENCES} unpatched block and no temp-store " | ||
| f"statements; found {old_count} unpatched blocks, {new_count} temp-store " | ||
| f"statements, and {patched_count} complete patched blocks" | ||
| ) | ||
| path.write_text(source.replace(OLD, NEW), encoding="utf-8") | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def main() -> int: | ||
| parser = argparse.ArgumentParser() | ||
| parser.add_argument( | ||
| "path", | ||
| nargs="?", | ||
| default="/opt/hermes/hermes_state.py", | ||
| help="Hermes state module to patch", | ||
| ) | ||
| args = parser.parse_args() | ||
| patch_file(Path(args.path)) | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main()) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { spawnSync } from "node:child_process"; | ||
| import { createHash } from "node:crypto"; | ||
| import fs from "node:fs"; | ||
| import os from "node:os"; | ||
| import path from "node:path"; | ||
|
|
||
| import { afterEach, describe, expect, it } from "vitest"; | ||
|
|
||
| const root = path.join(import.meta.dirname, ".."); | ||
| const patcher = path.join(root, "agents", "hermes", "patch-hermes-sqlite-temp-store.py"); | ||
| const dockerfile = fs.readFileSync(path.join(root, "agents", "hermes", "Dockerfile"), "utf8"); | ||
| const fixtures: string[] = []; | ||
|
|
||
| const walSetup = 'apply_wal_with_fallback(self._conn, db_label="state.db")'; | ||
| const tempStore = ' self._conn.execute("PRAGMA temp_store=MEMORY")'; | ||
| const foreignKeys = ' self._conn.execute("PRAGMA foreign_keys=ON")'; | ||
| const unpatchedSource = `${walSetup}\n${foreignKeys}\n`; | ||
| const patchedSource = `${walSetup}\n${tempStore}\n${foreignKeys}\n`; | ||
|
|
||
| function fixtureFile(source: string): string { | ||
| const fixture = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-hermes-sqlite-temp-store-")); | ||
| fixtures.push(fixture); | ||
| const stateModule = path.join(fixture, "hermes_state.py"); | ||
| fs.writeFileSync(stateModule, source); | ||
| return stateModule; | ||
| } | ||
|
|
||
| function runPatcher(stateModule: string) { | ||
| return spawnSync("python3", ["-I", patcher, stateModule], { | ||
| encoding: "utf8", | ||
| timeout: 5000, | ||
| }); | ||
| } | ||
|
|
||
| afterEach(() => { | ||
| for (const fixture of fixtures.splice(0)) { | ||
| fs.rmSync(fixture, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| describe("Hermes SQLite temp-store patch", () => { | ||
| it("inserts in-memory temp storage before foreign-key enforcement (#8301)", () => { | ||
| const stateModule = fixtureFile(unpatchedSource); | ||
|
|
||
| const result = runPatcher(stateModule); | ||
|
|
||
| expect(result.status, result.stderr).toBe(0); | ||
| expect(fs.readFileSync(stateModule, "utf8")).toBe(patchedSource); | ||
| }); | ||
|
|
||
| it("accepts one already-patched connection block (#8301)", () => { | ||
| const stateModule = fixtureFile(patchedSource); | ||
|
|
||
| const result = runPatcher(stateModule); | ||
|
|
||
| expect(result.status, result.stderr).toBe(0); | ||
| expect(fs.readFileSync(stateModule, "utf8")).toBe(patchedSource); | ||
| }); | ||
|
|
||
| it.each([ | ||
| ["duplicate", `${patchedSource}${tempStore}\n`], | ||
| ["partial", `${walSetup}\n${tempStore}\n`], | ||
| ["misplaced", `${tempStore}\n${unpatchedSource}`], | ||
| ])("rejects a %s temp-store patch (#8301)", (_case, source) => { | ||
| const stateModule = fixtureFile(source); | ||
|
|
||
| const result = runPatcher(stateModule); | ||
|
|
||
| expect(result.status).toBe(1); | ||
| expect(result.stderr).toContain("Hermes SessionDB.__init__ connection setup shape changed"); | ||
| expect(fs.readFileSync(stateModule, "utf8")).toBe(source); | ||
| }); | ||
|
|
||
| it("binds the Hermes image to the reviewed patcher (#8301)", () => { | ||
| const digest = createHash("sha256").update(fs.readFileSync(patcher)).digest("hex"); | ||
|
|
||
| expect(dockerfile).toContain(`ARG NEMOCLAW_HERMES_SQLITE_TEMP_STORE_PATCHER_SHA256=${digest}`); | ||
| expect(dockerfile).toContain( | ||
| "COPY agents/hermes/patch-hermes-sqlite-temp-store.py " + | ||
| "/usr/local/lib/nemoclaw/patch-hermes-sqlite-temp-store.py", | ||
| ); | ||
| expect(dockerfile).toContain( | ||
| "RUN /usr/bin/python3 -I /usr/local/lib/nemoclaw/patch-hermes-sqlite-temp-store.py", | ||
| ); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.