Skip to content

fix(hooks): JSON-escape GT_BIN path on Windows in resolveAndSubstitute (gt-1at)#3246

Closed
baumgold wants to merge 2 commits intogastownhall:mainfrom
baumgold:polecat/gt-1at-windows-json-gtbin
Closed

fix(hooks): JSON-escape GT_BIN path on Windows in resolveAndSubstitute (gt-1at)#3246
baumgold wants to merge 2 commits intogastownhall:mainfrom
baumgold:polecat/gt-1at-windows-json-gtbin

Conversation

@baumgold
Copy link
Copy Markdown
Contributor

@baumgold baumgold commented Mar 24, 2026

Fixes #3250

On Windows, GT_BIN paths contain backslashes (e.g. C:\\tools\\gt.exe). When substituted literally into a JSON template, raw backslashes produce invalid JSON.

Changes:

  • Escape backslashes in resolveAndSubstitute before {{GT_BIN}} substitution
  • Fix test reformatting to use json.MarshalIndent instead of strings.ReplaceAll(":", " : ") which was corrupting Windows drive letters (C:C :)

nux and others added 2 commits March 24, 2026 16:41
…e (gt-1at)

Windows paths contain backslashes which are invalid in JSON strings without
escaping. When {{GT_BIN}} was substituted into JSON templates, it produced
invalid JSON on Windows, causing TemplateContentEqual to fail to unmarshal
the content and incorrectly return false (triggering SyncUpdated instead
of SyncUnchanged).

Fix: escape backslashes before substitution.

Fixes TestSyncForRole_JSONWhitespaceInsensitive on Windows.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… paths (gt-1at)

strings.ReplaceAll(original, ":", " : ") in TestSyncForRole_JSONWhitespaceInsensitive
corrupts Windows drive letters (C: → C :), making the JSON structurally different
and causing the test to fail even after the backslash-escaping fix.

Replace with json.Unmarshal + json.MarshalIndent round-trip which changes only
indentation whitespace without touching string values.
@codecov-commenter
Copy link
Copy Markdown

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
9319 5 9314 48
View the top 3 failed test(s) by shortest run time
github.com/steveyegge/gastown/internal/cmd::TestBuildRefineryPatrolVars_FullConfig
Stack Traces | 0s run time
=== RUN   TestBuildRefineryPatrolVars_FullConfig
    patrol_helpers_test.go:166: expected 5 vars, got 7: [target_branch=main integration_branch_refinery_enabled=true integration_branch_auto_land=false run_tests=true delete_merged_branches=true judgment_enabled=false review_depth=standard]
--- FAIL: TestBuildRefineryPatrolVars_FullConfig (0.00s)
github.com/steveyegge/gastown/internal/cmd::TestRunHooksSyncNonClaudeAgent
Stack Traces | 0.01s run time
=== RUN   TestRunHooksSyncNonClaudeAgent
Syncing hooks...
  ✓ mayor/.claude/settings.json (created)
  ✓ deacon/.claude/settings.json (created)
  ✓ myrig/crew/.claude/settings.json (created)
warning: role_agents[crew]=opencode - agent "opencode" binary "opencode" not found in PATH, falling back to default

Synced 3 targets (3 created, 0 updated, 0 unchanged)
    hooks_sync_test.go:366: opencode plugin not created in worktree alice
--- FAIL: TestRunHooksSyncNonClaudeAgent (0.01s)
github.com/steveyegge/gastown/internal/doctor::TestHooksSyncCheck_Fix_TemplateAgent
Stack Traces | 0.01s run time
=== RUN   TestHooksSyncCheck_Fix_TemplateAgent
warning: role_agents[crew]=opencode - agent "opencode" binary "opencode" not found in PATH, falling back to default
    hooks_sync_check_test.go:219: expected StatusWarning before fix, got OK
--- FAIL: TestHooksSyncCheck_Fix_TemplateAgent (0.01s)
github.com/steveyegge/gastown/internal/doctor::TestHooksSyncCheck_TemplateAgent_Missing
Stack Traces | 0.01s run time
=== RUN   TestHooksSyncCheck_TemplateAgent_Missing
warning: role_agents[crew]=opencode - agent "opencode" binary "opencode" not found in PATH, falling back to default
    hooks_sync_check_test.go:193: expected StatusWarning for missing template agent file, got OK: All 3 hook targets in sync
--- FAIL: TestHooksSyncCheck_TemplateAgent_Missing (0.01s)
github.com/steveyegge/gastown/internal/doctor::TestHooksSyncCheck_TemplateAgent_OutOfSync
Stack Traces | 0.01s run time
=== RUN   TestHooksSyncCheck_TemplateAgent_OutOfSync
warning: role_agents[crew]=opencode - agent "opencode" binary "opencode" not found in PATH, falling back to default
    hooks_sync_check_test.go:172: expected StatusWarning for out-of-sync template agent, got OK: All 3 hook targets in sync
--- FAIL: TestHooksSyncCheck_TemplateAgent_OutOfSync (0.01s)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@baumgold
Copy link
Copy Markdown
Contributor Author

Build Status Note

The CI failures on this PR are pre-existing failures on main, not introduced by this PR's changes:

Merge order: #3242 must land first. Once merged, this PR's CI should pass.

The Windows-specific fix ({{GT_BIN}} backslash escaping + MarshalIndent test) is the only change here and all hooks tests pass locally.

steveyegge added a commit that referenced this pull request Mar 27, 2026
Fix-merge three baumgold PRs with conflict resolution:

- PR #3246: JSON-escape GT_BIN on Windows — use json.MarshalIndent in
  test reformatter to avoid corrupting Windows drive letters
- PR #3243: doctor hooks-sync uses ResolveRoleAgentName + preset directly
  instead of ResolveAgentConfigByName, preventing false negatives when
  non-Claude binary not in PATH
- PR #3242: hooks-sync uses same preset-based approach for installing
  hooks for configured agent even when binary not found
- Both #3243/#3242: remove t.Parallel() from tests that modify
  package-level globals, eliminating data races

Co-authored-by: baumgold <baumgold@users.noreply.github.com>
htristan pushed a commit to htristan/gastown that referenced this pull request Mar 27, 2026
…ooks fixes (gt-91g)

Fix-merge three baumgold PRs with conflict resolution:

- PR gastownhall#3246: JSON-escape GT_BIN on Windows — use json.MarshalIndent in
  test reformatter to avoid corrupting Windows drive letters
- PR gastownhall#3243: doctor hooks-sync uses ResolveRoleAgentName + preset directly
  instead of ResolveAgentConfigByName, preventing false negatives when
  non-Claude binary not in PATH
- PR gastownhall#3242: hooks-sync uses same preset-based approach for installing
  hooks for configured agent even when binary not found
- Both gastownhall#3243/gastownhall#3242: remove t.Parallel() from tests that modify
  package-level globals, eliminating data races

Co-authored-by: baumgold <baumgold@users.noreply.github.com>
@baumgold baumgold closed this by deleting the head repository Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/needs-triage Inbox — we haven't looked at it yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: GT_BIN path with backslashes produces invalid JSON in hooks template substitution

2 participants