From a3232682dec302650fd04a0c7e158d3598daabe5 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah Date: Mon, 8 Jun 2026 23:10:48 -0700 Subject: [PATCH] Document Base agent workflow guardrails --- .github/pull_request_template.md | 2 + AGENTS.md | 11 +++++ docs/github-workflow.md | 40 +++++++++++++++++ docs/testing.md | 21 +++++++++ skills.md | 75 +++++++++++++++++++++++++++++++- 5 files changed, 147 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 24be7f5..fa166d1 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -23,7 +23,9 @@ Closes # - [ ] Branch name follows `/--`. - [ ] PR is scoped to one issue, unless a documented multi-issue exception applies. - [ ] PR body explains what changed and how it was validated. +- [ ] Validation commands were run from the current checkout or worktree, or unavailable checks are explained. - [ ] Relevant BATS and Python tests pass. +- [ ] Bug fixes include regression proof or a documented reproduction when practical. - [ ] Documentation is updated when behavior or user-facing commands change. - [ ] PR includes `Fixes #` or `Closes #` when it should close the issue. - [ ] `Demo Impact` is meaningful for `needs-demo` work, or explicitly says `None.` diff --git a/AGENTS.md b/AGENTS.md index 2ba84ea..5dabb34 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,6 +10,9 @@ them. - Keep Base focused as the shared developer workspace control plane. - Keep project-specific setup, service code, and application behavior in the owning project repository unless Base is explicitly the right shared layer. +- Adopt external agent workflow ideas only after translating them into + Base-specific guidance. Do not vendor or require a third-party methodology + when a smaller Base-native rule is enough. - When the user explicitly says a session is design-only or asks for no code changes, stay in discussion mode and do not edit files. - Surface unresolved product or architecture decisions instead of silently @@ -29,6 +32,8 @@ them. - Branch from `origin/master` with `/--`. - Use a dedicated worktree under `~/work/base-worktrees/` for PR work. +- Before creating a worktree, check whether the current checkout is already a + linked worktree for the intended issue. - Link PRs with `Fixes #` or `Closes #` when merge should close the issue. - After merge, sync `master`, remove the worktree, and delete local and remote @@ -41,6 +46,10 @@ milestones, GitHub Projects, and cleanup rules. - Run the narrowest relevant checks first, then broaden when shared behavior is touched. +- For bug fixes, reproduce the symptom and identify the root cause before + changing code. Prefer one focused hypothesis and one focused fix at a time. +- Do not claim work is fixed or complete without fresh verification output from + the current checkout or worktree. - For documentation-only changes, run `git diff --check`. - For general Base changes, run `basectl test base` and `git diff --check`. - For shell changes, include the relevant BATS tests and ShellCheck when @@ -52,6 +61,8 @@ milestones, GitHub Projects, and cleanup rules. `docs/testing.md`. - If a required check cannot be run locally, say so in the PR and final summary. +- For review feedback, verify the suggestion against Base's architecture, + product boundaries, and existing tests before implementing it. ## Change Boundaries diff --git a/docs/github-workflow.md b/docs/github-workflow.md index 8a80ce7..b9d2a1e 100644 --- a/docs/github-workflow.md +++ b/docs/github-workflow.md @@ -92,6 +92,24 @@ Issue work should use: ~/work/base-worktrees/ ``` +Before creating a worktree, check whether the current checkout is already a +linked worktree for the intended issue: + +```bash +git rev-parse --git-dir +git rev-parse --git-common-dir +git branch --show-current +git rev-parse --show-superproject-working-tree +``` + +When the git directory differs from the common directory, and the checkout is +not a submodule, the current checkout is already a linked worktree. A +non-empty `--show-superproject-working-tree` result means the checkout is a +submodule and should be treated as a normal repository for worktree detection. +Continue in an existing issue worktree instead of creating a nested or +duplicate worktree. If the checkout is a normal repository, create the issue +worktree from current `origin/master`. + Create a worktree from current `origin/master`: ```bash @@ -100,6 +118,10 @@ git worktree add -b documentation/241-20260529-document-github-workflow \ ~/work/base-worktrees/documentation-241-github-workflow origin/master ``` +Keep the worktree while the pull request is open so review feedback can be +handled on the same branch. Cleanup happens after merge, or after an explicit +discard decision. + After a pull request is merged: ```bash @@ -178,6 +200,24 @@ of adding a separate PR item to the Project. PR reviewers are PR-specific and should be selected from the implementation surface, ownership, and risk of the change rather than copied from the issue. +### Review Feedback + +Review feedback should be handled as technical input, not as an automatic patch +queue. Before implementing a suggestion, check whether it is correct for Base's +architecture, product boundaries, supported platforms, and current tests. + +Implement clear, correct feedback directly. Push back or ask for clarification +when feedback would: + +- move project-specific behavior into Base +- make `check` or `doctor` mutate local state +- broaden a narrow PR into an architecture change +- conflict with existing command contracts or validation rules +- add unused behavior that is not required by the issue + +Fix one review item or related group at a time, then rerun the narrowest +verification that proves the change. + ### Multi-Issue PRs One PR may close multiple issues only when the work is atomic and splitting it diff --git a/docs/testing.md b/docs/testing.md index be02ab8..3748685 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -3,6 +3,27 @@ Base uses three test layers. Prefer the narrowest layer that proves the behavior, then broaden when a change crosses command or runtime boundaries. +## Regression And Completion Evidence + +Bug fixes should start with a failing test, fixture, or reproduction whenever +practical. The useful proof is not just that the final test passes, but that the +test or reproduction failed for the expected reason before the fix. + +Use this order for behavior changes and bug fixes: + +1. Reproduce the symptom with the narrowest command or test. +2. Identify the root cause before changing code. +3. Add or update the focused test, fixture, or reproduction. +4. Verify the test fails for the expected reason. +5. Implement the smallest fix that addresses the root cause. +6. Rerun the focused verification, then broaden only when shared behavior is + touched. + +If an automated regression test is not practical, record the manual reproduction +and the final verification command in the PR. Do not claim a bug is fixed, +tests pass, or a contract is preserved without fresh output from the current +checkout or worktree. + ## Python Unit Tests Python engine and helper behavior lives under `cli/python/**/tests/` and diff --git a/skills.md b/skills.md index 5eeb732..37c6334 100644 --- a/skills.md +++ b/skills.md @@ -25,12 +25,83 @@ requests for Base. `bug/245-20260529-fix-profile-project-prompt`. - Do all pull request implementation work in a dedicated worktree under `~/work/base-worktrees/`. -- After merge, sync `master`, remove the worktree, and delete local and remote - branches. +- Before creating a worktree, check whether the current checkout is already a + linked worktree for the issue. Do not create nested or duplicate worktrees. +- Keep the PR worktree available while review feedback is pending. After merge, + sync `master`, remove the worktree, and delete local and remote branches. - Link PRs to issues with `Fixes #` or `Closes #`. - See `docs/github-workflow.md` for the full policy, including milestones and GitHub Projects. +## Debug and verify Base behavior + +Use this workflow when investigating failed Base commands, broken setup/check/ +doctor output, failed builds, project discovery surprises, runtime shell drift, +or CI failures. + +- Read the full error output first, including stack traces, command output, + finding IDs, and paths. +- Reproduce the symptom from a clean command line before fixing it. If the + issue is not reproducible, gather more evidence instead of guessing. +- Check recent changes with `git status`, `git diff`, and relevant commit or PR + context. +- Trace the bad value or failed state to its source. For cross-layer failures, + inspect each boundary separately: public launcher, Bash command, Python + helper, manifest parser, project command, environment variables, and working + directory. +- Form one hypothesis, make the smallest change that tests it, and rerun the + focused verification. Do not stack unrelated fixes. +- Keep `basectl check` and `basectl doctor` non-mutating. They should diagnose + readiness, not repair local state. +- Before claiming completion, run the command that proves the claim in the + current checkout or worktree and read the output. Report the command and the + result in the PR and final summary. + +For example, a `basectl doctor` bug should usually have both a focused unit +test around the finding logic and a smoke test that exercises the command or +Python entry point that emits the finding. + +## Change behavior, fix bugs, or handle review feedback + +Use this workflow when changing user-facing Base behavior, shared runtime +behavior, command output, JSON contracts, doctor findings, setup logic, or +public workflow docs. + +- Start bug fixes with a failing test, fixture, or reproduction whenever + practical. Prove it fails for the expected reason before relying on it. +- Keep test scope proportional to risk: focused BATS or pytest first, then + `basectl test base` or integration checks when shared behavior is touched. +- When an automated test is not practical, document the manual reproduction and + verification command clearly. +- For docs-only or configuration-only changes, `git diff --check` is usually + enough unless the change affects CI validation or generated output. +- Evaluate review feedback against Base's product boundaries before + implementing it. Base is the workspace control plane; project-specific + application behavior belongs in the owning project. +- If feedback suggests a larger design or product shift, stop and surface the + decision instead of hiding it inside a small PR. + +## Add or revise a Base agent workflow + +Use this workflow when adding or changing reusable AI-assisted development +guidance such as this `skills.md` file, `AGENTS.md`, or workflow documentation. + +- Put durable repo-local rules in `AGENTS.md`, `CONTRIBUTING.md`, + `STANDARDS.md`, `skills.md`, or focused docs. Do not add personal Codex + runtime settings to the repo. +- Prefer trigger-focused workflow names and descriptions. The first lines + should make it clear when the workflow applies. +- Keep entries concise and Base-specific. Link to focused docs for longer + policy instead of duplicating it. +- Align examples with Base conventions: `basectl gh`, `origin/master`, + `/--`, and + `~/work/base-worktrees/`. +- Review the workflow against likely pressure cases: time pressure, ambiguous + review feedback, failing tests, dirty worktrees, and temptation to broaden + Base beyond the workspace control-plane boundary. +- Validate documentation-only workflow changes with `git diff --check`. If a + CI workflow validates the guidance, run or update that validation too. + ## Add a basectl subcommand Use this workflow when adding or changing a `basectl ` feature.