diff --git a/analysis/breaker/2025-02-05.md b/analysis/breaker/2025-02-05.md new file mode 100644 index 0000000..e1bfd4b --- /dev/null +++ b/analysis/breaker/2025-02-05.md @@ -0,0 +1,138 @@ +# Breaker Report: 2025-02-05 + +Focused on changes in commits b6d2582..9443f933 (past 10 commits). + +## Issue 1: JSON Escaping Bug in g3-research Script + +### Title +`g3-research` produces invalid JSON when query contains actual newlines + +### Repro +```bash +# In skills/research/g3-research, the write_status function uses: +escaped_query=$(echo -n "$query" | sed 's/\\/\\\\/g; s/"/\\"/g; s/\n/\\n/g') + +# Test with actual newlines: +QUERY=$'What is\nthe best\nRust library?' +escaped=$(echo -n "$QUERY" | sed 's/\\/\\\\/g; s/"/\\"/g; s/\n/\\n/g') +echo "{\"query\": \"$escaped\"}" | python3 -m json.tool +# Output: Invalid control character at: line 1 column 19 (char 18) +``` + +**Expected**: Valid JSON with `\n` escape sequences +**Actual**: Invalid JSON with literal newline characters + +### Diagnosis +- **File**: `skills/research/g3-research:66` +- **Root cause**: The sed pattern `s/\n/\\n/g` matches the literal two-character string `\n`, not actual newline characters. Sed processes line-by-line by default and doesn't see newlines in the pattern space. +- **Triggering condition**: User query contains actual newline characters (e.g., from multi-line input or programmatic construction) +- **Deterministic**: Yes + +### Impact +- **Severity**: Incorrect behavior - `status.json` becomes unparseable +- **Likelihood**: Uncommon but possible - queries are typically single-line, but multi-line queries from programmatic sources or copy-paste could trigger this + +### Fix +Replace sed with perl which handles newlines correctly: +```bash +escaped_query=$(echo -n "$query" | perl -pe 's/\\/\\\\/g; s/"/\\"/g; s/\n/\\n/g') +``` + +--- + +## Issue 2: Embedded Skill Path Not Readable + +### Title +Embedded skills have non-existent file paths that agents are instructed to `read_file` + +### Repro +``` +# When no repo skills/ directory exists, the research skill is loaded from embedded +# The generated prompt contains: + + research + ... + /SKILL.md + + +# The prompt instructs: +"read the full skill file using `read_file` to get detailed instructions" + +# Agent attempts: +read_file("/SKILL.md") +# Result: File not found error +``` + +**Expected**: Agent can read skill documentation +**Actual**: File path doesn't exist on disk + +### Diagnosis +- **File**: `crates/g3-core/src/skills/discovery.rs:97` - sets path to `/SKILL.md` +- **File**: `crates/g3-core/src/skills/prompt.rs:14-15` - instructs agent to use `read_file` +- **Root cause**: Embedded skills use a synthetic path marker, but the prompt doesn't account for this +- **Triggering condition**: User has no `skills/` directory in their repo (embedded skill not overridden) +- **Deterministic**: Yes + +### Impact +- **Severity**: Annoying - agent will fail to read skill docs and may hallucinate or ask for help +- **Likelihood**: Common for users outside the g3 repo itself + +### Possible Fixes +1. Include the full skill body in the prompt for embedded skills (increases prompt size) +2. Add special handling in `read_file` for `` paths +3. Change prompt to say "skill instructions are below" for embedded skills and inline the body + +--- + +## Issue 3: Hardcoded 'main' Branch in SDLC Pipeline + +### Title +`studio sdlc` assumes default branch is named 'main' + +### Repro +```bash +# In a repo where default branch is 'master': +studio sdlc run + +# has_commits_on_branch runs: +git rev-list --count main..sdlc/session-branch +# Fails silently (returns Ok(false)) because 'main' doesn't exist + +# merge_to_main runs: +git checkout main +# Fails with "Failed to checkout main" +``` + +**Expected**: Works with any default branch name +**Actual**: Fails or behaves incorrectly on repos using 'master' or other branch names + +### Diagnosis +- **File**: `crates/studio/src/main.rs:720` - `has_commits_on_branch()` hardcodes `main..{branch}` +- **File**: `crates/studio/src/git.rs` - `merge_to_main()` hardcodes `checkout main` +- **Root cause**: No detection of actual default branch name +- **Triggering condition**: Repository uses 'master' or custom default branch +- **Deterministic**: Yes + +### Impact +- **Severity**: Incorrect behavior - merge fails or skipped incorrectly +- **Likelihood**: Common - many repos still use 'master' + +### Fix +Detect default branch: +```bash +git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@' +# or +git config --get init.defaultBranch +``` + +--- + +## Summary + +| # | Issue | Severity | Likelihood | +|---|-------|----------|------------| +| 1 | JSON escaping with newlines | Incorrect behavior | Uncommon | +| 2 | Embedded skill path unreadable | Annoying | Common | +| 3 | Hardcoded 'main' branch | Incorrect behavior | Common | + +All issues are deterministic and reproducible. diff --git a/analysis/memory.md b/analysis/memory.md index 8c34033..316bef4 100644 --- a/analysis/memory.md +++ b/analysis/memory.md @@ -1,5 +1,5 @@ # Workspace Memory -> Updated: 2026-02-04T23:42:21Z | Size: 19.9k chars +> Updated: 2026-02-05T03:06:55Z | Size: 23.5k chars ### Remember Tool Wiring - `crates/g3-core/src/tools/memory.rs` [0..5000] - `execute_remember()`, `get_memory_path()`, `merge_memory()` @@ -412,3 +412,33 @@ allowed-tools: Bash Read # Optional/experimental # Skill Title Detailed instructions... ``` + +### Embedded Skills System +Skills are portable packages with SKILL.md + optional scripts, discovered from multiple locations. + +- `crates/g3-core/src/skills/embedded.rs` + - `EmbeddedSkill` [22..28] - name, skill_md content, scripts array + - `EMBEDDED_SKILLS` [32..42] - static array with include_str! for research skill + - `get_embedded_skill()` [48..50] - lookup by name + +- `crates/g3-core/src/skills/extraction.rs` + - `extract_script()` [28..85] - extracts embedded script to `.g3/bin/`, tracks version hash + - `needs_update()` [107..118] - compares stored hash vs current content + - `compute_hash()` [121..128] - DefaultHasher for version tracking + +- `crates/g3-core/src/skills/discovery.rs` + - `discover_skills()` [38..85] - scans 5 locations in priority order (embedded → global → extra → workspace → repo) + - `load_embedded_skills()` [88..102] - sets synthetic path `/SKILL.md` + - `is_embedded_skill()` [161..163] - checks if path starts with ` PathBuf { PathBuf::from(expanded.as_ref()) } -/// Check if a skill is from an embedded source. -pub fn is_embedded_skill(skill: &Skill) -> bool { - skill.path.starts_with(" Option<&'static EmbeddedSkill> { EMBEDDED_SKILLS.iter().find(|s| s.name == name) } -/// Get embedded skills as a map for easy lookup. -pub fn get_embedded_skills_map() -> HashMap<&'static str, &'static EmbeddedSkill> { - EMBEDDED_SKILLS.iter().map(|s| (s.name, s)).collect() -} - #[cfg(test)] mod tests { use super::*; @@ -78,10 +71,4 @@ mod tests { assert!(get_embedded_skill("research").is_some()); assert!(get_embedded_skill("nonexistent").is_none()); } - - #[test] - fn test_skills_map() { - let map = get_embedded_skills_map(); - assert!(map.contains_key("research")); - } }