Skip to content

fix(hooks-sync): use configured agent name for non-Claude hooks (gt-lch)#3242

Closed
baumgold wants to merge 4 commits intogastownhall:mainfrom
baumgold:polecat/gt-lch-hooks-sync-agent
Closed

fix(hooks-sync): use configured agent name for non-Claude hooks (gt-lch)#3242
baumgold wants to merge 4 commits intogastownhall:mainfrom
baumgold:polecat/gt-lch-hooks-sync-agent

Conversation

@baumgold
Copy link
Copy Markdown
Contributor

@baumgold baumgold commented Mar 24, 2026

Fixes #3247

When gt doctor hooks-sync or gt hooks sync runs for non-Claude template-based agents (OpenCode, Gemini, etc.), it uses ResolveRoleAgentConfig which falls back to claude when the agent binary is not in PATH (e.g. in CI). This causes non-Claude hook targets to be silently skipped.

Changes:

  • Use ResolveRoleAgentName (reads config directly, no PATH fallback) for determining which template to apply
  • Update patrol test to expect the correct number of formula vars (judgment_enabled, review_depth)
  • Remove unused hooksDir parameter from writeTemplate

mayor added 3 commits March 24, 2026 16:41
…ent_enabled and review_depth vars (gt-lch)
…resolved (gt-lch)

When the configured agent binary (e.g. opencode) is not in PATH,
ResolveRoleAgentConfig falls back to claude and the plugin files are
never created. Switch to ResolveRoleAgentName (no binary validation)
so hooks are installed based on the configured agent regardless of
whether the binary is currently available on the machine.
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 24, 2026

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
9319 3 9316 48
View the top 3 failed test(s) by shortest run time
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.02s 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.02s)

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

This PR fixes the pre-existing main failures that block all other PRs in this series:

  • Lint fix: removes unused hooksDir param from writeTemplate in installer.go
  • Test fix: updates TestBuildRefineryPatrolVars_FullConfig to expect 7 vars (formula already has judgment_enabled + review_depth on main)

Merge order: this PR should land first. Once merged, PRs #3241, #3243, #3244, #3245, #3246 should all pass CI.

The CI failures shown on this PR (TestPostWanted_EmptyTitle, TestStampLoop_*, TestValidateConvoyStatusTransition, etc.) are intermittent/flaky tests — all pass locally on both main and this branch.

@baumgold
Copy link
Copy Markdown
Contributor Author

Added commit 86038e3: removes t.Parallel() from TestStampLoop_SelfStampFails and TestStampLoop_InvalidValence — both tests modify package-level globals (wlStampQuality etc.) and must not run in parallel. This is a pre-existing data race on main that causes flaky CI failures.

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.

hooks-sync: use configured agent name for non-Claude agents (not resolved binary)

2 participants