Skip to content

docs: add agent PR workflow and invariants - #18

Merged
malpern merged 1 commit into
mainfrom
worktree-agent-workflow-docs
May 22, 2026
Merged

docs: add agent PR workflow and invariants#18
malpern merged 1 commit into
mainfrom
worktree-agent-workflow-docs

Conversation

@malpern

@malpern malpern commented May 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds docs/agent-pr-invariants.md — phase-based verification gates that define "done" for each stage of agent work (setup, development, PR, babysitting, merge, cleanup)
  • Adds docs/agent-pr-workflow.md — step-by-step reference for the typical path through those phases, including a cherry-pick-to-keypath/bundled phase
  • Adapted from KeyPath's proven agent workflow docs, tailored for Rust/cargo toolchain and the main / keypath/bundled two-branch model

Also updates CLAUDE.md (gitignored, local-only) to reference both docs and adds the "never merge main into keypath/bundled" rule explicitly.

Test plan

  • Review invariant gates match actual CI and build commands (cargo build, cargo test -p kanata-tcp-protocol)
  • Verify branch model documentation matches actual repo layout
  • Confirm non-negotiable rules align with pre-push hook behavior

Adapted from KeyPath's battle-tested agent workflow docs for the
kanata fork's Rust/cargo toolchain and two-branch model (main vs
keypath/bundled). Defines phase-based invariants, a cherry-pick gate
for the shipping branch, and non-negotiable safety rules.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@claude

claude Bot commented May 22, 2026

Copy link
Copy Markdown

Review

This PR is documentation-only (no Rust code, no API changes), so the standard code-review axes — Rust idioms, hot-path performance, bug surface, backward compatibility — don't really apply here. The relevant question is whether the workflow these docs encode matches the actual repo, and whether the invariants are strict enough to catch what they claim to.

Strengths

  • Invariants-over-checklists framing is the right call for agent workflows. The "self-healing" argument in agent-pr-invariants.md is well-stated.
  • The "What Can Go Wrong" table in agent-pr-workflow.md is the best part — concrete symptom → cause → prevention is exactly the format that helps an agent recover from a partial failure.
  • Fixes #NNN discipline (step 9) is good practice and easy to forget.
  • Separation of "what must be true" vs. "how to get there" between the two docs is clean.

Issues

1. The test invariant is too narrow for a keyboard engine. Both docs assert cargo test -p kanata-tcp-protocol as the bar for "tests pass." But this repo's CI (.github/workflows/rust.yml) actually runs cargo test --all across multiple feature combinations: default, --no-default-features, cmd, simulated_output + sim_tests, gui, plus Windows/macOS-specific feature sets. The TCP protocol crate is a tiny slice of the codebase — most logic lives in kanata, kanata-parser, kanata-keyberon, and the simulated-input tests. An agent that satisfies the current "After Development" gate can ship a regression in the hot path (event handling, parser, keyberon state machine) and still pass the invariant. Suggest either:

  • Tightening to cargo test --all (or at minimum cargo test --all --features=cmd), and
  • Adding cargo clippy --all -- -D warnings and cargo fmt --all --check — CI fails on both and they're cheap locally.

2. Pre-push hook is referenced but not in the repo. Both docs lean on "the pre-push hook blocks force-pushes to keypath/bundled" as enforcement. There's no pre-push under .git/hooks/ and nothing in the repo that would install one (no core.hooksPath, no install script). If the hook lives only in the maintainer's local checkout, the invariants framing is slightly misleading — a fresh worktree or a new contributor's clone has no such protection. Either ship the hook (e.g., a tracked hooks/pre-push plus a one-line setup note) or soften the language to "you have configured a pre-push hook that…".

3. Diverged-histories claim is load-bearing and unverified. The "never merge in either direction" rule rests on main and keypath/bundled having genuinely diverged histories. Worth a one-line note explaining why they diverged (e.g., keypath/bundled rebased onto a different base, or contains squashed fork commits), so a future agent doesn't "fix" the divergence by merging.

4. Pre-squashing step is optional, not required. Step 7 (git reset --soft <target-branch> && git commit) before pushing produces a single commit, but step 17 already uses gh pr merge --squash, which squashes server-side regardless of commit count on the branch. Pre-squashing is fine if you want the PR to show as one commit during review, but the "Single squashed commit on the branch" invariant under "After PR Creation" makes it sound mandatory when it's a style choice. Worth flagging as optional, or dropping the invariant.

5. Minor wording / consistency:

  • Invariants doc: "Cherry-Pick Gate (keypath/bundled)" — the parenthetical reads like a subtitle; consider just "Cherry-Pick Gate" since it's the only one.
  • Workflow doc step 20: ExitWorktree with action: "remove" and discard_changes: true mixes harness-tool syntax into a human-readable doc. If this is intentional (agent-facing), fine; if not, a plain-language version reads better.
  • The "Non-Negotiable Rules" block is duplicated near-verbatim between the two docs. Consider keeping it canonical in one (agent-pr-invariants.md is the natural home) and linking from the other to avoid drift.

6. Missing: how to recover from a partial state. The invariants define "done," but agents most often get stuck in the middle: PR merged but local branch not pulled, cherry-pick applied but push rejected, worktree exited with uncommitted changes. The "What Can Go Wrong" table touches this; a short "Recovery" section (or a column added to that table for "if you're already in this state, do X") would close the loop and match the invariants-are-self-healing philosophy.

Nits

  • agent-pr-workflow.md line for "Push the branch": git push -u origin <branch-name> — fine, but on subsequent force-pushes during iteration this needs --force-with-lease (safer than --force). Worth a one-line note in Phase 4 since steps 12–13 imply force-pushing.
  • Step 11 says "poll gh pr checks <number> until all checks complete." gh pr checks --watch exits when checks finish and is friendlier than a manual loop.

Verdict

Useful docs, and the invariants framing is a good fit for agent work. The main thing worth fixing before merge is the test-gate scope (item 1) — as written, an agent that follows this doc to the letter can land a hot-path regression with green local checks. Items 2 and 3 are smaller but worth a sentence each.

— Reviewed by Claude (automated code review)

@malpern
malpern merged commit 9d061bc into main May 22, 2026
1 of 2 checks passed
@malpern
malpern deleted the worktree-agent-workflow-docs branch May 22, 2026 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant