Skip to content

fix(codex): compare status metadata project as canonical normalized paths (#459) - #511

Open
chemica-tan wants to merge 1 commit into
fujibee:mainfrom
chemica-tan:pr/status-canonical-compare
Open

fix(codex): compare status metadata project as canonical normalized paths (#459)#511
chemica-tan wants to merge 1 commit into
fujibee:mainfrom
chemica-tan:pr/status-canonical-compare

Conversation

@chemica-tan

@chemica-tan chemica-tan commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #459.

Mechanism

The bridge records its project through Node's path.resolve() — on Windows that is the native spelling (C:\work\proj). Callers of delivery.sh status pass the Git Bash spelling (/c/work/proj or C:/work/proj). The verbatim compare in _delivery.sh ([ "$meta_project" != "$project" ]) therefore labels every live bridge stale pidfile (metadata mismatch) on Windows.

Because this compare runs before the liveness check, it also masks the kill -0 blindness reported in #415 (and being worked on in #505): on a vanilla Windows install, status never even reaches the liveness layer. Both layers need fixing for status to be truthful on Windows; this PR is the metadata layer only.

Change

Compare canonical normalized forms instead, reusing the existing helpers (agmsg_canonical_path + agmsg_normalize_project_path from lib/resolve-project.sh) — the same normalization the rest of the tree already uses for project identity. 10 lines, no new dependencies.

Known trade-off inherited from the reused normalizer (documented in lib/resolve-project.sh): the drive reinterpretation is form-based, not platform-gated, so on POSIX two genuinely distinct paths /c/foo and /C/foo can normalize to the same drive-like spelling and would compare equal here. In this call site that can only affect the status label; a platform-gated equality comparator would remove the trade-off if you prefer that.

Write-side normalization (making the bridge record the Git Bash spelling) was considered and rejected: it would not repair meta files written by bridges already in the field, and the read side would still need tolerance for them.

Tests

  • metadata project spelling variant is not a mismatch — trailing-slash variant of the same directory; runs on all platforms.
  • metadata project in native Windows spelling is not a mismatch — the exact cygpath -w shape the bridge writes; Windows-only (skips elsewhere).

The existing genuinely-different-directory fixture still reports a mismatch after this change. Restore-to-FAIL verified on this branch: with the _delivery.sh hunk reverted, both new tests fail.

Exact commands:

npx bats --filter 'metadata project spelling variant' tests/test_delivery.bats
npx bats --filter 'metadata project in native Windows spelling' tests/test_delivery.bats

Note the current windows-latest CI job runs only the Windows-filtered cases of tests/test_install.bats, so test_delivery.bats does not execute on Windows in CI — the Windows-spelling result above is from the stated Windows/Git Bash machine; the spelling-variant test runs on the Ubuntu/macOS shards.

Verification

Measured on Windows 11 + Git Bash (MSYS2 3.4.7) against codex-cli 0.144.6; this diff has been running in our downstream deployment since 2026-07-21. Not measured on macOS/Linux — there the compare is unchanged for identical spellings, and the trailing-slash test covers the canonicalization path on all platforms.

Note on #505

#505 touches the line just below this hunk (kill -0_agmsg_pid_alive). The two text changes are independent and rebase trivially in either order.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EjjWN3rwUxE6qWBicECXHC

…aths

The bridge records its meta project via Node's path.resolve (C:\x\y on
Windows) while delivery.sh callers pass Git Bash spellings (/c/x/y or
C:/x/y), so the verbatim compare from f6a8bcb (fujibee#232) labeled every live
bridge "stale pidfile (metadata mismatch)" on Windows. Reuse the
project-identity machinery the launcher already relies on
(agmsg_canonical_path + agmsg_normalize_project_path) on both sides.

Read-only status path; kill/spawn gating is unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PwnD2ZoEN1adKuqfbeBKg5
@fujibee

fujibee commented Jul 29, 2026

Copy link
Copy Markdown
Owner

@chemica-tan

The production change reads correctly and we found nothing blocking in it. The helper order is sound — lib/resolve-project.sh is sourced by delivery.sh before the per-type plug, so both helpers are defined at the call site — and the canonical-then-normalize order is what makes the native Windows spelling work: agmsg_canonical_path falls back to the input unchanged when cd fails, and the normalizer then folds the backslashes. It also composes correctly with the liveness change that has since landed on main (#505): different hunk, same function, and the two are independent. Your framing of #459 and #415 as two separate layers matches what we see.

The blocker is verification rather than code.

The regression this fixes is the native Windows spelling, and that path has not actually been executed anywhere. The test you added for it is guarded by case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*), so it skips on the Linux and macOS runners; and the required Windows job (bats (windows-latest, install helpers)) runs tests/test_install.bats only, so it never picks that file up either. The trailing-slash test does run, but it exercises canonicalization in general rather than the C:\x fallback into slash normalization that is the reported failure. So the green check on this PR does not yet cover the thing being fixed.

Could you run the Windows-targeted test on the Windows 11 + Git Bash host you reported from and paste the output here? A single run is enough. If that host is not available, say so and we will record the Windows path as unverified rather than block on it — we just do not want the check mark to imply coverage it does not have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants