close
Skip to content

fix(gateway): refuse to boot when the data home cannot persist state - #2279

Merged
iamwhatever merged 1 commit into
kirodotdev:mainfrom
rubencu:fix/gateway-persistence-preflight
Aug 9, 2026
Merged

fix(gateway): refuse to boot when the data home cannot persist state#2279
iamwhatever merged 1 commit into
kirodotdev:mainfrom
rubencu:fix/gateway-persistence-preflight

Conversation

@rubencu

@rubencu rubencu commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Problem

A gateway whose environment breaks basic file syscalls still binds its port and serves traffic — but every save silently fails. Chat-slot history writes, cron-history appends, and session-PID tracking all require tempfile.mkstemp (via atomic_write) plus an advisory lock (platform_compat.file_lock) in the data home.

Observed in the wild (Linux dev host, 2026-08-08): a gateway spawned from inside a sandboxed agent session inherits the sandbox's seccomp filter — seccomp survives fork/exec, nohup/disown included — so fcntl.flock and mkstemp raise OSError(ENOSYS) while writes to already-open fds keep working. The result over one 5-minute lifetime:

  • every chat-slot save at shutdown failed (Failed to save slot ... to history × N — silent chat-history loss),
  • cron-history appends crashed with [Errno 38] Function not implemented,
  • a second gateway started from the same environment died with a raw traceback in cleanup_orphaned_sessions instead of anything actionable.

The process never exits on its own — it limps for its whole lifetime dropping writes.

Fix

  • platform_compat.probe_file_persistence(directory): creates a probe file with mkstemp and locks it with file_lock — the exact two primitives the persistence paths use. Returns None on success or a human-readable failure description; when errno == ENOSYS, the message names the inherited-seccomp cause and the remedy (start from a regular shell or the system service). Probe file removed in all outcomes.
  • GatewayOrchestrator.run() runs the probe against data_home() before the first lock consumer (cleanup_orphaned_sessions, where the raw-traceback crash happened). On failure: CRITICAL log + one-line stderr message + SystemExit(1).

Fail-loud over fail-silent, matching the repo's existing lock philosophy (file_lock already "FAILS CLOSED" on Windows rather than proceeding unserialized) and the pidfd ENOSYS probe precedent in cli.py.

Tests

test/test_persistence_probe.py (11 tests): healthy dir passes / leaves no litter / creates missing dirs; ENOSYS on create and on lock produce step-specific messages with the seccomp hint; non-ENOSYS errors get no misleading hint; probe file removed even when locking fails; orchestrator wiring — run() exits 1 on probe failure and does so before cleanup_orphaned_sessions runs.

Verified: new module green, test_no_config_dir_in_async ratchet green (probe uses data_home(), issue #1057), isort/flake8/mypy clean (mypy 1.14.1, CI-parity venv, 847 files). Full suite run: 38,395 passed; 69 failures reproduced identically on clean origin/main @ 6590a1c on the same host (git/network-dependent modules timing out in the sandboxed environment) — environmental, not from this diff.

@rubencu
rubencu requested a review from a team as a code owner August 8, 2026 22:00
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 8, 2026
@rubencu
rubencu force-pushed the fix/gateway-persistence-preflight branch from b073e3c to 3a359f7 Compare August 9, 2026 01:32
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 5e0c688d51b07e88749a0c254534f4da4529b320 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5e0c688

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — ✅ PASS

Advisory design-level review of 5e0c688d51b07e88749a0c254534f4da4529b320 via the fork AI-review pipeline — updated in place on each push; does not block merge.

Design assessment complete. The change is a boot-time persistence preflight (probe_file_persistence in platform_compat, wired into GatewayOrchestrator.run() before the first lock consumer) that converts a documented silent-data-loss failure mode into a loud, actionable refusal to start. The probe exercises exactly the primitives the real save paths use (mkstemp, write, fsync, file_lock, replace_with_retry, unlink), lives in the module that owns the lock shim, runs off-loop, and is trivially reversible with no contract or data migration surface. Tests pin both the probe semantics and the wiring order. I found no design-level concerns.

Design-Verdict: PASS

Real observed harm, guarded with the exact production primitives, at the right layer, before the first consumer — loud, scoped, and trivially reversible.

[DESIGN-REVIEWED] 5e0c688

@rubencu
rubencu force-pushed the fix/gateway-persistence-preflight branch from 3a359f7 to 03c431a Compare August 9, 2026 01:55
@rubencu

rubencu commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Disposition — GPT round 1 (SHA 3a359f703c431a):

  • Event-loop blocking probe (BLOCKING) — ✅ fixed as suggested. The whole probe, including the data_home() resolution, now runs off-loop via await asyncio.to_thread(...) in GatewayOrchestrator.run(), matching the existing warm_backend off-loop pattern a few lines below. A stalled filesystem can no longer wedge the event loop during startup. Comment updated to state the off-loop rationale. All 11 probe tests, isort/flake8/mypy, and the brand gate pass locally.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Opus 5 Review (fork) — ✅ no blocking findings

Reviewed 5e0c688d51b07e88749a0c254534f4da4529b320 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5e0c688

@rubencu
rubencu force-pushed the fix/gateway-persistence-preflight branch from 03c431a to b7860fc Compare August 9, 2026 02:17
@rubencu

rubencu commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Disposition — GPT round 2 (SHA 03c431ab7860fc):

  • Thread-exhaustion RuntimeError from asyncio.to_thread (BLOCKING) — ✅ fixed as suggested. The await asyncio.to_thread(...) call is now wrapped: RuntimeError (executor exhausted/shut down) is converted into a preflight-failure message and routed through the existing clean CRITICAL-log + SystemExit(1) exit instead of a raw traceback. Rationale in-code: a process that cannot spawn one worker thread at boot cannot run session pools either. Added test_thread_exhaustion_routes_through_clean_exit proving RuntimeError produces the same clean exit code 1. 11 probe tests + isort/flake8/mypy + brand gate green locally.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 9, 2026
bolichen97
bolichen97 previously approved these changes Aug 9, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved for rc.3: green CI, targeted fix, reviewed for insider release cut.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 9, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) August 9, 2026 02:29
auto-merge was automatically disabled August 9, 2026 02:39

Head branch was pushed to by a user without write access

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@rubencu
rubencu force-pushed the fix/gateway-persistence-preflight branch from 375ec8f to 872a57c Compare August 9, 2026 03:45
@rubencu

rubencu commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Disposition — GPT round 4 (SHA 375ec8f872a57c):

  • Probe never tests writing or atomic replacement (BLOCKING) — ✅ fixed as suggested, and generalized. The probe now exercises the complete persistence contract in production order: mkstempos.write (a byte-quota/ENOSPC environment that allows empty files now fails) → file_lockos.replace onto a sibling path (the exact commit primitive atomic_write uses; inlined rather than imported because atomic_write imports platform_compat) → os.unlink. Each step reports its own failure message. Added 3 tests (write-ENOSPC, replace-EPERM, replace-failure cleanup) for 15 total. isort/flake8/mypy + brand gate green locally.

This closes the enumeration: the probe now covers create, write, lock, atomic-replace, and delete — every filesystem primitive the persistence layer performs.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@rubencu
rubencu force-pushed the fix/gateway-persistence-preflight branch from 872a57c to 70b6694 Compare August 9, 2026 04:07
@rubencu

rubencu commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Disposition — GPT round 5 (SHA 872a57c70b6694):

  • Raw os.replace rejects healthy Windows data homes (BLOCKING) — ✅ fixed as suggested. The probe now commits through atomic_write.replace_with_retry — the exact production primitive — which absorbs the bounded Windows AV/indexer sharing-violation window and is a plain os.replace on POSIX. Imported lazily inside the probe because atomic_write imports platform_compat at module top (cycle avoidance, noted in-code). The probe runs off-loop via asyncio.to_thread, so the helper's retry path (gated on no running loop in the thread) is active exactly as it is for production writers. Added test_probe_uses_the_production_replace_primitive (spy asserts the probe routes through the helper) and repointed the two replace-failure tests at the helper. 17 tests + isort/flake8/mypy + brand gate green locally.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 9, 2026
A gateway whose environment breaks basic file syscalls still binds its
port and serves traffic, but every save silently fails: chat-slot
history writes, cron-history appends, and session-PID tracking all need
mkstemp + an advisory lock in the data home. Observed on Linux when a
gateway is spawned from inside a sandboxed agent session: the sandbox's
seccomp filter survives fork/exec (nohup included), so fcntl.flock and
tempfile.mkstemp raise OSError(ENOSYS) while writes to already-open fds
keep working. The process limps along for its whole lifetime dropping
chat history on the floor, and the shutdown save of every slot fails.
A second gateway started from the same environment crashes with a raw
traceback in cleanup_orphaned_sessions instead of a useful error.

Add probe_file_persistence() to platform_compat (mkstemp + file_lock in
the data home, probe file removed in all outcomes) and run it at the top
of GatewayOrchestrator.run(), before the first lock consumer. On failure
the gateway logs CRITICAL, prints an actionable one-liner (naming the
inherited-seccomp cause when errno is ENOSYS), and exits 1 -- loud and
early instead of silent data loss.
@rubencu
rubencu force-pushed the fix/gateway-persistence-preflight branch from 70b6694 to 5e0c688 Compare August 9, 2026 04:29
@rubencu

rubencu commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Disposition — GPT round 6 (SHA 70b66945e0c688):

  • Probe omits fsync (BLOCKING) — ✅ fixed as suggested. os.fsync(fd) now runs after the probe write (its own flush files in step), covering environments where buffered writes succeed but the flush fails with EIO/ENOSPC — matching chat history's atomic_write(..., fsync=True). Added test_fsync_failure_reports_flush_step. 17 tests + isort/flake8/mypy + brand gate green locally.

With this the probe covers create → write → fsync → lock → atomic-replace (via replace_with_retry) → delete — the complete set of I/O primitives any persistence path performs. There is no further filesystem operation in atomic_write or the lock helpers left unprobed.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@iamwhatever
iamwhatever merged commit ea60666 into kirodotdev:main Aug 9, 2026
49 checks passed
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 9, 2026
@rubencu
rubencu deleted the fix/gateway-persistence-preflight branch August 9, 2026 19:09
bolichen97 added a commit that referenced this pull request Aug 9, 2026
)

The 0.2.0 section was written in #2305, the commit that became v0.2.0-rc.4.
Seventy-one commits have landed on main since, nineteen of them feat:, and
the section was never revisited. It therefore both omitted shipped features
and described one that no longer exists as written.

The wrong entry mattered most: the Webhooks bullet told the reader to manage
inbound automation "from Settings", but #2343 moved that page behind a
per-device Preview pages toggle under Developer and hides it by default. A
0.2.0 user following the release notes would have gone looking for a page
that is not there.

Added, all from the rc.6 range: opt-in Slack setup and the multi-channel
repositioning (#2340), Telegram multi-account (#2203) and inbound
attachments (#2201), sub-agent completions reaching non-Slack parents
(#2352), Discord reply continuation (#2326), Slack OPTIONS as a control
(#1467), the Agent Templates two-pane inspector, project-local agent
discovery (#2167), send-a-copy-to-another-instance, Jira and setting link
chips (#2019, #1907), CJK emphasis rendering, the MCP Apps switch (#2293,
#2337), the Connections provider registry (#2285), GitHub Enterprise
Server support in Code Review Sage (#2154), operator notes on user deny
patterns (#2341), the locked git-publish floor rules (#2369), the
persist-or-refuse boot guard (#2279), and the turn-ceiling bounds on the
approval and stall windows (#2372, #2373).

Scope is exactly 5fe4bd5..ab20b4e, the range v0.2.0-rc.6 ships. The
three commits main carries beyond rc.6, including meeting deletion (#2268),
belong to the next release and are deliberately not described here.
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…irodotdev#2279)

A gateway whose environment breaks basic file syscalls still binds its
port and serves traffic, but every save silently fails: chat-slot
history writes, cron-history appends, and session-PID tracking all need
mkstemp + an advisory lock in the data home. Observed on Linux when a
gateway is spawned from inside a sandboxed agent session: the sandbox's
seccomp filter survives fork/exec (nohup included), so fcntl.flock and
tempfile.mkstemp raise OSError(ENOSYS) while writes to already-open fds
keep working. The process limps along for its whole lifetime dropping
chat history on the floor, and the shutdown save of every slot fails.
A second gateway started from the same environment crashes with a raw
traceback in cleanup_orphaned_sessions instead of a useful error.

Add probe_file_persistence() to platform_compat (mkstemp + file_lock in
the data home, probe file removed in all outcomes) and run it at the top
of GatewayOrchestrator.run(), before the first lock consumer. On failure
the gateway logs CRITICAL, prints an actionable one-liner (naming the
inherited-seccomp cause when errno is ENOSYS), and exits 1 -- loud and
early instead of silent data loss.
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…rodotdev#2412)

The 0.2.0 section was written in kirodotdev#2305, the commit that became v0.2.0-rc.4.
Seventy-one commits have landed on main since, nineteen of them feat:, and
the section was never revisited. It therefore both omitted shipped features
and described one that no longer exists as written.

The wrong entry mattered most: the Webhooks bullet told the reader to manage
inbound automation "from Settings", but kirodotdev#2343 moved that page behind a
per-device Preview pages toggle under Developer and hides it by default. A
0.2.0 user following the release notes would have gone looking for a page
that is not there.

Added, all from the rc.6 range: opt-in Slack setup and the multi-channel
repositioning (kirodotdev#2340), Telegram multi-account (kirodotdev#2203) and inbound
attachments (kirodotdev#2201), sub-agent completions reaching non-Slack parents
(kirodotdev#2352), Discord reply continuation (kirodotdev#2326), Slack OPTIONS as a control
(kirodotdev#1467), the Agent Templates two-pane inspector, project-local agent
discovery (kirodotdev#2167), send-a-copy-to-another-instance, Jira and setting link
chips (kirodotdev#2019, kirodotdev#1907), CJK emphasis rendering, the MCP Apps switch (kirodotdev#2293,
kirodotdev#2337), the Connections provider registry (kirodotdev#2285), GitHub Enterprise
Server support in Code Review Sage (kirodotdev#2154), operator notes on user deny
patterns (kirodotdev#2341), the locked git-publish floor rules (kirodotdev#2369), the
persist-or-refuse boot guard (kirodotdev#2279), and the turn-ceiling bounds on the
approval and stall windows (kirodotdev#2372, kirodotdev#2373).

Scope is exactly 5fe4bd5..ab20b4e, the range v0.2.0-rc.6 ships. The
three commits main carries beyond rc.6, including meeting deletion (kirodotdev#2268),
belong to the next release and are deliberately not described here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants