-
Notifications
You must be signed in to change notification settings - Fork 0
docs: codify Code Lawyer standards #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| # CODE_STANDARDS | ||
|
|
||
| These standards define the required review and repair posture for Graft. | ||
| They are operational doctrine, not suggestions. When these rules conflict | ||
| with convenience, convenience loses. | ||
|
|
||
| ## Code Lawyer Mandate | ||
|
|
||
| Code Lawyer is the repository's strict audit posture: | ||
|
|
||
| - Treat correctness, determinism, architecture, typing, documentation, and | ||
| style as release-relevant integrity concerns. | ||
| - Prefer inspectable evidence over assertion. Anchor findings to files, | ||
| line numbers, commands, commits, PR threads, or reproducible tests. | ||
| - Do not normalize sludge. Duplication, hidden nondeterminism, vague | ||
| ownership, stale docs, loose types, and unbounded processes must either | ||
| be fixed or filed as explicit debt. | ||
| - Never hide risk behind green reruns. A pass after an unexplained failure | ||
| is evidence of nondeterminism until the failure mode is understood or | ||
| bounded. | ||
|
|
||
| ## Severity Ladder | ||
|
|
||
| Use this priority order when building a review or repair queue: | ||
|
|
||
| | Severity | Meaning | | ||
| | :--- | :--- | | ||
| | P0 Critical | Data loss, security boundary failure, release corruption, destructive Git risk, or a broken invariant with broad blast radius. | | ||
| | P1 High | User-visible bug, deterministic correctness failure, authz/path escape, broken public API, or CI/release gate failure. | | ||
| | P2 Medium | Edge-case bug, architectural mismatch, race risk, missing contract coverage, or misleading generated output. | | ||
| | P3 Low | Docs drift, incomplete witness, degraded diagnostics, missing targeted regression, or narrow maintainability issue. | | ||
| | P4 Style | Formatting, naming, markdown structure, consistency, or local readability issue that does not change behavior. | | ||
| | P5 Nit | Tiny polish item that should still be fixed when already touching the area. | | ||
|
|
||
| ## Phase 0: Lockdown | ||
|
|
||
| Before any Code Lawyer review or repair loop: | ||
|
|
||
| ```bash | ||
| export GH_PAGER=cat | ||
| git status --porcelain | ||
| ``` | ||
|
|
||
| - Dirty worktree: halt, report dirty paths, and abort. | ||
| - Clean worktree: run `git fetch origin`. | ||
| - If `gh` authentication fails, halt immediately with: | ||
|
|
||
| ```text | ||
| Auth error - run `gh auth login` and retry. | ||
| ``` | ||
|
|
||
| Do not continue a GitHub-dependent workflow after an auth failure. | ||
|
|
||
| ## Phase I: Discovery and Radical Transparency | ||
|
|
||
| 1. Fetch unresolved review context. For PR work, use a full GraphQL query | ||
| that includes global comments, reviews, inline review threads, | ||
| `isResolved`, `isOutdated`, path, line, and comment bodies. | ||
| 2. Run the branch audit: | ||
|
|
||
| ```bash | ||
| git diff origin/main...HEAD | ||
| ``` | ||
|
|
||
| 3. Audit for logic bugs, races, edge cases, nondeterminism, duplication, | ||
| separation-of-concerns violations, style drift, typing holes, stale docs, | ||
| and release-gate gaps. | ||
| 4. If self-discovered issues exist on a PR, immediately post a PR comment | ||
| with a clean issue table and include `@codex` for a second opinion. | ||
|
|
||
| Findings must include: | ||
|
|
||
| - filepath | ||
| - line or range when available | ||
| - infraction type | ||
| - severity | ||
| - evidence | ||
| - recommended mitigation | ||
|
|
||
| ## Phase II: Surgical Execution Loop | ||
|
|
||
| Merge PR feedback and self-discovered findings into one prioritized queue, | ||
| ordered P0 through P5. Resolve one issue at a time. | ||
|
|
||
| For each issue, print a local log entry: | ||
|
|
||
| ```text | ||
| === [N] [P0-P5] =================================== | ||
| Source: [PR / Self] | ||
| File: path/to/file.ext | ||
| Lines: Lxx-Lyy | ||
| Issue: [one-line summary] | ||
| ``` | ||
|
|
||
| Then run a Red-Green-Verify-Document-Commit loop: | ||
|
|
||
| 1. RED: create the smallest deterministic regression that fails only for | ||
| the issue under repair. Run it and confirm failure. | ||
| 2. GREEN: make the smallest architecture-preserving fix. | ||
| 3. VERIFY: rerun the regression and the relevant suite. Both must pass. | ||
| 4. DOCUMENT: update `CHANGELOG.md` or internal docs when behavior, | ||
| schema, API, release truth, or invariant posture changes. | ||
| 5. COMMIT: make one focused commit per issue: | ||
|
|
||
| ```bash | ||
| git add -A | ||
| git commit -m "Fix: <precise description>" | ||
| ``` | ||
|
|
||
| 6. RESOLVE: if the issue came from a PR thread, mark that exact thread | ||
| resolved through GraphQL. | ||
|
|
||
| Regression quality rules: | ||
|
|
||
| - Avoid wall-clock timing, unseeded randomness, ambient network state, and | ||
| live-checkout fixture coupling. | ||
| - Assert observable behavior, not implementation text. | ||
| - Prefer error types, codes, and structured output over fragile message | ||
| matching. | ||
| - Stdout and stderr are not enough for behavioral proof unless the public | ||
| contract is explicitly a CLI rendering contract. | ||
|
|
||
| ## Phase III: Closure | ||
|
|
||
| For PR work, post an Activity Summary comment: | ||
|
|
||
| ```markdown | ||
| | Issue | Severity | File | Commit SHA | Outcome | | ||
| | :--- | :--- | :--- | :--- | :--- | | ||
| | {Issue Description} | {P0-P5/Docs} | {File} | {Hash} | {Result} | | ||
| ``` | ||
|
|
||
| Print a console SitRep for every resolved item: | ||
|
|
||
| ```text | ||
| --[ ITEM N ]----------------------------------------------------------------- | ||
| ISSUE: {Description} | ||
| SEVERITY: {Severity} | ||
| COMMIT: {Hash} | ||
| REGRESSION: {Test Name}; {Command} | ||
| OUTCOME: {Summary of technical change} | ||
| ----------------------------------------------------------------------------- | ||
| ``` | ||
|
|
||
| ## Phase IV: Merge Gate | ||
|
|
||
| Before declaring a PR mergeable, verify all gates: | ||
|
|
||
| - CI green via `gh pr checks`. | ||
| - At least two approvals. | ||
| - Zero Changes Requested reviews. | ||
| - No active CodeRabbit cooldown/rate-limit blocker. | ||
| - Local tests and linters are 100% clean for the changed surface. | ||
| - No unresolved actionable review threads. | ||
| - Worktree is clean. | ||
|
|
||
| If ready: | ||
|
|
||
| ```text | ||
| MERGE GATE: OPEN ✅ | ||
| All invariants satisfied. Reply **YES** to execute merge. | ||
| ``` | ||
|
|
||
| If blocked: | ||
|
|
||
| ```text | ||
| MERGE GATE: LOCKED 🔒 | ||
| Blocking reasons: | ||
| • ... | ||
| Reply with fixes or "FORCE" only if you accept risk. | ||
| ``` | ||
|
|
||
| Admin override is allowed only when the operator explicitly accepts that | ||
| risk. Even then, use normal merge mechanics; never rebase, amend, or force. | ||
|
|
||
| ## Repository Integrity Rules | ||
|
|
||
| - Keep work scoped to the issue being resolved. | ||
| - Do not combine unrelated fixes in one commit. | ||
| - Do not rewrite history. | ||
| - Do not push to `main` without explicit permission. | ||
| - Keep generated or build artifacts out of commits unless they are the | ||
| documented deliverable. | ||
| - Update docs when behavior, workflow, release truth, or invariants change. | ||
| - File unresolved bad-code findings in `docs/method/backlog/bad-code/`. | ||
| - File non-blocking ideas in `docs/method/backlog/cool-ideas/`. | ||
| - Treat `pnpm lint`, `pnpm typecheck`, and the relevant tests as the | ||
| minimum proof surface for code changes. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import * as fs from "node:fs"; | ||
| import * as path from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../.."); | ||
|
|
||
| function readRepoFile(relativePath: string): string { | ||
| return fs.readFileSync(path.join(ROOT, relativePath), "utf8"); | ||
| } | ||
|
|
||
| describe("repository code standards", () => { | ||
| it("keeps the Code Lawyer standards discoverable from agent orientation", () => { | ||
| const standards = readRepoFile("CODE_STANDARDS.md"); | ||
| const agents = readRepoFile("AGENTS.md"); | ||
|
|
||
| expect(standards).toContain("# CODE_STANDARDS"); | ||
| expect(standards).toContain("Code Lawyer"); | ||
| expect(standards).toContain("Phase 0: Lockdown"); | ||
| expect(standards).toContain("Merge Gate"); | ||
| expect(agents).toContain("CODE_STANDARDS.md"); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.