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 non-existent feature flags (inter_commit_move, async_mode) and omits actual ones
Lines 96-101 show define_feature_flags! with inter_commit_move and async_mode flags, neither of which exist. The actual flags defined in src/feature_flags.rs:80-86 are: rewrite_stash (true/true), auth_keyring (false/false), transcript_streaming (true/true), transcript_sweep (true/true), checkpoint_debug_log (false/false). The commentary about async_mode debug/release divergence (line 111) is also incorrect since that flag doesn't exist. This will give AI agents a wrong mental model of the feature flag system.
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.
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.
jwiegley
force-pushed
the
johnw/claude-skills
branch
2 times, most recently
from
a305538 to
8f84ccc
Compare
| } | ||
| ``` | ||
|
|
||
| Also add to `src/commands/checkpoint_agent/bash_tool.rs` `is_known_preset_for_bash_tool` if the agent uses bash tool detection. |
There was a problem hiding this comment.
🟡 Add-agent SKILL references non-existent function is_known_preset_for_bash_tool
Line 179 instructs adding the new agent to is_known_preset_for_bash_tool in bash_tool.rs, but this function does not exist anywhere in the codebase (verified via grep). Following this instruction would lead to confusion when the function cannot be found.
| Also add to `src/commands/checkpoint_agent/bash_tool.rs` `is_known_preset_for_bash_tool` if the agent uses bash tool detection. | |
| Also add your agent variant to the `Agent` enum in `src/commands/checkpoint_agent/bash_tool.rs` and handle it in `classify_tool()` if the agent uses bash tool detection. | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| Load the transcript lazily and return `PromptUpdateResult::Updated(transcript, model)`. Register a transcript updater in `src/authorship/prompt_utils.rs`: | ||
|
|
||
| ```rust | ||
| "my_agent" => { | ||
| my_agent_preset::transcript_and_model_from_storage( | ||
| agent_metadata.get("transcript_path").map(String::as_str), | ||
| &agent_id.id, | ||
| agent_metadata.get("__test_storage_path").map(String::as_str), | ||
| ) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🟡 Add-agent SKILL references non-existent prompt_utils.rs for transcript registration
Lines 153-163 instruct registering a transcript updater in src/authorship/prompt_utils.rs, but the actual transcript/streaming infrastructure appears to have been refactored. The SKILL references a pattern of matching on "my_agent" and calling a transcript_and_model_from_storage function, which follows the now-obsolete AgentRunResult-based architecture rather than the current StreamSource/StreamFormat pattern visible in src/commands/checkpoint_agent/presets/mod.rs:95-141.
Prompt for agents
The transcript loading section describes a pattern based on the old API (returning transcript directly from the preset). The current architecture uses `StreamSource` and `StreamFormat` (defined in `src/commands/checkpoint_agent/presets/mod.rs:95-122`) where each preset returns a `StreamSource` with a path and format enum variant, and the streaming infrastructure handles transcript loading separately. Update this section to describe the actual `StreamSource`/`StreamFormat` pattern. Reference existing presets like `amp.rs` or `pi.rs` for how they construct `StreamSource` objects.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Create `src/mdm/agents/my_agent.rs`: | ||
|
|
||
| ```rust | ||
| use crate::mdm::hook_installer::{HookInstaller, HookInstallerParams, HookCheckResult}; | ||
|
|
||
| pub struct MyAgentInstaller; | ||
|
|
||
| impl HookInstaller for MyAgentInstaller { | ||
| fn name(&self) -> &str { "My Agent" } | ||
| fn id(&self) -> &str { "my_agent" } | ||
| fn process_names(&self) -> Vec<&str> { vec!["my-agent", "myagent"] } | ||
|
|
||
| fn check_hooks(&self, params: &HookInstallerParams) -> Result<HookCheckResult, GitAiError> { | ||
| // Check if the agent is installed (binary, config dir, etc.) | ||
| let config_path = resolve_my_agent_config_path()?; | ||
|
|
||
| let tool_installed = config_path.exists() || binary_exists("my-agent"); | ||
| if !tool_installed { | ||
| return Ok(HookCheckResult::not_installed()); | ||
| } | ||
|
|
||
| // Check if git-ai hooks are already in the config | ||
| let hooks_installed = check_my_agent_hooks_installed(&config_path, params)?; | ||
| Ok(HookCheckResult { tool_installed, hooks_installed, hooks_up_to_date: hooks_installed }) | ||
| } | ||
|
|
||
| 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.
🚩 Add-agent SKILL.md MDM installer section may reference outdated HookInstaller trait API
The MDM installer section (Step 3, lines 183-221) describes a HookInstaller trait with methods name(), id(), process_names(), check_hooks(), install_hooks(), uninstall_hooks(). While I confirmed the installer files exist in src/mdm/agents/, I was unable to verify the exact trait signature before running out of investigation budget. Given how wrong the preset API section is, the installer trait API described here may also be outdated or fabricated. Reviewers should verify against the actual src/mdm/hook_installer.rs (or wherever HookInstaller is defined).
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| **Read-only commands short-circuit entirely**: `command_classification::is_definitely_read_only_invocation` bypasses the wrapper for `blame`, `log`, `status`, `diff`, `stash list`, `worktree list`, etc. — critical for performance with IDE git panels (thousands of calls/session). | ||
|
|
||
| **Daemon mode** (`feature_flags().async_mode`): wrapper becomes a pure git passthrough + daemon ping. Pre/post state is sent to the daemon which handles attribution asynchronously. |
There was a problem hiding this comment.
🚩 Architecture SKILL's daemon mode description references non-existent async_mode flag
Line 46 states 'Daemon mode (feature_flags().async_mode): wrapper becomes a pure git passthrough + daemon ping.' Since async_mode doesn't exist as a feature flag (see BUG-0003), this description of how daemon mode is activated is misleading. The actual mechanism for daemon mode should be verified — it may be controlled differently (perhaps always on in release, or via a different config mechanism). This could cause an AI agent to misunderstand how to enable/disable daemon mode during development or testing.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Load the transcript lazily and return `PromptUpdateResult::Updated(transcript, model)`. Register a transcript updater in `src/authorship/prompt_utils.rs`: | ||
|
|
||
| ```rust | ||
| "my_agent" => { | ||
| my_agent_preset::transcript_and_model_from_storage( | ||
| agent_metadata.get("transcript_path").map(String::as_str), | ||
| &agent_id.id, | ||
| agent_metadata.get("__test_storage_path").map(String::as_str), | ||
| ) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🚩 add-agent SKILL references non-existent PromptUpdateResult and transcript updater registration pattern
Lines 153-163 describe registering a transcript updater in src/authorship/prompt_utils.rs using a PromptUpdateResult::Updated(transcript, model) return type. While prompt_utils.rs does exist at that path, PromptUpdateResult does not exist anywhere in the codebase (verified via grep). The actual transcript loading mechanism likely uses a different pattern. An AI agent following this step would need to study the actual prompt_utils.rs implementation rather than the described pattern.
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
8f84ccc to
c70886f
Compare
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