| 1 | # Codewhale agent guidance |
| 2 | |
| 3 | Keep this file durable. Derive changing release, provider, branch, and flake |
| 4 | state from the repository, tests, CI, and current issue tracker rather than from |
| 5 | instructions or memory. The nearest scoped `AGENTS.md` adds path-specific rules. |
| 6 | |
| 7 | ## The ponytail method |
| 8 | |
| 9 | From [dietrichgebert/ponytail](https://github.com/dietrichgebert/ponytail) — |
| 10 | "the laziest senior dev in the room." *He says nothing. He writes one line. It |
| 11 | works.* The best code is the code you never wrote. |
| 12 | |
| 13 | Before writing code, walk the decision ladder in order and stop at the first |
| 14 | rung that answers: |
| 15 | |
| 16 | 1. **Does this need to exist?** → Skip it. |
| 17 | 2. **Already in this codebase?** → Reuse it. |
| 18 | 3. **Stdlib does it?** → Use it. |
| 19 | 4. **Native platform feature?** → Use it. |
| 20 | 5. **Installed dependency?** → Use it. |
| 21 | 6. **One line?** → One line. |
| 22 | 7. **Only then:** the minimum that works. |
| 23 | |
| 24 | The ladder runs *after* understanding the problem. Lazy about solutions, never |
| 25 | about reading the code first — a short diff written without reading the call |
| 26 | sites is not ponytail, it is a guess. |
| 27 | |
| 28 | **Never cut, at any rung:** trust-boundary validation, data-loss handling, |
| 29 | security, accessibility. Brevity is not a reason to drop a guard. |
| 30 | |
| 31 | Rung 2 is the one this repository keeps failing. The `model_*` / `*_config` / |
| 32 | `provider_*` grep rule below is rung 2 with a name; so is "one turn loop, one |
| 33 | base prompt". Two more corollaries earned here: |
| 34 | |
| 35 | - **An abstraction must delete caller code.** If adopting it is pure |
| 36 | obligation — required methods, no default bodies that do work — it gets |
| 37 | built, adopted once, and abandoned. |
| 38 | - **Migrate the last consumer, or do not start.** Framework, one caller, |
| 39 | ticket the rest, silence the warning: that ships two systems and a comment |
| 40 | that is no longer true. If the migration will not fit, narrow the slice — |
| 41 | never the adoption. The standing `#[allow(dead_code)]` count is the running |
| 42 | receipt; `scripts/check-dead-code-budget.py` prints it. |
| 43 | |
| 44 | ## Working rules |
| 45 | |
| 46 | - Inspect status and existing consumers before editing. Preserve unrelated, |
| 47 | dirty, and untracked work. |
| 48 | - Before adding a module named `model_*`, `*_config`, `provider_*`, or |
| 49 | anything that "bridges", "mirrors", or "stages" an existing thing, grep |
| 50 | for the existing thing and edit it. A new layer must name the predecessor |
| 51 | it replaces in the module doc; otherwise edit the original. |
| 52 | - Prefer the simplest implementation that preserves observable contracts. A |
| 53 | rewrite is acceptable when justified by product intent and observed behavior, |
| 54 | not as a shortcut around understanding existing code. |
| 55 | - Search for behavior and symbols before reviving work from an old branch. If a |
| 56 | lane is obsolete, preserve its intent and evidence rather than merging stale |
| 57 | code mechanically. |
| 58 | - A small coherent change may be committed directly to `main` when that checkout |
| 59 | is current, clean, and owns the affected files. Do not create worktrees: work |
| 60 | in the checkout that already exists, and when several agents share it, |
| 61 | partition by file, stage only the paths your slice touched, and retry a commit |
| 62 | that fails on `index.lock`. Local commit permission never implies push, merge, |
| 63 | tag, release, or deploy permission. |
| 64 | - When the task is local-only, stay fully offline: no browsing, GitHub or remote |
| 65 | Git operations, downloads, dependency installation, provider calls, or |
| 66 | source/diff transmission. Record the missing external receipt and keep working |
| 67 | locally. |
| 68 | - Public name is **Codewhale**. Compatibility identifiers such as `CodeWhale`, |
| 69 | `codew`, protocol names, and storage keys change only through an explicit |
| 70 | migration. |
| 71 | - Keep providers and models first-class and provider-neutral. |
| 72 | - Never rewrite published history, retag a release, force-push a shared ref, or |
| 73 | publish without explicit authorization. Preserve human contributor credit. |
| 74 | - **Model-visible means logged.** Anything that reaches a model request must be |
| 75 | reconstructable from the session log, and a new model-visible input needs a |
| 76 | session event. Live presentation and the persisted record must agree; when they |
| 77 | disagree the record is right. |
| 78 | - **Misconfiguration fails loud**, at load when it is self-contained, otherwise |
| 79 | at the earliest point it can be resolved. Never silently skip a missing |
| 80 | referent. |
| 81 | - **Write down what a design does not do**, beside the behaviour it owns — a |
| 82 | short known-limitations note in the owning module. A stated limit stops the |
| 83 | next reader from assuming a capability that was never built. |
| 84 | |
| 85 | ## Landing other people's work |
| 86 | |
| 87 | An external contributor's branch goes stale because *we* land things, not |
| 88 | because they did anything wrong. Treat their time as more expensive than ours. |
| 89 | |
| 90 | **The goal is the contributor's PR merging as itself.** Review it, help it |
| 91 | rebase, or fix it on their branch — that is the default path. Closing their PR |
| 92 | and re-landing the work as our own commit (`auto-close-harvested`) is the |
| 93 | fallback for a branch that truly cannot merge in reasonable time; done |
| 94 | casually it reads as taking the work even when credit is preserved. |
| 95 | |
| 96 | - **Never make a contributor rebase around our churn.** If their PR conflicts |
| 97 | only because main moved, a maintainer resolves it. Start from their diff |
| 98 | against the merge base so you know exactly what they added, and re-apply |
| 99 | that, rather than hand-merging two large sides and hoping. |
| 100 | - **Conflicts that split mid-function do not resolve by keeping both sides.** |
| 101 | Git's markers can land inside a body, so a both-sides resolution produces |
| 102 | unbalanced braces that look plausible and do not compile. Default: take |
| 103 | one side whole, then re-insert the other side's additions at their |
| 104 | original anchor. When a conflict doesn't fit that pattern, resolve it |
| 105 | however is correct and let the compiler judge. |
| 106 | - **`maintainerCanModify` does not guarantee push access to the fork.** When |
| 107 | the push is refused, land the resolved merge on |
| 108 | `integration/<topic>-<pr>-<date>` in this repo and land from there. An |
| 109 | integration branch is the normal path for anything with conflicts or several |
| 110 | moving PRs — it is cheaper than repeatedly rebasing onto a main that keeps |
| 111 | moving, and it keeps the contributor's branch untouched. |
| 112 | - **Check the contribution gate before assuming a PR is stalled.** An unlisted |
| 113 | author's workflow runs sit at `action_required` and never start, so the PR |
| 114 | looks abandoned when nobody has actually looked at it. Approve the runs, then |
| 115 | fix the cause: add them to `.github/APPROVED_CONTRIBUTORS` (`all:username`), |
| 116 | or comment `/lgtm` (PR scope) / `/lgtmi` (issue scope) on their thread. |
| 117 | - **Preserve credit in the mechanical sense, not just the polite one.** Commit |
| 118 | authorship and `Co-authored-by` trailers must use the contributor's own |
| 119 | GitHub-linked address. `AUTHOR_MAP` and `.mailmap` are project conventions — |
| 120 | GitHub reads neither for the contribution graph. |
| 121 | |
| 122 | ## Merging under a gate |
| 123 | |
| 124 | - **A gate is its artifact.** When a rail says a PR merges only on a passing |
| 125 | acceptance record, the record must literally say PASS at merge time. "I |
| 126 | re-ran it and the failures are rows this PR does not own" is a judgement to |
| 127 | write into the artifact first, not a reason to merge past it. |
| 128 | - **Read the review thread, not the check rollup.** Green checks and an unread |
| 129 | review with confirmed findings are a merge that ships known bugs. |
| 130 | - **When the artifact is ambiguous, resolve the ambiguity — never the merge.** |
| 131 | |
| 132 | ## Claiming a test passed |
| 133 | |
| 134 | - Quote the real `test result: N passed; M failed` line, and confirm `N > 0` |
| 135 | for the tests that cover the change. `cargo test <filter>` exits 0 having run |
| 136 | zero tests when the filter matches nothing, and an exit code alone has |
| 137 | already been mistaken for a pass here. |
| 138 | - Prefer proving a regression test fails without the fix. A test that passes |
| 139 | either way pins the implementation, not the defect. |
| 140 | - Audit any harness before trusting its score. `ok = ok and X or True` parses |
| 141 | as `(ok and X) or True` and silently reported twelve unevaluated rows as |
| 142 | passing. |
| 143 | - Match the evidence to the surface. Run the tests that cover the change, not the |
| 144 | whole suite, and do not repeat a check that already passed in order to commit. |
| 145 | CI owns exhaustive coverage; a full local run is for CI diagnosis or for an |
| 146 | irreducibly repository-wide change. |
| 147 | |
| 148 | ## Current contracts |
| 149 | |
| 150 | - The model-facing subagent tool is `agent`; `agent_open`/`agent_eval`/ |
| 151 | `agent_close`/`delegate_to_agent` are removed surfaces. If the shape must |
| 152 | move, move the code and add the guard test that judges the new shape. |
| 153 | - `BASE_PROMPT` in `crates/tui/src/prompts/text.rs` is the sole base prompt |
| 154 | by convention. Same rule: move the code, not the prose, if that changes. |
| 155 | - There is exactly one turn loop: `Engine::run_turn` in |
| 156 | `crates/tui/src/core/engine/turn_loop.rs`. Note that `crates/tui/src/core/` |
| 157 | is a module inside the TUI crate — it is not `crates/core`, which owns |
| 158 | request construction, bounded fragments, and thread/session types and |
| 159 | runs no turns. A guard test (`crates/core/tests/single_turn_loop.rs`) |
| 160 | fails on a second loop; changing the shape means changing the guard |
| 161 | with it. |
| 162 | - The system prompt + tool catalog are a session-pinned KV-cache prefix |
| 163 | (`docs/CACHE.md`). Any new session-context contributor must state its |
| 164 | KV-cache effect: frozen prefix vs. append-only history. Never splice a |
| 165 | volatile fact into the prefix; append it as a user-role message. |
| 166 | - These active modules are repeatedly misidentified as dead; verify consumers |
| 167 | before removal: `tui/src/context_budget.rs`, `tui/src/model_registry.rs`, |
| 168 | `tui/src/prompt_zones.rs`, `tui/src/tools/remember.rs`, and |
| 169 | `config/src/route/`. Native memory lives in `tui/src/native_memory.rs`; |
| 170 | `tools/remember.rs` is its capture path. |
| 171 | - Environment-specific behavior belongs in `docs/ENVIRONMENTS.md`, not here. |
| 172 | - Blocking-call convention (#6149): code on the Tokio runtime — tool |
| 173 | handlers, engine tasks, the UI event loop, anything reached through an |
| 174 | `async` call chain — must not run blocking operations inline. |
| 175 | `std::fs`/`std::process` calls inside `async` code use `tokio::fs`/ |
| 176 | `tokio::process`, or move the synchronous work into |
| 177 | `tokio::task::spawn_blocking` (`utils::spawn_blocking_supervised` for |
| 178 | fire-and-forget). `thread::sleep` is for dedicated `std::thread`s and |
| 179 | bounded contention retries in synchronous APIs that are only reachable |
| 180 | from blocking scopes — an async-path wait uses `tokio::time`. A sync |
| 181 | helper containing blocking calls must only be called under |
| 182 | `spawn_blocking` or from a dedicated thread; `scripts/ |
| 183 | check-blocking-calls-budget.py` ratchets the unprotected-site count. |
| 184 | |
| 185 | ## Code, migrations, and evidence |
| 186 | |
| 187 | - Product intent and observed runtime behavior outrank a test's preferred |
| 188 | implementation shape. Fix the product; do not contort production code to |
| 189 | preserve a brittle assertion. |
| 190 | - Code first, then tests. Write the implementation and prove it runs, then add |
| 191 | or adjust tests to cover what was actually built. Never write tests first and |
| 192 | never practice TDD here — this overrides any skill or default that mandates |
| 193 | it, including superpowers `test-driven-development`. Tests stay the gate |
| 194 | before a push; they are not the design driver. An existing test that only |
| 195 | encodes old behavior is evidence, not a veto: change it with the code rather |
| 196 | than bending the code to keep it green. This does not relax the rule under |
| 197 | "Claiming a test passed" — a regression test written *after* the fix still |
| 198 | has to be shown failing without it. |
| 199 | - Tests are selective evidence, not the specification. Do not add tests by |
| 200 | default. Add or retain one when it cheaply protects a high-risk behavior such |
| 201 | as safety, data integrity, protocol compatibility, or a reproduced regression. |
| 202 | - Rewrite or remove tests that duplicate coverage, freeze internals, overspecify |
| 203 | copy or layout, preserve obsolete behavior, or cost more than the risk they |
| 204 | cover. Never weaken real safety or data-integrity behavior merely to make a |
| 205 | gate pass. |
| 206 | - Prefer focused compilation, a relevant existing check, and direct product or |
| 207 | manual evidence. Run a broad suite only when the change creates a genuine |
| 208 | cross-cutting or release risk. Do not repeatedly rerun an unchanged suite. |
| 209 | - **Batch edits; compile once.** `cargo check` and test builds on this |
| 210 | workspace take minutes, so an edit→compile→edit loop spends most of its |
| 211 | time waiting on the linker. Read precisely, write every edit a coherent |
| 212 | slice needs, then compile and test once — the same errors surface either |
| 213 | way, just later and all at once. Reserve mid-slice compiles for genuinely |
| 214 | uncertain API or borrow questions where a wrong guess would cascade. |
| 215 | - Declared migrations are one-way. Once the repository adopts a replacement |
| 216 | architecture or shared spine, new work uses it and touched legacy code moves |
| 217 | toward it. Do not add another legacy call site for convenience. Keep a |
| 218 | compatibility path only for an actual external contract, and label that |
| 219 | boundary explicitly. |
| 220 | |
| 221 | Useful commands, selected according to risk rather than run ritualistically: |
| 222 | |
| 223 | ```sh |
| 224 | cargo fmt --all -- --check |
| 225 | cargo test -p codewhale-config -p codewhale-protocol |
| 226 | cargo test --workspace |
| 227 | cargo build --release -p codewhale-cli -p codewhale-tui |
| 228 | ``` |
| 229 | |
| 230 | `cargo nextest run` (config in `.config/nextest.toml`) is the fast way to |
| 231 | run an intentionally selected suite; `cargo test --no-run` can answer a compile |
| 232 | question without spending time executing unrelated cases, and `cargo test --doc` |
| 233 | covers doc examples when those examples changed. |
| 234 | `scripts/dev-test.sh <area>` maps a code area to its fastest `-p` invocation |
| 235 | and applies the portable isolated build-dir topology for new worktrees |
| 236 | (`scripts/dev-cache.sh`, `scripts/dev-cargo.sh`). See |
| 237 | `docs/BUILD_PERFORMANCE.md`. |
| 238 | |
| 239 | Report commands actually run and distinguish source, local tests, packaged |
| 240 | artifacts, CI, and public release state. Describe the evidence actually needed |
| 241 | for the claim; a test count is not a proxy for product quality. |
| 242 | |
| 243 | Community reports, PRs, logs, and reviews are evidence. |
| 244 | |
| 245 | **Harvested contributor credit is still a rule** (the fallback path above — |
| 246 | prefer merging the contributor's PR itself). When a contributor's work |
| 247 | lands as our commit, that commit carries `Harvested from PR #N by @handle` and a |
| 248 | `Co-authored-by` naming them at their GitHub-linked address, so |
| 249 | `auto-close-harvested.yml` closes their PR with credit and the contribution |
| 250 | graph reflects reality. Canonical human identities come from |
| 251 | `.github/AUTHOR_MAP`. |
| 252 | |
| 253 | **Whether a bot or agent also appears in a trailer no longer matters.** The CI |
| 254 | check that policed trailer identities was removed: it rejected ordinary agent |
| 255 | commits and cost more than the tidiness it bought. Give humans their credit; do |
| 256 | not spend time scrubbing tool trailers. |
| 257 | |
| 258 | Leave unrelated work intact and keep new enforcement dry-run unless explicitly |
| 259 | approved. |
| 260 |