From acef8903ae4bc2d03e394ef931aafb9a1b31dc0d Mon Sep 17 00:00:00 2001 From: ANonABento Date: Fri, 21 Aug 2026 20:49:59 -0400 Subject: [PATCH] fix(pipeline): scratch files leaked into agent commits; install a logger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found by actually running an agent from a column end to end, which is the only reason either was visible. The git exclude never applied in a worktree. `execute_spawn_cli` built the exclude path by joining `.git/info/exclude` onto the working dir, then guarded on `.exists()`. In a linked worktree `.git` is a *file*, not a directory, so that path never exists — the guard found nothing and silently skipped the whole exclusion. Every worktree-based agent run has therefore been committing `.task.md`, and `.agent.md` had just inherited the same fate: the first dogfood run produced a commit containing `.agent.md`, `.task.md`, `agent-proof.txt` and `math.js`, where only the last two belong. `git rev-parse --git-path info/exclude` resolves the real common-dir file for both a plain repo and a linked worktree. `exclude_from_git` now uses it, is idempotent per pattern, appends only what's missing, creates the parent directory, and no-ops outside a repo. Re-running the same pipeline afterwards produced a commit with only `agent-proof.txt` and `math.js`. Nothing was installing a logger. The `log` crate is a silent no-op until something registers one, and nothing did — no `tauri-plugin-log`, no `env_logger`, no `set_logger` anywhere. All 202 call sites across the app were discarded, including 19 `log::error!`. That is what made the first dogfood failure undiagnosable: a trigger that failed and a trigger that never fired looked identical from outside. `env_logger` now initializes in `run()`, defaulting to `kaitencode=warn` and controllable with `RUST_LOG`. Turning it on immediately showed the full trigger path — argv, working dir, tmux channel — and the rest of this session was debuggable as a result. Also records in the roadmap that critical-now is empty (the stale /usr/local/bin/claude is gone, verified), and notes the same duplicate-binary shape now present for codex: /usr/local/bin/codex is 0.130.0 while fnm's shim is 0.145.0, the version every codex behaviour here was verified against. The fnm path is per-shell and lives in tmpfs, so a reboot or a launcher start hands the app the stale one. Checked: not currently breaking anything — both carry the flags the app uses and both have `resume --last`. --- .tickets/_docs/ROADMAP.md | 28 ++++-- Cargo.lock | 106 ++++++++++++++++++++ src-tauri/Cargo.toml | 4 + src-tauri/src/lib.rs | 11 +++ src-tauri/src/pipeline/triggers.rs | 153 +++++++++++++++++++++++------ 5 files changed, 266 insertions(+), 36 deletions(-) diff --git a/.tickets/_docs/ROADMAP.md b/.tickets/_docs/ROADMAP.md index 930c461c..dbe01e1d 100644 --- a/.tickets/_docs/ROADMAP.md +++ b/.tickets/_docs/ROADMAP.md @@ -63,11 +63,10 @@ replaces the terminal. --- -## 🔴 Critical now (blocks comfortable daily-driving) +## 🔴 Critical now -1. **Two-claude footgun (env hygiene).** Stale `/usr/local/bin/claude` (2.1.138 npm - copy) can win in some PATHs. App now resolves absolute paths correctly, but - `rm /usr/local/bin/claude` removes the ambiguity. One-liner, do it. +Empty. The last item — the stale `/usr/local/bin/claude` — was removed +2026-08-21; `which -a claude` now returns exactly one path (2.1.239). ## 🟡 Important (rough edges, not blockers) @@ -92,6 +91,22 @@ replaces the terminal. needs an `/api/*` route first. Add as needed (the checklist spec establishes the route+tool pattern). +## 🟢 Env hygiene (latent, not breaking) + +- **Two `codex` copies**, the same shape as the claude footgun that was just + closed: `/usr/local/bin/codex` is **0.130.0** (root-owned, stale) while fnm's + shim is **0.145.0** — the version every codex behaviour in this repo was + verified against. The app resolves via `which`, so it picks whichever the + launching shell's PATH puts first. Today the running instance gets 0.145.0 + through an fnm shim under `/run/user/1000/…`, but that path is tmpfs, is + per-shell-session, and vanishes on reboot or when launched from a desktop + launcher — at which point `/usr/local/bin/codex` wins. + **Checked 2026-08-21: not currently breaking anything.** Both versions carry + `--skip-git-repo-check` and `--dangerously-bypass-approvals-and-sandbox`, and + both have the `resume` subcommand with `--last`. So this is version ambiguity + waiting to bite, not a live bug. `sudo rm /usr/local/bin/codex` closes it the + same way the claude one was closed (root-owned, needs your own shell). + ## ⚪ Inert / deferred (known, intentional) - Custom keyboard shortcuts render but are inert (`shortcuts-tab.tsx:54`). @@ -118,5 +133,6 @@ replaces the terminal. ## Suggested order -`#1 rm stale claude` (free) → `#3 effort wiring` → `#4 checklist/roadmap MCP` → -`#5 MCP gaps`. `#2` is cosmetic cleanup; do it opportunistically. +Nothing is critical. Next by value: **dogfood agents-in-columns** on a real +board → `#3 effort wiring` → `#4 checklist/roadmap MCP` → `#5 MCP gaps`. `#2` +(inert settings surfaces) and the codex env-hygiene note are opportunistic. diff --git a/Cargo.lock b/Cargo.lock index 69438ef4..f4d8074d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -873,6 +873,37 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c87e182de0887fd5361989c677c4e8f5000cd9491d6d563161a8f3a5519fc7f" +[[package]] +name = "defmt" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2953bfe4f93bbd20cc71198842756f77d161884c99ebbabc41d80231ded88d1" +dependencies = [ + "bitflags 1.3.2", + "defmt-macros", +] + +[[package]] +name = "defmt-macros" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bad9c72e7ca2137e0dc3813245a0d282fd6daad32fd800af018306a9169b5fe8" +dependencies = [ + "defmt-parser", + "proc-macro2", + "quote", + "syn 2.0.117", +] + +[[package]] +name = "defmt-parser" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10d60334b3b2e7c9d91ef8150abfb6fa4c1c39ebbcf4a81c2e346aad939fee3e" +dependencies = [ + "thiserror 2.0.18", +] + [[package]] name = "deranged" version = "0.5.8" @@ -1121,6 +1152,29 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "env_filter" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "900d271a03799a1ee8d1ca9b19893b48ca674a9284fefcfb85f05e74ed314217" +dependencies = [ + "log", + "regex", +] + +[[package]] +name = "env_logger" +version = "0.11.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de671bd27a75a797dc9ae289ba1e77276e75e2026408aab65185384e2d5cd3f6" +dependencies = [ + "anstream", + "anstyle", + "env_filter", + "jiff", + "log", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -2188,6 +2242,42 @@ dependencies = [ "system-deps", ] +[[package]] +name = "jiff" +version = "0.2.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "668b7183bd07af9a4885f5c35b0cc5c83c4607a913c16b7e17291832910d2dcc" +dependencies = [ + "defmt", + "jiff-core", + "jiff-static", + "log", + "portable-atomic", + "portable-atomic-util", + "serde_core", +] + +[[package]] +name = "jiff-core" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7feca88439efe53da3754500c1851dedf3cb36c524dd5cf8225cc0794de95d09" +dependencies = [ + "defmt", +] + +[[package]] +name = "jiff-static" +version = "0.2.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a69dcb3a21cfb32ce1cd056169337ca284af0766dd766e7878819b251a49204" +dependencies = [ + "jiff-core", + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "jni" version = "0.21.1" @@ -2285,6 +2375,7 @@ dependencies = [ "chrono", "cpal", "dirs 5.0.1", + "env_logger", "futures", "git2", "glob", @@ -3319,6 +3410,21 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "portable-atomic" +version = "1.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05c8b63e8d9609db387f0324918f81d68fe27748f084ef092fb35954d0539a85" + +[[package]] +name = "portable-atomic-util" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a106d1259c23fac8e543272398ae0e3c0b8d33c88ed73d0cc71b0f1d902618" +dependencies = [ + "portable-atomic", +] + [[package]] name = "potential_utf" version = "0.1.5" diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 7da29242..e8bb4034 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -30,6 +30,10 @@ git2 = "0.19" axum = "0.8" dirs = "5" log = "0.4" +# Makes the app's existing log:: diagnostics visible. Without a logger +# installed the `log` crate is a silent no-op, so every warn!/info! in the +# pipeline and chat layers went nowhere. Off unless RUST_LOG is set. +env_logger = "0.11" # Local whisper transcription (optional - requires macOS 10.15+) whisper-rs = { version = "0.15", optional = true } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index b22af202..4690d996 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -32,6 +32,17 @@ use whisper::AudioRecorder; #[cfg_attr(mobile, tauri::mobile_entry_point)] pub fn run() { + // Install a logger so the app's `log::` diagnostics are actually emitted. + // The `log` crate is a silent no-op until something registers a logger, so + // until now every warn!/info! in the pipeline, chat and trigger layers went + // nowhere — which made a failing trigger indistinguishable from one that + // never fired. Quiet by default; `RUST_LOG=kaitencode=debug` turns it on. + env_logger::Builder::from_env( + env_logger::Env::default().default_filter_or("kaitencode=warn"), + ) + .format_timestamp_millis() + .init(); + let conn = db::init().expect("Failed to initialize database"); // Clear stale cli_session_id references (previous app sessions are dead) diff --git a/src-tauri/src/pipeline/triggers.rs b/src-tauri/src/pipeline/triggers.rs index 9bbb8890..ad22813f 100644 --- a/src-tauri/src/pipeline/triggers.rs +++ b/src-tauri/src/pipeline/triggers.rs @@ -1057,6 +1057,71 @@ pub(crate) fn resolve_model_override( .map(ToString::to_string) } +/// Add patterns to the repo's git exclude file, so trigger scratch files +/// (`.task.md`, `.agent.md`) don't end up in the agent's commit. +/// +/// The path has to come from git, not from string-joining `.git/info/exclude` +/// onto the working dir. In a **linked worktree `.git` is a file**, not a +/// directory, so that join names something that never exists — the old code +/// checked `.exists()`, found nothing, and silently skipped the whole +/// exclusion. Every worktree-based agent run has been committing `.task.md` +/// as a result. `rev-parse --git-path` resolves to the real common-dir file. +fn exclude_from_git(working_dir: &str, patterns: &[&str]) { + if working_dir.is_empty() { + return; + } + let Ok(output) = std::process::Command::new("git") + .args(["-C", working_dir, "rev-parse", "--git-path", "info/exclude"]) + .output() + else { + return; + }; + if !output.status.success() { + return; + } + let raw = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if raw.is_empty() { + return; + } + // `--git-path` answers relative to the -C directory when the path is inside + // the repo it was asked from. + let path = { + let p = PathBuf::from(&raw); + if p.is_absolute() { + p + } else { + Path::new(working_dir).join(p) + } + }; + + let existing = std::fs::read_to_string(&path).unwrap_or_default(); + let missing: Vec<&str> = patterns + .iter() + .copied() + .filter(|pat| !existing.lines().any(|line| line.trim() == *pat)) + .collect(); + if missing.is_empty() { + return; + } + + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(parent); + } + let mut next = existing.trim_end().to_string(); + if !next.is_empty() { + next.push('\n'); + } + next.push_str(&missing.join("\n")); + next.push('\n'); + if let Err(e) = std::fs::write(&path, next) { + log::warn!( + "[triggers] Could not update git exclude at {}: {}", + path.display(), + e + ); + } +} + /// Look up the skills an agent references, dropping ids that no longer exist. /// /// Deliberately lenient: a deleted skill shows in the dossier as "missing @@ -1841,36 +1906,15 @@ fn execute_spawn_cli( } } - // Exclude .task.md from git (avoid agent committing it) - let exclude_path = std::path::Path::new(&working_dir) - .join(".git") - .join("info") - .join("exclude"); - if exclude_path.exists() { - if let Ok(content) = std::fs::read_to_string(&exclude_path) { - if !content.contains(".task.md") { - let _ = std::fs::write( - &exclude_path, - format!( - "{}\n.task.md\n.task-handoff.md\n{}\n", - content.trim_end(), - roster::plan::AGENT_INSTRUCTIONS_FILE - ), - ); - } else if !content.contains(roster::plan::AGENT_INSTRUCTIONS_FILE) { - // Worktrees created before agents existed already list - // .task.md, so the branch above never fires for them. - let _ = std::fs::write( - &exclude_path, - format!( - "{}\n{}\n", - content.trim_end(), - roster::plan::AGENT_INSTRUCTIONS_FILE - ), - ); - } - } - } + // Keep the scratch files out of git. + exclude_from_git( + &working_dir, + &[ + ".task.md", + ".task-handoff.md", + roster::plan::AGENT_INSTRUCTIONS_FILE, + ], + ); } // `runtime_mode` (resolved above) is already normalized: @@ -3525,6 +3569,55 @@ mod tests { db::update_column(conn, column_id, None, None, None, None, None, Some(&json)).unwrap(); } + #[test] + fn exclude_from_git_writes_to_the_path_git_reports() { + // The bug this replaced: joining `.git/info/exclude` onto the working + // dir. In a linked worktree `.git` is a FILE, so that path never + // exists and the exclusion was silently skipped — every worktree run + // committed `.task.md`. + let dir = std::env::temp_dir().join(format!("kc-excl-{}", Uuid::new_v4())); + std::fs::create_dir_all(&dir).unwrap(); + let ok = std::process::Command::new("git") + .args(["-C", dir.to_str().unwrap(), "init", "-q"]) + .status() + .map(|s| s.success()) + .unwrap_or(false); + if !ok { + return; // no git on this machine; nothing to assert + } + + let repo = dir.to_string_lossy().to_string(); + exclude_from_git(&repo, &[".task.md", ".agent.md"]); + + let exclude = dir.join(".git").join("info").join("exclude"); + let body = std::fs::read_to_string(&exclude).unwrap(); + assert!(body.lines().any(|l| l.trim() == ".task.md"), "{}", body); + assert!(body.lines().any(|l| l.trim() == ".agent.md"), "{}", body); + + // Idempotent: running again must not duplicate entries. + exclude_from_git(&repo, &[".task.md", ".agent.md"]); + let again = std::fs::read_to_string(&exclude).unwrap(); + assert_eq!(again.matches(".task.md").count(), 1, "{}", again); + + // A pattern not yet present is appended without disturbing the rest. + exclude_from_git(&repo, &[".task.md", "brand-new.md"]); + let third = std::fs::read_to_string(&exclude).unwrap(); + assert!(third.lines().any(|l| l.trim() == "brand-new.md")); + assert_eq!(third.matches(".agent.md").count(), 1); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn exclude_from_git_is_a_no_op_outside_a_repo() { + let dir = std::env::temp_dir().join(format!("kc-norepo-{}", Uuid::new_v4())); + std::fs::create_dir_all(&dir).unwrap(); + exclude_from_git(&dir.to_string_lossy(), &[".task.md"]); + assert!(!dir.join(".git").exists()); + exclude_from_git("", &[".task.md"]); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn columns_using_agent_sweeps_every_workspace() { // Agents are global, so deleting one has to account for boards the