{
  "status": "DONE_WITH_CONCERNS",
  "review_mode": "Static read-only review. No files were edited and no commands or tests were run, per assignment.",
  "findings": [
    {
      "severity": "P1",
      "confidence": "10/10",
      "path": "tools/omp-spt/src/launch_omp.rs:146-185",
      "symbol": "resolve_omp_from",
      "issue": "The resolver validates only the PATH candidate. An existing OMP_SPT_OMP_BIN override returns at lines 158-166 and a file in a known install location returns at lines 169-170 without running the Oh My Pi --version probe. The tagged test at tools/omp-spt/src/launch_omp.rs:369-424 codifies this bug by asserting probes == 0 for both cases. An unrelated executable placed at a canonical location, or explicitly configured by mistake, can therefore become the broker PTY owner despite REQ-OMP-EXECUTABLE-RESOLUTION saying every path is validated and collisions are rejected.",
      "fix": "Run probe_omp_binary for every candidate selected by precedence, not only PATH. An invalid explicit override should fail immediately; invalid known-location candidates should be rejected deterministically before considering another candidate. Add collision tests for both OMP_SPT_OMP_BIN and a known install path, and check the minimum OMP version required by the extension/CLI surface rather than accepting every omp/<digits> signature."
    },
    {
      "severity": "P1",
      "confidence": "10/10",
      "path": "adapter/strings/omp-spt.mjs:433-441; tests/omp-extension.mjs:216-264; traceable-reqs.toml:61-64",
      "symbol": "ompSpt session immutability handlers / testLifecycleCustodyAndContext",
      "issue": "The extension blocks only session_before_switch. OMP exposes a separate cancelable session_before_branch event, and the accepted hazard explicitly says branches must be blocked. A branch can mint a different session/transcript under the same SPT endpoint identity. The unit evidence tag falsely passes because testLifecycleCustodyAndContext checks only new and resume and asserts that no branch handler is registered.",
      "fix": "Register the same canceling guard for session_before_branch, and add unit cases for branch plus every session_before_switch reason (new, resume, fork, handoff). Keep tree navigation allowed only if it is verified not to change the OMP session ID. Public OMP event source: https://github.com/can1357/oh-my-pi/blob/main/docs/extensions.md and https://github.com/can1357/oh-my-pi/blob/main/packages/coding-agent/src/extensibility/shared-events.ts"
    },
    {
      "severity": "P1",
      "confidence": "10/10",
      "path": "adapter/strings/omp-spt.mjs:195-216",
      "symbol": "settleItem",
      "issue": "A failed spt send is caught, logged, and then item.settled is set true in finally. The accepted delivery is permanently discarded without a correlated reply or explicit failure reaching the sender, directly violating REQ-OMP-EXTENSION-CUSTODY and ADR-0010. Existing tests cover OMP submission rejection but never an outbound send failure.",
      "fix": "Do not mark custody settled when the outcome command fails. Give outcome delivery a finite retry policy; on exhaustion, fail the endpoint closed while retaining the item as unresolved until teardown has made the failure explicit. Add a test where the first/terminal send attempts reject and assert that the item is retried or the endpoint closes, never silently advances as settled."
    },
    {
      "severity": "P1",
      "confidence": "9/10",
      "path": "adapter/strings/omp-spt.mjs:172-180, 413-430, 450-458, 482-493",
      "symbol": "setState / session_start / session_shutdown",
      "issue": "Initialization is not serialized with lifecycle events. setState silently returns before bind supplies a token. If agent_start occurs while bind is pending, busy is lost and session_start later advertises idle even though the agent is active. If session_shutdown occurs while bind is pending, endSession returns because token is absent; bind can then complete after shutdown and no session-end is ever sent, leaving a stale bound endpoint. The current fake harness always awaits session_start to completion, so neither interleaving is tested.",
      "fix": "Track a single initialization promise and a desired state independent of token availability. After bind, publish agentActive ? busy : idle only if still running; if stopping became true, immediately session-end and never start the listener or restore status. Make shutdown await/coordinate the in-flight bind. Add deferred-bind tests for agent_start-before-bind and shutdown-before-bind."
    },
    {
      "severity": "P2",
      "confidence": "9/10",
      "path": "adapter/strings/omp-spt.mjs:183-192, 247-263, 482-493",
      "symbol": "endSession / failClosed",
      "issue": "endPromise permanently caches a rejected session-end attempt. In failClosed, a transient session-end failure is caught and shutdown is requested; the subsequent session_shutdown handler receives the same already-rejected promise instead of retrying, so explicit teardown can never recover.",
      "fix": "Cache only an in-flight teardown attempt and clear the cache on rejection, or implement a bounded teardown retry shared by failClosed and session_shutdown. Add a test where the first session-end fails and the shutdown path performs a second attempt before giving up loudly."
    },
    {
      "severity": "P2",
      "confidence": "10/10",
      "path": "tools/omp-spt/src/echo_commune_omp.rs:29-37",
      "symbol": "bounded_tail",
      "issue": "The byte cap is used directly as a Rust string index. If history.len() - cap lands inside a multibyte UTF-8 character, history[start..] panics. Real OMP transcripts can contain non-ASCII user/model text, so echo-commune can crash instead of producing or reporting a bounded delta. The current tests use ASCII only.",
      "fix": "Search for the next newline in history.as_bytes()[start..], then slice at the byte after that ASCII newline, or advance start to the next char boundary before slicing. Add a regression test whose cutoff splits a 2-4-byte code point."
    },
    {
      "severity": "P2",
      "confidence": "8/10",
      "path": "adapter/strings/omp-spt.mjs:333-353, 387-393",
      "symbol": "handleListenerDeath / startListener stdout handler",
      "issue": "The restart budget resets only after a message event is parsed. A listener can run successfully for hours while idle, then a later unrelated disconnect consumes the next lifetime attempt; a quiet endpoint eventually fails closed after a few widely separated transient disconnects. This is not a consecutive-failure budget.",
      "fix": "Reset the attempt counter after a restarted listener demonstrates stability, for example after a bounded healthy-duration timer or an explicit ready/heartbeat signal, while retaining the current counter for immediate crash loops. Add a fake-clock test for long stable idle periods between deaths."
    },
    {
      "severity": "P2",
      "confidence": "10/10",
      "path": "traceable-reqs.toml; tools/omp-spt/src/echo_commune_omp.rs:201-217; tests/manifest-shortcut.sh:77-91; tools/omp-spt/src/digest_omp.rs:480-598,600-678; tools/omp-spt/tests/launch_omp.rs:103-156",
      "symbol": "Traceable requirement evidence placement",
      "issue": "Several stage tags satisfy the scanner without proving their requirement titles. REQ-SESSION-ECHO-COMMUNE's only unit tag is on an argument-parser test, not bounded summarization, locate-miss behavior, or real OMP failure reporting. REQ-PSYCHE-EPHEMERAL-SHIM's only unit tag is on manifest env/key assertions, while the actual turn/reseed tests are untagged and no spawned-turn behavior is exercised. REQ-DIST-DIGEST-EXTRACTOR tags only path/locator tests; the actual JSONL-to-record mapping tests are untagged. REQ-OMP-NATIVE-TUI's int tags run fake executables that only capture argv/exit status, so they do not prove native OMP owns the broker PTY or that the packaged extension supplies lifecycle behavior as the title and ADR-0013 require.",
      "fix": "Move/add stage tags to tests whose assertions directly cover each title, and add missing fake-process behavioral tests for Psyche/echo-commune. Put the REQ-OMP-NATIVE-TUI int tag on the real native endpoint release acceptance gate required by ADR-0013, or split the broad title into separately provable launch-argv and live-endpoint requirements. Do not count a nearby tag as evidence for unrelated assertions."
    },
    {
      "severity": "P2",
      "confidence": "10/10",
      "path": "tests/manifest-schema.sh:1-28; tests/adapter-archive.sh:1-33",
      "symbol": "Tagged unit gates",
      "issue": "Both tagged proof tests can report success without executing the behavior named by the requirement. manifest-schema.sh exits 0 with 'OK (skipped)' when Python/jsonschema is unavailable. adapter-archive.sh exits with rc=0 after skipping archive construction whenever either platform binary is missing. This lets REQ-DIST-MANIFEST-SCHEMA and REQ-DIST-ADAPTER-RELEASE remain green without validating the manifest or building/inspecting a multi-platform adapter.spt.",
      "fix": "Make the tagged CI proof fail when prerequisites are unavailable, with CI explicitly installing them/building both targets. If local convenience skips are desired, keep them in an untagged wrapper. For the archive unit stage, use deterministic fixture binaries so archive layout is always exercised; leave real cross-platform acquisition to the integration stage."
    },
    {
      "severity": "P3",
      "confidence": "10/10",
      "path": "docs/adr/0010-native-delivery-self-heals-or-closes.md:7",
      "symbol": "Accepted ADR product wording",
      "issue": "This active accepted ADR teaches the OMP contract as mirroring claude-spt. That retains a foreign-harness product surface in an active OMP-native decision instead of describing the native contract directly, contrary to the cutover boundary.",
      "fix": "Replace the lineage sentence with an OMP-native statement such as 'Normal peer delivery uses the message-stub contract: ...'. Keep any claude-spt provenance only in explicitly historical/superseded text."
    }
  ],
  "checks_without_findings": [
    "No missing helper command dispatch was found: tools/omp-spt/src/main.rs classifies and dispatches digest-omp, echo-commune-omp, history-omp, psyche-omp, and launch-omp.",
    "The currently used OMP event names, context return shape, sendUserMessage call shape, and native CLI flags match the current public OMP docs/source; the substantive public-API gap is the unhandled separate branch event.",
    "No static Rust or JavaScript compile blocker was found in the reviewed files."
  ]
}