Add institutional knowledge skills for git-ai contributors#1222
Add institutional knowledge skills for git-ai contributors#1222jwiegley wants to merge 1 commit into
Conversation
jwiegley
force-pushed
the
johnw/claude-skills
branch
3 times, most recently
from
2e59b1f to
ab1880a
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| impl AgentCheckpointPreset for MyAgentPreset { | ||
| fn run(&self, flags: AgentCheckpointFlags) -> Result<AgentRunResult, GitAiError> { | ||
| let input: MyAgentHookInput = serde_json::from_str( | ||
| flags.hook_input.as_deref().unwrap_or("") | ||
| ).map_err(|e| GitAiError::PresetError(format!("Failed to parse hook input: {}", e)))?; | ||
|
|
||
| let is_pre_edit = input.hook_event_name == "PreToolUse"; | ||
| let is_post_edit = input.hook_event_name == "PostToolUse"; | ||
|
|
||
| // Extract file paths from the tool_input | ||
| let file_paths = extract_file_paths_from_tool_input(&input.tool_input); | ||
|
|
||
| if is_pre_edit { | ||
| // Check if this is a bash/shell tool (use bash_tool fast path) | ||
| if is_bash_tool(&input.tool_name) { | ||
| let strategy = prepare_agent_bash_pre_hook( | ||
| &input.session_id, | ||
| input.tool_use_id.as_deref(), | ||
| SupportedAgent::MyAgent, | ||
| &input.tool_name.as_deref().unwrap_or(""), | ||
| &flags, | ||
| )?; | ||
| match strategy { | ||
| BashPreHookStrategy::EmitHumanCheckpoint => { | ||
| return Ok(AgentRunResult { | ||
| agent_id: make_agent_id("my_agent", &input.session_id, ""), | ||
| checkpoint_kind: CheckpointKind::Human, | ||
| ..Default::default() | ||
| }); | ||
| } | ||
| BashPreHookStrategy::SkipCheckpoint => { | ||
| return Err(GitAiError::PresetError("skip".to_string())); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // File edit: pre-edit is always a Human (untracked) checkpoint | ||
| return Ok(AgentRunResult { | ||
| agent_id: make_agent_id("my_agent", &input.session_id, ""), | ||
| checkpoint_kind: CheckpointKind::Human, | ||
| will_edit_filepaths: Some(file_paths), | ||
| repo_working_dir: Some(input.cwd.clone()), | ||
| ..Default::default() | ||
| }); | ||
| } | ||
|
|
||
| if is_post_edit { | ||
| // Load transcript and model from your agent's storage | ||
| let (transcript, model) = load_my_agent_transcript(&input.session_id)?; | ||
|
|
||
| return Ok(AgentRunResult { | ||
| agent_id: make_agent_id("my_agent", &input.session_id, &model), | ||
| checkpoint_kind: CheckpointKind::AiAgent, | ||
| edited_filepaths: Some(file_paths), | ||
| transcript: Some(transcript), | ||
| repo_working_dir: Some(input.cwd.clone()), | ||
| ..Default::default() | ||
| }); | ||
| } | ||
|
|
||
| Err(GitAiError::PresetError(format!("Unknown event: {}", input.hook_event_name))) | ||
| } |
There was a problem hiding this comment.
🔴 add-agent SKILL describes non-existent API: wrong trait, types, return values, and method signature
The entire preset implementation example in Step 1 uses an API surface that doesn't exist in the codebase. The actual trait is AgentPreset (at src/commands/checkpoint_agent/presets/mod.rs:144), not AgentCheckpointPreset. The actual method signature is fn parse(&self, hook_input: &str, trace_id: &str) -> Result<Vec<ParsedHookEvent>, GitAiError>, not fn run(&self, flags: AgentCheckpointFlags) -> Result<AgentRunResult, GitAiError>. The types AgentCheckpointFlags, AgentRunResult, BashPreHookStrategy, and prepare_agent_bash_pre_hook don't exist anywhere in the codebase (verified via grep). An AI agent following this guide would produce code that doesn't compile.
Verified non-existent symbols
AgentCheckpointPreset— actual:AgentPresetAgentCheckpointFlags— no equivalent existsAgentRunResult— actual return:Vec<ParsedHookEvent>BashPreHookStrategy— doesn't existprepare_agent_bash_pre_hook— doesn't existSupportedAgent— actual:Agent(in bash_tool.rs:370)make_agent_idhelper — not a standalone function
Prompt for agents
The entire Step 1 preset implementation example in skills/git-ai-add-agent/SKILL.md needs to be rewritten to match the actual API. The real trait is AgentPreset (in src/commands/checkpoint_agent/presets/mod.rs), with method signature: fn parse(&self, hook_input: &str, trace_id: &str) -> Result<Vec<ParsedHookEvent>, GitAiError>. The return type uses ParsedHookEvent enum variants (PreFileEdit, PostFileEdit, PreBashCall, PostBashCall, KnownHumanEdit, UntrackedEdit) defined in the same mod.rs file. Each variant carries a PresetContext struct with agent_id, session_id, trace_id, cwd, and metadata fields. Study the existing presets in src/commands/checkpoint_agent/presets/ (e.g., pi.rs, amp.rs, claude.rs) for the actual implementation pattern. The bash tool handling uses Agent enum (not SupportedAgent) in src/commands/checkpoint_agent/bash_tool.rs with classify_tool() and handle_bash_pre_tool_use_with_context / handle_bash_post_tool_use functions.
Was this helpful? React with 👍 or 👎 to provide feedback.
| In `src/commands/checkpoint_agent/agent_presets.rs`, add to the preset lookup: | ||
|
|
||
| ```rust | ||
| pub fn get_preset_for_agent(name: &str) -> Option<Box<dyn AgentCheckpointPreset>> { | ||
| match name { | ||
| // ... existing presets ... | ||
| "my_agent" | "my-agent" => Some(Box::new(MyAgentPreset)), | ||
| _ => None, | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🔴 add-agent SKILL references wrong file paths for presets throughout
Step 2 tells the AI agent to register the preset in src/commands/checkpoint_agent/agent_presets.rs using a function called get_preset_for_agent(). Neither exists. The actual file is src/commands/checkpoint_agent/presets/mod.rs and the actual function is resolve_preset() (at line 148 of that file). Similarly, Step 1 references src/commands/checkpoint_agent/amp_preset.rs and pi_preset.rs but the actual paths are src/commands/checkpoint_agent/presets/amp.rs and presets/pi.rs. Line 350 references opencode_preset.rs but it's presets/opencode.rs. An AI agent following these paths would fail to find the files.
| In `src/commands/checkpoint_agent/agent_presets.rs`, add to the preset lookup: | |
| ```rust | |
| pub fn get_preset_for_agent(name: &str) -> Option<Box<dyn AgentCheckpointPreset>> { | |
| match name { | |
| // ... existing presets ... | |
| "my_agent" | "my-agent" => Some(Box::new(MyAgentPreset)), | |
| _ => None, | |
| } | |
| } | |
| ``` | |
| In `src/commands/checkpoint_agent/presets/mod.rs`, add to the preset lookup: | |
| ```rust | |
| pub fn resolve_preset(name: &str) -> Result<Box<dyn AgentPreset>, GitAiError> { | |
| match name { | |
| // ... existing presets ... | |
| "my-agent" => Some(Box::new(my_agent::MyAgentPreset)), | |
| _ => None, | |
| } | |
| } |
<!-- devin-review-badge-begin -->
<a href="https://app.devin.ai/review/git-ai-project/git-ai/pull/1222" target="_blank">
<picture>
<source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1">
<img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open in Devin Review">
</picture>
</a>
<!-- devin-review-badge-end -->
---
*Was this helpful? React with 👍 or 👎 to provide feedback.*
| define_feature_flags!( | ||
| rewrite_stash: rewrite_stash, debug = true, release = true, | ||
| inter_commit_move: checkpoint_inter_commit_move, debug = false, release = false, | ||
| auth_keyring: auth_keyring, debug = false, release = false, | ||
| async_mode: async_mode, debug = false, release = true, // ← diverges! | ||
| ); |
There was a problem hiding this comment.
🔴 Architecture SKILL.md lists phantom feature flags async_mode and inter_commit_move that don't exist
The feature flag example at lines 96-101 lists inter_commit_move and async_mode as feature flags, and lines 46, 100, and 111 reference async_mode for daemon mode behavior. Neither flag exists in the actual define_feature_flags! invocation (src/feature_flags.rs:80-88). The actual flags are: rewrite_stash, auth_keyring, git_hooks_enabled, git_hooks_externally_managed, transcript_streaming, transcript_sweep, checkpoint_debug_log. This would mislead agents into referencing feature_flags().async_mode (which won't compile) or believing daemon mode is controlled by a feature flag.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ]); | ||
| ``` | ||
|
|
||
| **Valid checkpoint preset names:** `claude`, `codex`, `continue-cli`, `cursor`, `gemini`, `github-copilot`, `amp`, `windsurf`, `opencode`, `pi`, `ai_tab`, `firebender`, `mock_ai`, `mock_known_human`, `known_human`, `droid`, `agent-v1`. |
There was a problem hiding this comment.
🟡 Test SKILL omits human from valid preset names list despite using it in examples
Line 171 lists valid checkpoint preset names but omits human, which is both a valid preset in resolve_preset() (src/commands/checkpoint_agent/presets/mod.rs:165) and used extensively in the same SKILL file's examples (e.g., line 157: repo.git_ai(&["checkpoint", "human", "example.rs"])). An AI agent reading the valid names list would not know human is a valid preset, contradicting the code examples directly below. Also omits cursor-background which is registered in resolve_preset().
| **Valid checkpoint preset names:** `claude`, `codex`, `continue-cli`, `cursor`, `gemini`, `github-copilot`, `amp`, `windsurf`, `opencode`, `pi`, `ai_tab`, `firebender`, `mock_ai`, `mock_known_human`, `known_human`, `droid`, `agent-v1`. | |
| **Valid checkpoint preset names:** `claude`, `codex`, `continue-cli`, `cursor`, `cursor-background`, `gemini`, `github-copilot`, `amp`, `windsurf`, `opencode`, `pi`, `ai_tab`, `firebender`, `mock_ai`, `mock_known_human`, `known_human`, `human`, `droid`, `agent-v1`. |
Was this helpful? React with 👍 or 👎 to provide feedback.
| git-ai checkpoint <preset> [<file>...] | ||
| ``` | ||
|
|
||
| `<preset>` is matched to `AgentCheckpointPreset` implementations in `src/commands/checkpoint_agent/agent_presets.rs`. Test-only presets: `mock_ai`, `mock_known_human`, `human` (bare/legacy). |
There was a problem hiding this comment.
🟡 Attribution SKILL references wrong file path for agent presets
Line 39 states presets are in src/commands/checkpoint_agent/agent_presets.rs, but the actual file is src/commands/checkpoint_agent/presets/mod.rs. This same incorrect path appears in the attribution SKILL at line 200. An AI agent trying to navigate to this file would not find it.
| `<preset>` is matched to `AgentCheckpointPreset` implementations in `src/commands/checkpoint_agent/agent_presets.rs`. Test-only presets: `mock_ai`, `mock_known_human`, `human` (bare/legacy). | |
| `<preset>` is matched to `AgentPreset` implementations in `src/commands/checkpoint_agent/presets/mod.rs`. Test-only presets: `mock_ai`, `mock_known_human`, `human` (bare/legacy). |
Was this helpful? React with 👍 or 👎 to provide feedback.
jwiegley
force-pushed
the
johnw/claude-skills
branch
2 times, most recently
from
143e613 to
db69cf5
Compare
| SupportedAgent::MyAgent => { | ||
| if ["bash", "shell", "run_command"].contains(&tool_name_lower.as_str()) { | ||
| ToolClass::Bash | ||
| } else if ["edit_file", "write_file", "apply_patch"].contains(&tool_name_lower.as_str()) { | ||
| ToolClass::FileEdit | ||
| } else { | ||
| ToolClass::Skip | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| And add to the `SupportedAgent` enum in `bash_tool.rs`. |
There was a problem hiding this comment.
🔴 Add-agent SKILL.md references non-existent SupportedAgent enum — actual name is Agent
Lines 64 and 136 reference SupportedAgent::MyAgent, but the actual enum is Agent (defined at src/commands/checkpoint_agent/bash_tool.rs:370). The enum variants are Claude, Gemini, ContinueCli, Droid, Amp, OpenCode, Firebender, Codex, Pi, Windsurf, Cursor. An agent following this guide would reference a non-existent type.
Prompt for agents
Replace all references to `SupportedAgent` with `Agent` (the actual enum at `src/commands/checkpoint_agent/bash_tool.rs:370`). Also replace `is_known_preset_for_bash_tool` (line 179) which does not exist — the bash tool classification is done entirely through the `classify_tool(agent: Agent, tool_name: &str) -> ToolClass` function at bash_tool.rs:308. Update the code examples at lines 125-133 to match the actual `classify_tool` match arm pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| ## Step 1: Create the Preset | ||
|
|
||
| Create `src/commands/checkpoint_agent/<agent>_preset.rs` (for complex agents) or add to `agent_presets.rs` (for simpler ones). |
There was a problem hiding this comment.
🔴 Add-agent SKILL.md directs agents to wrong directory structure for preset files
Line 20 says to create src/commands/checkpoint_agent/<agent>_preset.rs, and the checklist at line 333 says src/commands/checkpoint_agent/<agent>_preset.rs. But the actual preset files live in src/commands/checkpoint_agent/presets/<agent>.rs (a presets/ subdirectory, without the _preset suffix). For example, the Pi preset is at presets/pi.rs, not pi_preset.rs. An agent following these instructions would create files in the wrong location and with the wrong naming convention.
| Create `src/commands/checkpoint_agent/<agent>_preset.rs` (for complex agents) or add to `agent_presets.rs` (for simpler ones). | |
| Create `src/commands/checkpoint_agent/presets/<agent>.rs` (for complex agents) or add to `presets/mod.rs` (for simpler ones). |
Was this helpful? React with 👍 or 👎 to provide feedback.
jwiegley
force-pushed
the
johnw/claude-skills
branch
from
db69cf5 to
ad2cad8
Compare
| // Check if this is a bash/shell tool (use bash_tool fast path) | ||
| if is_bash_tool(&input.tool_name) { | ||
| let strategy = prepare_agent_bash_pre_hook( | ||
| &input.session_id, | ||
| input.tool_use_id.as_deref(), | ||
| SupportedAgent::MyAgent, | ||
| &input.tool_name.as_deref().unwrap_or(""), | ||
| &flags, | ||
| )?; | ||
| match strategy { | ||
| BashPreHookStrategy::EmitHumanCheckpoint => { | ||
| return Ok(AgentRunResult { | ||
| agent_id: make_agent_id("my_agent", &input.session_id, ""), | ||
| checkpoint_kind: CheckpointKind::Human, | ||
| ..Default::default() | ||
| }); | ||
| } | ||
| BashPreHookStrategy::SkipCheckpoint => { | ||
| return Err(GitAiError::PresetError("skip".to_string())); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Add-agent guide references non-existent prepare_agent_bash_pre_hook and BashPreHookStrategy
Lines 61-79 use prepare_agent_bash_pre_hook() returning BashPreHookStrategy::EmitHumanCheckpoint / BashPreHookStrategy::SkipCheckpoint, but neither the function nor the enum exist. The actual function is handle_bash_pre_tool_use_with_context() at src/commands/checkpoint_agent/bash_tool.rs:906 returning BashPreHookResult, and the checkpoint action enum is BashCheckpointAction at line 267. Following this guide produces completely non-functional bash tool handling code.
Prompt for agents
The bash tool handling code example in skills/git-ai-add-agent/SKILL.md lines 59-80 uses fabricated function and type names. Replace prepare_agent_bash_pre_hook with handle_bash_pre_tool_use_with_context (src/commands/checkpoint_agent/bash_tool.rs:906). Replace BashPreHookStrategy with BashCheckpointAction (line 267 of bash_tool.rs). Study the actual BashCheckpointAction variants and BashPreHookResult struct to write a correct example.
Was this helpful? React with 👍 or 👎 to provide feedback.
jwiegley
force-pushed
the
johnw/claude-skills
branch
2 times, most recently
from
d231597 to
179b1ba
Compare
| repo.patch_git_ai_config(|patch| { | ||
| patch.exclude_prompts_in_repositories = Some(vec![]); | ||
| patch.feature_flags = Some(serde_json::json!({"async_mode": false})); | ||
| }); |
There was a problem hiding this comment.
🟡 Test SKILL config patch example references non-existent async_mode feature flag
The config patching example at line 65 uses {"async_mode": false} as a feature flag value, but async_mode does not exist as a feature flag in the codebase (verified via src/feature_flags.rs). A developer copying this example to disable daemon mode would silently have no effect, since the unknown key would be ignored by the deserialization.
Prompt for agents
The config patch example in skills/git-ai-test/SKILL.md line 65 uses async_mode which doesn't exist as a feature flag. Replace with an actual feature flag from src/feature_flags.rs, for example transcript_sweep or checkpoint_debug_log, or remove the feature_flags line from the example entirely since daemon/wrapper mode is controlled by GIT_AI_TEST_GIT_MODE env var, not feature flags.
Was this helpful? React with 👍 or 👎 to provide feedback.
Five skills encoding the codebase's patterns, idioms, and conventions so future contributors have actionable reference when working in the repo: - git-ai-test: TestRepo/TestFile API, lines! macro, two-path strategy (set_contents vs fs::write + explicit checkpoints), test isolation, reuse_tests_in_worktree! / subdir_test_variants! macros, and what makes a good attribution test - git-ai-architecture: binary dispatch (argv[0] routing, GIT_AI=git), GitAiError design, feature flag macro (debug/release defaults, env precedence), config OnceLock, three logging layers, cross-platform signal/process patterns, file navigation guide - git-ai-attribution: checkpoint types and semantics, working log storage layout, AuthorshipLog schema v3.0.0 wire format, hash conventions (h_ prefix), AttributionTracker 5-phase diff algorithm, VirtualAttributions constructors, coordinate space discipline, pre/post-commit hook logic, and key system invariants - git-ai-hooks: all 12 hook modules with pre/post responsibilities, CommandHooksContext, rewrite log schema, rebase authorship rewrite algorithm (fast/slow paths), working log keying through all rewrite operations, read-only command classification, signal forwarding - git-ai-add-agent: end-to-end checklist for adding a new AI agent preset (AgentCheckpointPreset, bash tool classification, transcript loading, HookInstaller MDM layer, test integration, naming conventions, reference implementations to study) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jwiegley
force-pushed
the
johnw/claude-skills
branch
from
179b1ba to
ce717b7
Compare
| fn install_hooks(&self, params: &HookInstallerParams) -> Result<Option<String>, GitAiError> { | ||
| // Write git-ai hook entries to the agent's config file | ||
| // Return Some(unified_diff_string) for user preview, or None on no-op | ||
| let binary_path = params.binary_path.display().to_string(); | ||
| let pre_hook_cmd = format!("{} checkpoint my-agent --hook-input stdin", binary_path); | ||
| let post_hook_cmd = format!("{} checkpoint my-agent --hook-input stdin", binary_path); | ||
| // ... write to config file ... | ||
| } | ||
|
|
||
| fn uninstall_hooks(&self, params: &HookInstallerParams) -> Result<Option<String>, GitAiError> { | ||
| // Remove git-ai entries from the agent's config | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 SKILL.md install_hooks and uninstall_hooks signatures are missing required dry_run parameter
The installer example at lines 209-220 shows install_hooks and uninstall_hooks without a dry_run: bool parameter. The actual HookInstaller trait at src/mdm/hook_installer.rs:61-73 requires fn install_hooks(&self, params: &HookInstallerParams, dry_run: bool) and fn uninstall_hooks(&self, params: &HookInstallerParams, dry_run: bool). Code following this guide will fail to compile because it doesn't satisfy the trait.
| fn install_hooks(&self, params: &HookInstallerParams) -> Result<Option<String>, GitAiError> { | |
| // Write git-ai hook entries to the agent's config file | |
| // Return Some(unified_diff_string) for user preview, or None on no-op | |
| let binary_path = params.binary_path.display().to_string(); | |
| let pre_hook_cmd = format!("{} checkpoint my-agent --hook-input stdin", binary_path); | |
| let post_hook_cmd = format!("{} checkpoint my-agent --hook-input stdin", binary_path); | |
| // ... write to config file ... | |
| } | |
| fn uninstall_hooks(&self, params: &HookInstallerParams) -> Result<Option<String>, GitAiError> { | |
| // Remove git-ai entries from the agent's config | |
| } | |
| } | |
| fn install_hooks(&self, params: &HookInstallerParams, dry_run: bool) -> Result<Option<String>, GitAiError> { | |
| // Write git-ai hook entries to the agent's config file | |
| // Return Some(unified_diff_string) for user preview, or None on no-op | |
| let binary_path = params.binary_path.display().to_string(); | |
| let pre_hook_cmd = format!("{} checkpoint my-agent --hook-input stdin", binary_path); | |
| let post_hook_cmd = format!("{} checkpoint my-agent --hook-input stdin", binary_path); | |
| // ... write to config file ... | |
| } | |
| fn uninstall_hooks(&self, params: &HookInstallerParams, dry_run: bool) -> Result<Option<String>, GitAiError> { | |
| // Remove git-ai entries from the agent's config | |
| } | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Five skills encoding the codebase's patterns, idioms, and conventions
so future contributors have actionable reference when working in the repo:
git-ai-test: TestRepo/TestFile API, lines! macro, two-path strategy
(set_contents vs fs::write + explicit checkpoints), test isolation,
reuse_tests_in_worktree! / subdir_test_variants! macros, and what
makes a good attribution test
git-ai-architecture: binary dispatch (argv[0] routing, GIT_AI=git),
GitAiError design, feature flag macro (debug/release defaults,
env precedence), config OnceLock, three logging layers,
cross-platform signal/process patterns, file navigation guide
git-ai-attribution: checkpoint types and semantics, working log
storage layout, AuthorshipLog schema v3.0.0 wire format, hash
conventions (h_ prefix), AttributionTracker 5-phase diff algorithm,
VirtualAttributions constructors, coordinate space discipline,
pre/post-commit hook logic, and key system invariants
git-ai-hooks: all 12 hook modules with pre/post responsibilities,
CommandHooksContext, rewrite log schema, rebase authorship rewrite
algorithm (fast/slow paths), working log keying through all rewrite
operations, read-only command classification, signal forwarding
git-ai-add-agent: end-to-end checklist for adding a new AI agent
preset (AgentCheckpointPreset, bash tool classification, transcript
loading, HookInstaller MDM layer, test integration, naming
conventions, reference implementations to study)
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com