Skip to content

fix(daemon): isolate agent/test processes from engine DB and self-heal ownership - #10

Open
ranxianglei wants to merge 10 commits into
masterfrom
2026-09-17_db-env-isolation-claim-heal
Open

ranxianglei wants to merge 10 commits into
masterfrom
2026-09-17_db-env-isolation-claim-heal

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #7

Changes (the four spec items)

  1. Runtime isolation - runtime/env-deny.ts extends ENV_DENY_ALWAYS with DAEMON_DB_PATH, WORK_DB_PATH, OPENCODE_DB_PATH, DAEMON_ENV, shared by the opencode and pi backends. Spawned agent processes (and any test run they launch) can no longer open the engine DB through inherited environment. Test spawns a fake binary that dumps its own env and asserts the keys are absent.
  2. Test self-isolation - tests/setup.ts scrubs the four vars plus WORK_DB_PREFIX and pins WORK_DB_PATH to a unique file under os.tmpdir() (pid + uuid; pid alone collides under bun pid recycling across parallel files). A root-level bunfig.toml wires the same preload so bare bun test from the repo root cannot reach production or default DB paths. A child-process regression sim proves an inherited DAEMON_DB_PATH cannot redirect the test DB.
  3. claimIssue self-heal - ensureOwned re-creates a vanished issue row before claiming (a recreated row gets a fresh local uid, so downstream references use the returned row) and, on foreign-key failure, re-registers the daemon identity via healDaemonRow then retries the claim once. New Store.releaseDanglingOwners() clears owners pointing at deleted daemons rows - a case releaseDeadOwners structurally cannot see - wired at boot and in the observer cycle.
  4. web->daemon reconciler - the observer cycle queries the gitea shim issues search endpoint hourly (when web reachable or owning zero issues) for open issues in tracked scopes (distinct scopes of active issues + new work.reconcileScopes / WORK_RECONCILE_SCOPES) and backfills missing rows as active without spawning work. Automates the manual backfill done during the bc#853 incident.

Verification

  • tsc --noEmit: clean
  • 23 new tests, green in every invocation context (package dir, repo root, hostile-env child process)
  • Full daemon suite: 228/258 pass; the 30 failures are identical to the master baseline in this sandbox (pre-existing environmental only: hardcoded /tmp paths under a read-only /tmp mount, loopback clone blocked by egress proxy) - zero regressions

Findings (reported in #7, no code changes here)

  • ~8 existing test harnesses hardcode /tmp/... paths -> EACCES wherever /tmp is read-only
  • Cross-package env collision: web and daemon both resolve their DB from WORK_DB_PATH with different schemas; a root-level bun test mixes both packages in one process env (nested bunfig files are ignored) - pre-existing landmine, out of scope here

ework-agent added 5 commits September 17, 2026 11:31
ENV_DENY_ALWAYS gains DAEMON_DB_PATH, WORK_DB_PATH, OPENCODE_DB_PATH and DAEMON_ENV, shared by the opencode and pi backends via runtime/env-deny.ts. Agent-spawned processes (and any test run they launch) can no longer open the engine database through inherited environment. #7 item 1.
tests/setup.ts now scrubs DAEMON_DB_PATH, WORK_DB_PATH, OPENCODE_DB_PATH, DAEMON_ENV and WORK_DB_PREFIX and pins WORK_DB_PATH to a unique file under os.tmpdir() (pid alone collides under bun pid recycling across parallel files). A root-level bunfig.toml wires the same preload so bare bun test from the repo root can no longer reach production or default DB paths. db.ts exports RESOLVED_DB_PATH for assertions; a child-process regression sim proves an inherited DAEMON_DB_PATH cannot redirect the test DB. #7 item 2.
WORK_RECONCILE_SCOPES comma-separated list, defaulting to empty; existing test Config literals gain the new required property. Prep for the web reconciler.
ensureOwned now re-creates a vanished issue row before claiming (a recreated row gets a fresh local uid, so all downstream references use the returned row) and, when the claim hits a foreign-key failure, re-registers the daemon identity via healDaemonRow and retries the claim once. Store gains releaseDanglingOwners() for issues pointing at deleted daemons rows - a case releaseDeadOwners can never see because its WHERE clause requires the daemons row to exist - wired at boot and in the observer cycle. Tests cover FK heal, row recreation, dangling-owner release and the lost-claim race. #7 item 3.
runObserverCycle hourly (when the web endpoint is reachable or the engine owns no issues) queries the gitea shim issues search endpoint for open issues in tracked scopes - distinct scopes of active issues plus work.reconcileScopes - and backfills missing rows as active without spawning work. Automates the manual backfill performed during the bc#853 incident. #7 item 4.
…l open issues

The web shim's issue search endpoint only supported a limit (capped at 200) with no offset or page parameter, so the hourly web->daemon reconciler could only ever see the most recently updated open issues up to its limit. On instances holding more open issues than that cap, missing rows beyond it would never be restored by the self-heal path added in this PR.

- store.ts: listAllIssues accepts an optional offset (clamped >= 0) and appends OFFSET to its SQL; listIssues is unchanged.

- giteaApi.ts: the search handler accepts a 1-based page parameter (default 1) translated to an offset; requests without page behave exactly as before.

- web-reconcile.ts: default limit raised 100 -> 200; walks pages until a short page, dedupes across shifting pages via a seen set, stops when a stale shim repeats the same page, and caps the walk at maxPages (default 25).

- Tests: multi-page walk, stale-shim repeat guard, maxPages cap, and store-level offset slicing/clamping.
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework

[bot] 🏷 Review complete — found and fixed one real defect, pushed to this branch (d4992f4).

Verdict on the four spec items: all implemented correctly. Verified line-by-line against master: shared ENV_DENY_ALWAYS extension covers both opencode and pi backends; test preload scrub+pin uses pid+uuid (correct — pid alone collides under bun pid recycling); ensureOwned recreate-then-claim returns the fresh row so downstream references stay consistent, and FK-failure retry goes through healDaemonRow exactly once; releaseDanglingOwners does catch what releaseDeadOwners structurally cannot (owners pointing at deleted daemon rows). isForeignKeyError matches both SQLite message and errno 1452. Diff is clean — every hunk serves one of the four items. CI: all three check runs green (smoke, type-check+unit, Docker E2E); mergeability clean.

The defect (now fixed): the web→daemon reconciler made a single search call with limit=100, but the shim's search endpoint only ever supported limit (capped at 200) — no offset or page parameter. So each hourly run could only see the 100 most-recently-updated open issues. I measured the live endpoint: at limit=200 it returned exactly 200 items, i.e. this instance holds ≥200 open issues while the incident wiped ~250 rows — anything beyond the cap would never have been restored, defeating item 4's purpose precisely where it matters most.

Fix (commit d4992f4, additive/back-compatible):

  • packages/web/src/store.ts — listAllIssues accepts an optional offset (clamped ≥ 0) and appends OFFSET; listIssues untouched.
  • packages/web/src/giteaApi.ts — search handler accepts a 1-based page param (default 1); requests without it behave exactly as before.
  • packages/daemon/src/sync/web-reconcile.ts — default limit raised 100→200; walks pages until a short page; seen-set dedup across shifting updated_at ordering; stops if a stale shim repeats the same page (prevents infinite loop against old deploys); capped at maxPages (default 25).
  • Tests on both sides: multi-page walk, stale-shim repeat guard, maxPages cap, store-level slicing/clamping.

Verification:

  • tsc --noEmit clean in web, daemon, and aio.
  • Daemon reconciliation + isolation/heal suites: 26/26 pass. Store-level pagination additionally exercised directly against a scratch DB (slicing, beyond-range, negative/NaN clamping, handler page end-to-end): 8/8 ok.
  • Full daemon suite: 231/261 pass — the 30 failures are byte-for-byte the same class as the master baseline in this sandbox (hardcoded /tmp paths under a read-only /tmp mount, loopback clone blocked by egress proxy). Zero regressions.
  • Web suite: identical failure count with and without my changes (verified by stashing); every failure traces to the pre-existing test preload hardcoding its DB path under /tmp — one of the ~8 harnesses your Findings section already lists as out-of-scope. Docker CI is unaffected.

I did not merge — that stays with you. Once you're happy, please merge via GitHub.

中文摘要:审查确认四项修复均正确实现且 CI 全绿;发现并直接修掉一个真实缺陷——web 侧 issue 搜索接口不支持分页(仅 limit≤200),导致每小时对账最多只能看到最近更新的 100 条 open issue,超出上限的丢失行永远无法自愈(实测该实例 open issue 已 ≥200 条);已在本分支补上端到端分页(store offset、接口 page 参数、reconciler 翻页+去重+防旧版 shim 死循环+页数上限)并加双侧测试,类型检查与全部测试通过、零回归,可以合并。

ework-agent added 2 commits September 17, 2026 12:34
The five live-server test harnesses spawn the real web server with stdout/stderr ignored, so a failed boot surfaced as a bare 20s healthz timeout with zero diagnostics (csp.test.ts failed this way in CI on three consecutive runs, on commits both before and after this PR's changes). Capture the child's stderr to a per-process scratch file and append its tail to the timeout error; no behavior change otherwise.
tests/setup.ts pinned WORK_DB_PATH to /tmp/ework-test-<pid>.db. Bun recycles pids across parallel test files, so two files can pin the same DB file; when both spawn the real server, one server's boot migration then fails with SQLITE_BUSY inside the other's file and the healthz wait times out with no diagnostics (reproduced in CI: csp.test.ts boot crash at migrateUsersTable). The daemon-side setup in this PR already uses pid+uuid for exactly this reason; bring the web preload to the same guarantee.
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework

[bot] 🏷 CI follow-up — root cause of the red build found and fixed

The re-run confirmed the picture: the Docker image smoke failure was a genuine race (it passed on re-run), but the Type-check + unit tests job kept failing on a pre-existing flake — packages/web/tests/csp.test.ts timing out waiting for its spawned server to boot. It failed identically on commits both before and after this PR's changes.

Two follow-up commits on the branch:

  1. 4370475 — the five live-server test harnesses (csp, wake-logins, dispatch-state, dup-reply-guard, new-issue-upload) spawned the real server with stdout/stderr discarded, so a failed boot was a bare 20s timeout with zero diagnostics. They now capture the child's stderr and append its tail to the timeout error. No behavior change otherwise.
  2. 3e61526 — with diagnostics in place, the real failure showed up: the spawned server crashes at boot with SQLiteError: database is locked (SQLITE_BUSY) in migrateUsersTable. Cause: packages/web/tests/setup.ts pinned WORK_DB_PATH to /tmp/ework-test-<pid>.db. Bun recycles pids across parallel test files, so two files can pin the same DB file; when both spawn a live server, one server's boot migration collides with the other's. That's the exact hazard class this PR's spec item 2 fixes on the daemon side (which is why the daemon preload already uses pid+uuid) — the web preload just never got the same treatment. The fix adds the uuid suffix, one line.

Both commits typecheck clean; my pagination tests were already passing in Docker CI ((pass) listAllIssues: pagination > … ×2). New CI run is in flight — I'll confirm status here when it lands.

中文摘要:定位并修复了 CI 反复失败的根因——web 测试预加载用纯 pid 命名测试库,bun 并行测试间 pid 复用导致两个测试文件共用同一个 SQLite 文件、启动迁移互相锁死(SQLITE_BUSY),已按本 PR daemon 侧同样的 pid+uuid 方案修复,并给 5 个起真实服务的测试补了 stderr 诊断;等新一轮 CI 结果确认后回复。

@ranxianglei

ranxianglei commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 CI green — PR ready for human review/merge

The run on 3e61526 is fully green: all three jobs pass (Type-check + unit tests, Docker image smoke, Docker E2E). Per package: web 202/0 across 21 files (tsc clean), daemon 261/0 across 33 files (tsc clean), aio 19/0. csp.test.ts boots and passes, and both new pagination tests pass (listAllIssues: pagination > … ×2). In Docker CI the full web suite runs (writable /tmp there), so this also covers the files that can't execute in my sandbox.

Final branch shape — original 5 commits plus three follow-ups from review:

  • d4992f4 fix(web,daemon): paginate issue search so reconciliation backfills all open issues (the gap I found: the shim's search endpoint capped at 200 with no page param, so hourly reconciliation could never see past the most-recently-updated 100 issues)
  • 4370475 test(web): surface spawned-server stderr in live-server test timeouts (diagnostics; no behavior change)
  • 3e61526 test(web): make per-file test DB path unique with uuid (the root cause of the recurring red builds)

Findings reported here only (no code changes, per protocol):

  1. The pid-only test DB name in the web preload was the same hazard class this PR fixes on the daemon side — fixed inline because it was blocking this PR's own gate. Other test packages that pin per-pid temp paths may share the class; worth a sweep when convenient.
  2. Benign SQLiteError: database is locked log noise from webhook recordDelivery appears in the live-server suites (parent and child processes share one test DB by design). No test fails on it; it's a pre-existing artifact of that shared-DB harness design and a candidate for a follow-up (busy-timeout or per-process DBs).
  3. The five live-server harnesses still pick ports from overlapping base + pid%200 windows — a low-probability collision risk that remains even after the DB fix. Suggest a dedicated follow-up if you want it eliminated.

PR state: open, mergeable/clean, CI green. I did not merge — that stays with you.

中文摘要:CI 已全部转绿(web 202/0、daemon 261/0、aio 19/0,csp 与分页测试均通过);分支在原有 5 个提交上补了 3 个 review 发现(分页缺口修复、测试 stderr 诊断、web 测试库路径加 uuid 根治反复红 CI);另报告了 3 个遗留观察项(其他包同类 pid 命名隐患、共享测试库的良性锁日志噪音、5 个测试服务器端口窗口重叠),均只报告不改码;PR 可合并,合并动作留给你。

@ranxianglei

Copy link
Copy Markdown
Owner Author

冲突

ework-agent added 2 commits September 17, 2026 13:03
…solation-claim-heal

# Conflicts:
#	packages/daemon/src/config.ts
#	packages/daemon/tests/concurrency.test.ts
#	packages/daemon/tests/exit-chain.test.ts
#	packages/daemon/tests/handle-closed.test.ts
#	packages/daemon/tests/model-pool.test.ts
#	packages/daemon/tests/multi-engine.test.ts
#	packages/daemon/tests/session-capture.test.ts
#	packages/daemon/tests/setup.ts
#	packages/daemon/tests/stranded-interrupted.test.ts
#	packages/web/tests/setup.ts
master added infraRetryMax/infraRetryBaseMs/recoveryReport to the work config; spread cfg.work so the literal stays complete
@ranxianglei

ranxianglei commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 冲突已解决,CI 全绿

master 在评审期间合入了 #6(workdir GC)、#8(pending-replay infra retry)、#11(restart recovery),与 #10 在 10 个文件上冲突。已将 origin/master 并入分支(3d6c9b0)并补了一个合并后的小修(56aaf26:master 给 work config 新增了 infraRetryMax / infraRetryBaseMs / recoveryReport 必填键,claim-heal 测试的字面量配置需同步补全)。

冲突处理方式(全部为两侧语义的并集,无一方被丢弃):

  • packages/web/tests/setup.ts:采用 master 的 tmpdir 迁移 + 我此前的 uuid 后缀 → 每个测试文件独立且不复用的 DB 路径(master 的 pid-only 命名仍有 bun pid 复用的碰撞隐患,uuid 保留)
  • packages/daemon/tests/setup.ts:保留我的 isolateTestDbEnv() 与 master 新增的 WORK_WAKE_LOGINS 清除
  • 7 个 daemon 测试文件的 work 配置统一改为 { ...cfg.work, …显式覆盖, reconcileScopes: [] }——展开继承 master 的新键,同时保持测试不触发对账
  • config.ts:两侧新增键并存(他们的 infra-retry/recovery-report 键 + 我的 reconcileScopes)
  • 无重复实现:master 的 "web reconciliation"(reconcileWebComments)是故障窗口内消息投递的补发,与我对账器做的 issue 行回填是不同层次,互不重叠

验证:

  • tsc 三个包全部干净
  • 本地沙箱:daemon 全套 291/0(此前 30 个环境性失败现已清零);web 184 通过 / 9 失败——与 origin/master 基线逐条相同(均为沙箱环境性:回环上游网络不可达、wake-logins 断言类),零回归
  • GitHub CI @ 56aaf26:Type-check + unit tests ✅ / Docker image smoke ✅ / Docker E2E ✅

PR 当前 mergeable/clean。合并动作留给你。

中文摘要:已把 master(含 #6/#8/#11)并入分支解决全部 10 处冲突,均为两侧语义并集(web 测试库 tmpdir+uuid、daemon 测试配置展开继承新键、config 键集合并),本地与 CI 三件套全绿且相对 master 零回归,PR 可合并。

@ranxianglei

Copy link
Copy Markdown
Owner Author

继续

@ranxianglei

ranxianglei commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Agent 侧工作已全部完成,当前状态如下

刚核实过(56aaf26):

剩下唯一的一步是合并 PR——这一步我不能做:系统规则禁止 agent 合并任何 PR(即使有人指示),合并只能由人来执行。请你自己合并:#10

如果你说的"继续"是指我此前报告的那几个观察项(web 测试服务器端口窗口重叠、共享测试库的 recordDelivery 锁日志噪音、其他包 pid-only 临时路径排查),按平台流程它们应各自开独立 issue 再进入开发——issue 由你们发起,我这边不会在工作会话里自行建 issue。开出来我会接着做。

中文摘要:PR #10 全部开发与验证已完成且 CI 全绿、可干净合并;合并操作受系统规则限制只能由人工执行;后续观察项需另开 issue 才会进入开发。

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.

daemon: agent 测试运行继承生产 DB 路径变量,可整库清空引擎数据库(bc#853 停摆实锤)

1 participant