fix: feat: add Codex and Gemini hook checks to chitin status#24
fix: feat: add Codex and Gemini hook checks to chitin status#24
Conversation
…tatus Co-Authored-By: ForgeCode <forgecode@chitinhq.com>
There was a problem hiding this comment.
Pull request overview
This PR extends chitin status to report hook installation status for the Codex and Gemini drivers, aligning status output with the drivers supported by chitin init.
Changes:
- Added Codex and Gemini hook checks to the
internal/statusstatus check list. - Implemented
checkCodexHooks()andcheckGeminiHooks()using the existing “config containschitin” detection pattern. - Added new tests for the status checks (and also added a large new
internal/policytest file + several new prototype log artifacts).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/status/status.go |
Adds Codex/Gemini hook checks and includes them in Run() output. |
internal/status/status_test.go |
Adds unit tests for the new checks and for Run() including them. |
internal/policy/loader_test.go |
Adds extensive tests for policy loading/parsing (not related to Issue #22 scope). |
outputs/logs/prototype-agent-2026-04-08T20-39-35.jsonl |
Adds prototype agent execution log artifact. |
outputs/logs/prototype-agent-2026-04-08T20-29-17.jsonl |
Adds prototype agent execution log artifact. |
outputs/logs/prototype-agent-2026-04-08T20-23-46.jsonl |
Adds prototype agent execution log artifact. |
outputs/logs/prototype-agent-2026-04-08T20-17-11.jsonl |
Adds prototype agent execution log artifact. |
outputs/logs/prototype-2026-04-08T20-41-47.md |
Adds prototype summary artifact (model error run). |
outputs/logs/prototype-2026-04-08T20-34-30.md |
Adds prototype summary artifact (max turns run). |
outputs/logs/prototype-2026-04-08T20-24-11.md |
Adds prototype summary artifact (drift run). |
outputs/logs/prototype-2026-04-08T20-18-39.md |
Adds prototype summary artifact (successful run). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Test case 3: Global codex hooks with chitin reference | ||
| homeDir := t.TempDir() | ||
| oldHome := os.Getenv("HOME") | ||
| os.Setenv("HOME", homeDir) | ||
| defer os.Setenv("HOME", oldHome) | ||
|
|
||
| globalCodexDir := filepath.Join(homeDir, ".codex") | ||
| if err := os.MkdirAll(globalCodexDir, 0755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| globalHooksFile := filepath.Join(globalCodexDir, "hooks.json") | ||
| if err := os.WriteFile(globalHooksFile, []byte(hooksContent), 0644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| check = checkCodexHooks(tmpDir) | ||
| if !check.OK { | ||
| t.Errorf("Expected check to pass when global hooks installed, got OK: false") | ||
| } | ||
| if check.Detail != globalHooksFile { | ||
| t.Errorf("Expected detail to be global hooks file path, got %q", check.Detail) | ||
| } |
There was a problem hiding this comment.
checkCodexHooks() checks repo-level hooks before global hooks, so in this test the existing local .codex/hooks.json created in test case 2 will always be selected and Detail will not point at the global hooks file. This makes the assertions in test case 3 fail. Consider isolating the global-hook scenario (e.g., use a fresh temp project dir without local hooks, remove/rename the local hooks file, or overwrite the local file to not contain "chitin" before asserting global selection).
| // Test case 3: Global gemini hooks with chitin reference | ||
| homeDir := t.TempDir() | ||
| oldHome := os.Getenv("HOME") | ||
| os.Setenv("HOME", homeDir) | ||
| defer os.Setenv("HOME", oldHome) | ||
|
|
||
| globalGeminiDir := filepath.Join(homeDir, ".gemini") | ||
| if err := os.MkdirAll(globalGeminiDir, 0755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| globalSettingsFile := filepath.Join(globalGeminiDir, "settings.json") | ||
| if err := os.WriteFile(globalSettingsFile, []byte(settingsContent), 0644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| check = checkGeminiHooks(tmpDir) | ||
| if !check.OK { | ||
| t.Errorf("Expected check to pass when global hooks installed, got OK: false") | ||
| } | ||
| if check.Detail != globalSettingsFile { | ||
| t.Errorf("Expected detail to be global settings file path, got %q", check.Detail) | ||
| } |
There was a problem hiding this comment.
Same issue as the Codex test: checkGeminiHooks() checks repo-level .gemini/settings.json before the global file, so the local settings file created in test case 2 will be returned and Detail will not match the global path asserted in test case 3. Use a separate temp project dir without local settings, or remove/neutralize the local file before asserting the global path.
| func TestRunIncludesCodexAndGeminiChecks(t *testing.T) { | ||
| tmpDir := t.TempDir() | ||
|
|
||
| checks := Run(tmpDir) | ||
|
|
||
| // Check that we have all the expected checks | ||
| expectedCheckNames := []string{ | ||
| "chitin binary", | ||
| "chitin.yaml", | ||
| "Claude Code hooks", | ||
| "Copilot hooks", | ||
| "Codex hooks", | ||
| "Gemini hooks", | ||
| "Agent identity", | ||
| } | ||
|
|
||
| if len(checks) != len(expectedCheckNames) { | ||
| t.Errorf("Expected %d checks, got %d", len(expectedCheckNames), len(checks)) | ||
| } | ||
|
|
||
| // Verify each check name exists | ||
| for i, expectedName := range expectedCheckNames { | ||
| if checks[i].Name != expectedName { | ||
| t.Errorf("Check %d: expected name %q, got %q", i, expectedName, checks[i].Name) | ||
| } | ||
| } |
There was a problem hiding this comment.
TestRunIncludesCodexAndGeminiChecks is tightly coupled to the exact number and order of checks returned by Run(). Any future addition/reordering of checks will break this test even if the Codex/Gemini behavior is still correct. Consider asserting that the returned list contains the expected check names (order-independent) and that the Codex/Gemini checks have Critical == false, without requiring an exact slice match.
| package policy | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
| ) |
There was a problem hiding this comment.
This file adds extensive new tests for internal/policy (Load/Find/Parse), which is not mentioned in the PR summary or Issue #22 (which is scoped to chitin status hook checks). Unless this is intentional, consider moving these policy tests into a separate PR to keep the change set focused and reduce review/merge risk.
Closes #22
Summary
Automated fix by ForgeCode agent (deepseek).
Test plan