Community review: deepseek-harness @ master (v0.1.0-rc.5) — findings & suggestions

[ardi-zanki]

Superseded — this review was consolidated and reposted as a shorter version here: #2450

[argszero]

Excellent review — precise file:line references throughout. I've spent the last several days in this codebase tracking a set of failure families through the community discussions, and several of your findings are the theoretical versions of bugs we now have field evidence for. Adding that layer, plus the priorities it implies:

1. Item 4 (writeFileAtomic no fsync) is the durability asymmetry at the heart of the session-corruption family

The JSONL session backend fsyncs every append (session-persistence-jsonl), while settings/credentials writes don't (verified: atomic-write/src/index.ts:44-54, TODO(settings-atomic-durability)). We now have 7 field-confirmed session-log corruption mechanisms (#1333/#1452 cross-process seq, #1497 torn tail, #1473 read race, #1586 recovery-vs-legacy writer, #2167 stale-view re-append, #2342 repair-injection-vs-live-writer) — every one of them a crash-durability or ownership-contract gap. Your item 4 is the settings-side half of the same theme: an unfsynced rename can lose the entire settings document on power loss, which is strictly worse than a torn JSONL tail (recoverable). This deserves promotion from MEDIUM to HIGH, and the fix (fsync file + parent dir, opt-in, enabled by settings-file + credentials-local) is small and PR-ready. wellorbetter's writer-lease (#1550) and zoahdev's repair-path-liveness fix (#2342) are already landing in the session half; the settings half is untouched.

2. Item 1 (redactSecrets fails open) is the second settings-seam security gap this week

#1688 (proto prototype pollution in cloneJsonShaped/mergeLayers) was confirmed with a fork fix branch; your item 1 is the sibling: a secret buried in a union/intersection/transform schema leaks verbatim (redact.ts:86-91, TODO(settings-wire-redaction)). Both are "the settings seam's guarantees don't hold for a corner of the schema space". The fail-closed fix the TODO proposes (reject such schemas at registration) is the right call — same detect-and-refuse philosophy as the session-side seq assertion.

3. Item 3 (settings-file permission check) closes a credential-family gap

credentials-local has assertOwnerOnly (:103, used :425/:468); settings-file load (:168-184) has none — yet settings documents can carry role('secret') fields. This is the settings-domain sibling of #1559 (project .env silently replacing DEEPSEEK_API_KEY): both are "a lower-trust layer can supply credential material without the checks the canonical layer enforces". Extracting assertOwnerOnly into a shared util and applying it in settings-file load is a clean, small PR.

4. Items 2/10 (stop hooks) and 8 (subagent tool-name) have live community surface

• Item 2 (stop_hook_active: false + no cap): there's a TODO(stop-loop-guard) in both bridges; a forced-continuation counter with a cap is the right shape.
• Item 8 (one-shot subagent tool-name collision): tool-subagent/src/index.ts:435, TODO(subagent-dup-toolname) — the intent registry is the correct fix; this is the tool-subagent plugin that also surfaced the agentOptions drop (tool-subagent 静默丢弃子代理路由覆盖（每调用 agentOptions）+ 两个面向仓库外（下游）插件的扩展点缺口 Subagent route override is silently dropped by tool-subagent (per-call agentOptions), plus two extension gaps for out-of-repo (downstream) plugins #1619).
• Item 30 (systemMessage parsed but never surfaced) and item 31 (static model config) are hooks-bridge gaps that the tool-subagent 静默丢弃子代理路由覆盖（每调用 agentOptions）+ 两个面向仓库外（下游）插件的扩展点缺口 Subagent route override is silently dropped by tool-subagent (per-call agentOptions), plus two extension gaps for out-of-repo (downstream) plugins #1619 "downstream plugin surface" workstream (event-type registration, settings-namespace exposure feat(settings): let plugin-registered namespaces opt into the configuration-client exposure boundary #1606, agentOptions forwarding) would naturally absorb.

5. Item 13 (landlock-run repo URL) is a 2-line release-blocker

native/landlock-run/packages/{entry,linux-x64,linux-arm64}/package.json → deepseek-harness → deepseek-ai. With the npm-public release (#2519) already out, every user clicking "Repository" on these three packages hits a 404 today.

6. Item 18 (AGENTS.md drift) — worth doing before the first tagged release

The root AGENTS.md layout is ~16 groups behind; packages/README.md is accurate. Linking AGENTS.md to packages/README.md as the single source is the durable fix — otherwise this drifts again at the next rename.

Priority suggestion from field data: 4 (durability asymmetry) and 1 (redact fail-open) are the two with real-user impact already demonstrated in the family threads; 3 and 13 are cheap wins. Items 2/8/10/30/31 cluster naturally into the hooks/subagent "downstream surface" workstream. Thanks for the review — it maps cleanly onto the families the community has been hitting, which is a strong signal the TODO markers are pointing at real gaps.