cmoriarty/braid
1
0
Fork 0
Code Issues 15 Pull requests Projects Releases Packages 1 Wiki Activity Actions

A code review step before the feature commit in the thorough and git-flow pipelines #108

New issue
Closed
opened 2026-09-29 01:28:02 -04:00 by cmoriarty · 1 comment
cmoriarty commented 2026-09-29 01:28:02 -04:00
Owner
Copy link

Why

Nothing reviews the code a run writes before the flow commits it. The proposal gets four review lenses and a human gate. The implementation goes from its tests straight into git.commit.feature and the pull request, and with auto-approve on, straight into the base branch. In cmoriarty/scratch, the first feature commit carried bytecode and packaging metadata that nobody looked at (#107).

Expected

  • In the thorough pipeline, and in git-flow (which is thorough on feature branches), an agent step reviews the change just before git.commit.feature. It reads the diff and every new file against the change's proposal, specs, design and tasks.
  • It reviews what the commit would contain: generated output, caches, environments, secrets and stray scratch files. It also reviews the code itself: bugs, leftover debug code, gaps against the specs, and missing tests.
  • It fixes what it finds and re-runs the project's fast tests after any code change. It records each finding and what it did. The record is committed with the change, and the pull request says what the review found.
  • Not in minimalist, quick-fix or docs, which stay lean.

Adding a node to a built-in pipeline runs into #102 for runs already in flight.

Related: #107.

## Why Nothing reviews the code a run writes before the flow commits it. The proposal gets four review lenses and a human gate. The implementation goes from its tests straight into `git.commit.feature` and the pull request, and with auto-approve on, straight into the base branch. In `cmoriarty/scratch`, the first feature commit carried bytecode and packaging metadata that nobody looked at (#107). ## Expected - In the thorough pipeline, and in git-flow (which is thorough on feature branches), an agent step reviews the change just before `git.commit.feature`. It reads the diff and every new file against the change's proposal, specs, design and tasks. - It reviews what the commit would contain: generated output, caches, environments, secrets and stray scratch files. It also reviews the code itself: bugs, leftover debug code, gaps against the specs, and missing tests. - It fixes what it finds and re-runs the project's fast tests after any code change. It records each finding and what it did. The record is committed with the change, and the pull request says what the review found. - Not in minimalist, quick-fix or docs, which stay lean. Adding a node to a built-in pipeline runs into #102 for runs already in flight. Related: #107.
cmoriarty commented 2026-09-29 04:12:35 -04:00
Author
Owner
Copy link

Shipped in 3d6a192, as the OpenSpec change code-review-step. It's archived in 24b9965 as a new code-review spec, plus updates to pipeline-steps and pull-request-description.

  • Where it runs: a new agent step, agent.code-review, sits in thorough and git-flow, between script.gitignore.feature (#107) and git.commit.feature. Minimalist, quick-fix, docs and hotfix don't have it.
  • What it sees: its prompt carries the change's proposal, specs, design and tasks. It also carries exactly what the commit would contain: every path from git status, which honours the ignores, and the tracked diff, capped at 40,000 characters. The agent itself still runs no git.
  • What it reviews: first, what the commit would carry that it shouldn't: generated output, caches, environments, packaging metadata, secrets, large binaries and stray files. Then the code against the change: bugs, security holes, leftovers, gaps against the specs, and tests that don't test.
  • What it does about it: it fixes what it can within the change and re-runs the fast tests after any code change. It records each finding (severity, whether fixed) in .osf/review/code.json.
  • When it stops the run: a blocking finding left unfixed, or failing tests, fail the step before the commit, so a person looks.
  • In the pull request: a new "Code review" section lists what was left for you, then what was fixed before the commit.

Verified:

  • On a local osfd, a clean thorough run's review re-ran the tests and recorded nothing to fix.
  • A repository whose pipeline planted a subtraction bug, a debug print, a scratch file and egg-info got all four fixed. The review made a .gitignore for the egg-info, the tests passed, and none of the leftovers was committed.
  • On production, a new thorough run's steps go agent.test.browser → script.gitignore.feature → agent.code-review → git.commit.feature.
Shipped in 3d6a192, as the OpenSpec change `code-review-step`. It's archived in 24b9965 as a new `code-review` spec, plus updates to `pipeline-steps` and `pull-request-description`. - **Where it runs:** a new agent step, `agent.code-review`, sits in `thorough` and `git-flow`, between `script.gitignore.feature` (#107) and `git.commit.feature`. Minimalist, quick-fix, docs and hotfix don't have it. - **What it sees:** its prompt carries the change's proposal, specs, design and tasks. It also carries exactly what the commit would contain: every path from `git status`, which honours the ignores, and the tracked diff, capped at 40,000 characters. The agent itself still runs no git. - **What it reviews:** first, what the commit would carry that it shouldn't: generated output, caches, environments, packaging metadata, secrets, large binaries and stray files. Then the code against the change: bugs, security holes, leftovers, gaps against the specs, and tests that don't test. - **What it does about it:** it fixes what it can within the change and re-runs the fast tests after any code change. It records each finding (severity, whether fixed) in `.osf/review/code.json`. - **When it stops the run:** a blocking finding left unfixed, or failing tests, fail the step before the commit, so a person looks. - **In the pull request:** a new "Code review" section lists what was left for you, then what was fixed before the commit. **Verified:** - On a local osfd, a clean thorough run's review re-ran the tests and recorded nothing to fix. - A repository whose pipeline planted a subtraction bug, a debug print, a scratch file and egg-info got all four fixed. The review made a `.gitignore` for the egg-info, the tests passed, and none of the leftovers was committed. - On production, a new thorough run's steps go `agent.test.browser` → `script.gitignore.feature` → `agent.code-review` → `git.commit.feature`.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
archive/prefect-kestra-experiments
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cmoriarty/braid#108
No description provided.