[luvs01]

Summary

Part of #7771, diagnostics A only. A guarded stop currently reports the same generic message when its tracked PID or runtime record is absent, unreadable, malformed, or inconsistent.

Add bounded, fixed refusal reasons to that existing message. An unreadable command line remains unverifiable; it is not treated as proof of elevation, another account, or denied access. Raw paths, command lines, stderr and tokens do not enter the public detail.

The new inspection and existing readPid/verifyPidIdentity paths share the same cached identity observation. Positive and negative cache hits, uncached unavailable results, sweep behavior, probe order and probe counts are preserved. Stop authorization, approval requirements, execution, timeouts and service-manager behavior are unchanged. This does not implement service-stop B or close #7771.

Verification

Windows x64, pinned Bun 1.4.4; base 3ea89653f5727aea41948ddbd5e833f74afe3e36:

• Final process-state.test.ts + cli-stop-json.test.ts: 58 passed, including cached-positive, cached-negative and unavailable-probe recovery parity.
• Final cli-stop-refusal-transport.test.ts: 2 passed.
• Adjacent stop suites previously passed 137 tests on the earlier candidate. The affected cache and refusal tests above were rerun after the cache correction.
• Direct bun check --noEmit: 2,244 files, zero errors; bun x tsc --noEmit passed. Privacy, structure, file-size/layout (11 tests) and git diff HEAD --check passed.
• Independent review found an extra-probe/cache admission regression in the first candidate. It was corrected, reproduced with three failing parity cases before the correction, and re-reviewed with no remaining blocking finding.

Exact-head Cross-platform CI passed: 46,775 tests passed, zero failures in the affected-test run. The same connected-stop listener case below passed on Linux in 113.56 ms. This does not resolve its Windows-specific observation.

Open validation limits

The unchanged connected-stop-capability-auth.test.ts case “the proxy admits and proves one stop read, refusing replay and altered scope” hits its existing 5,000 ms test timeout on both the pristine base and the candidate (15 pass / 1 fail), followed by a connection/teardown error. The cause is now traced to a pre-existing Windows credential-observer feedback loop: its own metadata writes emit ancestor-directory notifications and repeat synchronous ACL work. The independent fix in #7976 passed all 16 cases with the same 5,000 ms deadline, including this case at 2,178 ms. That evidence belongs to the separate fix branch, not to this unchanged diagnostics head; neither the timeout nor the assertion was relaxed.

The separate Windows integration checkout combined this head with #7976 commit ea3fdd682cabadc32cf8c16e29c725c675f7f11d (tree 9c367ac0a2ff58bf6d1d36ee333cb70e66eb7bcc): 98 passed / 1 existing skip / 0 failed, 419 assertions. The previously timing-out case completed in 2,219 ms. This disposes of the baseline failure as an independent observer defect and verifies the interaction; it does not make the observer fix part of this unchanged diagnostics head. The performance remediation remains in #7976.

The broader changed-test run reached the caller's 115-second bound and did not complete. It is not counted as a pass. Full local-suite execution is deferred because of disk/resource contention; exact-head hosted CI has since passed; the independent observer fix in #7976 has also passed its exact-head CI. No installed Desktop, elevated account, live service stop or permission change was exercised.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Structure documentation describes the bounded refusal and shared cache contracts.
• Exact-head hosted CI passed.
• The Windows baseline failure is diagnosed, fixed separately and checked in combination.
• Security-sensitive changes have final maintainer acceptance.

Summary by CodeRabbit

• Bug Fixes

  • Guarded stops now verify the tracked runtime before proceeding and report a specific reason when verification fails, such as a missing record or process identity mismatch.
  • Stop approvals are checked against verified runtime details before the stop proceeds.

• Documentation

  • Updated the guarded-stop documentation to explain verification outcomes and the reasons shown when approval changes. Refusal details avoid exposing file paths or sensitive process information.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: lidge-jun/opencodex
• Branch: dev
• File: .coderabbit.yaml
• Commit: 3ea8965
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 912799a9-bd12-493a-abe0-cc725a7bb110

Commits

Reviewing files that changed from the base of the PR and between 561544e and afd794d.

Files selected for processing: 3

• src/config/tracked-runtime-inspection.ts
• tests/cli/cli-stop-json.test.ts
• tests/service/process-state.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

Guarded stop now rechecks tracked runtime records and process identity. It returns a verified target or a fixed refusal cause. The CLI passes that result to the approval flow, which reports verification failures as approval-changed.

Changes: Guarded Stop Runtime Verification

Layer / File(s)	Summary
Process identity observation src/config/process-state.ts, tests/service/process-state.test.ts	process-state.ts exports a tri-state identity check. Readable matches and mismatches are cached; unreadable command lines return unverifiable without caching. Tests cover shared cached results and retries after failed probes.
Tracked record inspection src/config/tracked-runtime-inspection.ts, structure/config.md, structure/runtime.md, tests/service/process-state.test.ts	The probe checks the PID record, process liveness and identity, then the runtime record and PID match. It returns a verified target or a fixed refusal explanation. Tests cover refusal outcomes and verify that explanations omit observed error text.
Guarded-stop result handling src/cli/stop-approval.ts, src/cli/index.ts, tests/cli/cli-stop-json.test.ts	runApprovedStop accepts an untracked cause and returns approval-changed with that cause. The CLI uses describeTrackedRuntimeTarget; tests cover absent and mismatched PID records.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant GuardedStopCLI
  participant describeTrackedRuntimeTarget
  participant inspectTrackedRuntimeRecord
  participant processState
  participant runApprovedStop
  GuardedStopCLI->>describeTrackedRuntimeTarget: request tracked target
  describeTrackedRuntimeTarget->>inspectTrackedRuntimeRecord: inspect tracked records
  inspectTrackedRuntimeRecord->>processState: check liveness and identity
  inspectTrackedRuntimeRecord-->>describeTrackedRuntimeTarget: verified target or refusal
  describeTrackedRuntimeTarget-->>GuardedStopCLI: target or fixed refusal cause
  GuardedStopCLI->>runApprovedStop: provide tracked-runtime check
  runApprovedStop-->>GuardedStopCLI: approval-changed or continue target checks

Loading

Possibly related issues

• lidge-jun/opencodex#7771 — The PR adds fixed reasons when tracked-runtime verification fails, but does not implement the issue’s separate service-stop or port-release behavior.

Merge Risk: Minimal · up to afd79

No actionable merge-blocking risk is established in the reviewed changes.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		Direct issue #7771 has two relevant coding objectives. [#7771] The refusal must provide a concrete recovery step or attach to the healthy runtime. src/config/tracked-runtime-inspection.ts adds fixed…	Implement one accepted recovery path for [#7771]. Either attach to the healthy runtime, or add a fixed, path-free next-step message that tells the user how to stop the owning service and relaunch. Add automated assertions for the message. A…
Docstring Coverage		Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "fix(cli): explain tracked runtime stop refusals" matches the main change. The PR adds a read-only tracked-runtime inspection probe (src/config/tracked-runtime-inspection.ts) that returns bo…
Out of Scope Changes check		The changed files stay within the diagnostic refusal path for [#7771]. src/config/tracked-runtime-inspection.ts classifies tracked PID and runtime-record failures and maps them to fixed public cause…

Full details: Title check

Explanation

The title "fix(cli): explain tracked runtime stop refusals" matches the main change. The PR adds a read-only tracked-runtime inspection probe (src/config/tracked-runtime-inspection.ts) that returns bounded refusal reasons when a guarded stop cannot verify its tracked PID or runtime record. The stop flow in src/cli/stop-approval.ts surfaces these reasons as approval-changed details. The title is concise, uses the conventional commit form, and avoids file lists. It does not mention that service-stop behavior (issue #7771, part B) is out of scope, but the title does not need to cover every detail.

Full details: Out of Scope Changes check

Explanation

The changed files stay within the diagnostic refusal path for [#7771]. src/config/tracked-runtime-inspection.ts classifies tracked PID and runtime-record failures and maps them to fixed public causes. src/config/process-state.ts exposes the shared identity observation. src/cli/index.ts uses the inspection, and src/cli/stop-approval.ts carries the cause without changing stop execution. The structure documentation and tests cover these changes. No changed file adds unrelated cleanup, service-manager redesign, credential-observer work, or the separate #7976 fix. The missing service-stop behavior is an unmet linked-issue objective, not an out-of-scope change.

Full details: Linked Issues check

Explanation

Direct issue #7771 has two relevant coding objectives. [#7771] The refusal must provide a concrete recovery step or attach to the healthy runtime. src/config/tracked-runtime-inspection.ts adds fixed cause text such as the recorded PID file is absent and the recorded process identity could not be verified. src/cli/stop-approval.ts appends that cause to approvalChanged. The detail still ends with no stop was attempted. It does not provide a recovery step or an attach path. [#7771] ocx stop and ocx service stop must free the port when the service owns the runtime. The reviewed changes add no service-stop, wrapper-teardown, or port-release implementation. The changed runApprovedStop code only reports the refusal. The PR description also states that service-stop B is not implemented. The new diagnostic behavior is present, but both linked coding objectives remain incomplete.

Resolution

Implement one accepted recovery path for [#7771]. Either attach to the healthy runtime, or add a fixed, path-free next-step message that tells the user how to stop the owning service and relaunch. Add automated assertions for the message. Also implement and test the required service-owned behavior for both ocx stop and ocx service stop, including wrapper termination and port release. If service-stop B remains in another pull request, do not treat this pull request as satisfying that [#7771] requirement.

Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 6 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

Maintainer integration into dev (MAINTAINERS.md exception), exact head afd794d0e33dc517ab1607aaf5117ea5f41bdd79.

Review. A fresh independent review of 561544e came back NEAR-PASS with one minor finding: the identity-mismatch detail claimed the PID now belonged to a different process, which is more than the cached probe proves. afd794d changes that detail to "the recorded process does not match the expected start command" and updates the assertions. A fresh pre-merge review of afd794d: PASS.

Verification. On Windows x64 (Bun 1.4.4), process-state, cli-stop-json and cli-stop-refusal-transport passed 60 / 0 fail, and typecheck passed. On dev 4fc870074 merged with this head and #7970, typecheck, layout/tooling/ratchet (38 pass) and structure:check passed.

Exact-head CI. Cross-platform run 41047128445 on afd794d completed success (gates, test changed, structure gate and aggregate ci all succeeded). Service lifecycle linux-systemd succeeded.

Scope. This is #7771 diagnostics A only. Service-stop behavior B is not implemented, so #7771 stays open.

Not run: the full local suite and native Windows CI jobs, which are policy-skipped.