Skip to content

fix(hooks): POSIX-normalize worktree path in SessionStart hook - #81

Merged
thinkyou0714 merged 3 commits into
mainfrom
fix/sessionstart-windows-path
Jul 10, 2026
Merged

thinkyou0714 merged 3 commits into
mainfrom
fix/sessionstart-windows-path

Conversation

@thinkyou0714

Copy link
Copy Markdown
Owner

Problem (root cause)

writeSessionStartHook in src/core/hooks.ts embedded the raw worktree path (${worktreePath}) into the generated session-start.sh, unlike its siblings writeStopHook (L66) and writePreToolUseHook (L288), which normalize backslashes via .replace(/\/g, "/").

On Windows, a worktree path like C:\Users\…\session becomes TASK_STATE_FILE="C:\Users\…\session/TASK_STATE.md" inside the bash hook, where \U, \s, … are interpreted as bash escapes. Result: after a context compaction, the SessionStart hook silently fails to re-inject TASK_STATE.md.

Fix

  • Add const wtPosix = worktreePath.replace(/\/g, "/"); and use ${wtPosix} for TASK_STATE_FILE — matching the established pattern in the two sibling hook writers. Behavior is unchanged on POSIX; Windows is corrected.

Test

  • New tests/hooks-session-start.test.ts installs the hooks into a temp dir and asserts the generated session-start.sh embeds a backslash-free POSIX path. This is exercised by the existing windows-latest CI matrix job, where the old code would fail.

Verification (local)

  • npm run typecheck ✅ · npm run lint ✅ · new test ✅ (1 passed).
  • Note: 2 unrelated tests (hooks-circuit-breaker, hooks-blocklist) time out locally on Windows because they spawnSync("bash", …) (slow on this machine, vitest 5s default). They are pre-existing on main and pass on GitHub CI; a follow-up PR adds vitest.config.ts with a higher testTimeout to remove that local flakiness.

Scope: exactly 2 files (src/core/hooks.ts, tests/hooks-session-start.test.ts).

writeSessionStartHook embedded the raw worktree path into the generated
session-start.sh, unlike writeStopHook (L66) and writePreToolUseHook
(L288) which convert backslashes via .replace(/\/g, "/"). On Windows
this corrupted TASK_STATE_FILE (backslashes become bash escapes), so
TASK_STATE.md was not re-injected after context compaction.

Apply the same wtPosix conversion and add a regression test asserting the
generated hook embeds a backslash-free POSIX path (exercised by the
windows-latest CI job).
…ows-path

# Conflicts:
#	tests/hooks-session-start.test.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc83136e80

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/hooks-session-start.test.ts Outdated
expect(taskStateLine).toBeDefined();
expect(taskStateLine).not.toContain("\\");
expect(taskStateLine).toBe(
`TASK_STATE_FILE="${tmp.replace(/\/g, "/")}/TASK_STATE.md"`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Escape the backslash in the regex literal

In any environment that runs the test suite or typecheck, this added assertion prevents the file from parsing: /\/g was intended, but the source currently reads as a malformed regex/string sequence (/\/g, "/") in the template expression), so Vitest/tsc will fail before these hook tests can run. Escape the backslash in the regex literal used to normalize tmp.

Useful? React with 👍 / 👎.

@thinkyou0714
thinkyou0714 merged commit f6b0af8 into main Jul 10, 2026
11 checks passed
@thinkyou0714
thinkyou0714 deleted the fix/sessionstart-windows-path branch July 10, 2026 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant