From 6af1c69a9f8682bb7401708b932526b34b901244 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Thu, 18 Jun 2026 09:23:34 -0300 Subject: [PATCH 1/2] Inline non-fast-forward error check into Commit class - Add `Commit.isNonFastForwardError` static method replacing the standalone `src/cli/commit-errors.ts` module. - Update the push retry path in `src/cli/commit.ts` to call `Commit.isNonFastForwardError` and drop the removed import. - Delete the single-use `src/cli/commit-errors.ts` module. - Update `test/cli/commit-errors.test.ts` to import `Commit` and assert against the static method. --- src/cli/commit-errors.ts | 6 ------ src/cli/commit.ts | 8 ++++++-- test/cli/commit-errors.test.ts | 6 +++--- 3 files changed, 9 insertions(+), 11 deletions(-) delete mode 100644 src/cli/commit-errors.ts diff --git a/src/cli/commit-errors.ts b/src/cli/commit-errors.ts deleted file mode 100644 index 86e20f0..0000000 --- a/src/cli/commit-errors.ts +++ /dev/null @@ -1,6 +0,0 @@ -export { isNonFastForwardError }; - -const isNonFastForwardError = (error: Error): boolean => { - const msg = error.message.toLowerCase(); - return msg.includes("non-fast-forward") || msg.includes("updates were rejected"); -}; diff --git a/src/cli/commit.ts b/src/cli/commit.ts index 88042c8..54dcd9d 100644 --- a/src/cli/commit.ts +++ b/src/cli/commit.ts @@ -15,7 +15,6 @@ import { loading } from "@/infra/ui/spinner"; import { renderCommitNote, renderPushNote } from "@/infra/ui/push-note"; import color from "picocolors"; -import { isNonFastForwardError } from "@/cli/commit-errors"; const USER_ACTIONS = ["commit_push", "commit", "regenerate", "adjust", "cancel"] as const; type UserAction = (typeof USER_ACTIONS)[number]; @@ -112,6 +111,11 @@ class Commit { }); } + static isNonFastForwardError(error: Error): boolean { + const msg = error.message.toLowerCase(); + return msg.includes("non-fast-forward") || msg.includes("updates were rejected"); + } + private promptAction(message: string): Future { return Future.attemptP(async () => { p.note(message, "Proposed Commit Message"); @@ -162,7 +166,7 @@ class Commit { .hasUpstream() .chain((exists) => exists ? - this.push(request).chainRej((err) => (isNonFastForwardError(err) ? this.promptForceWithLease(request) : Future.reject(err))) + this.push(request).chainRej((err) => (Commit.isNonFastForwardError(err) ? this.promptForceWithLease(request) : Future.reject(err))) : this.promptPublishBranch(request) ); } diff --git a/test/cli/commit-errors.test.ts b/test/cli/commit-errors.test.ts index 88f3daf..e2d8d63 100644 --- a/test/cli/commit-errors.test.ts +++ b/test/cli/commit-errors.test.ts @@ -1,15 +1,15 @@ import { describe, expect, it } from "vitest"; -import { isNonFastForwardError } from "@/cli/commit-errors"; +import { Commit } from "@/cli/commit"; describe("isNonFastForwardError", () => { it.each(["error: failed to push: non-fast-forward", "Updates were rejected because the tip of your current branch is behind"])( "detects: %s", (message) => { - expect(isNonFastForwardError(new Error(message))).toBe(true); + expect(Commit.isNonFastForwardError(new Error(message))).toBe(true); } ); it("returns false for unrelated errors", () => { - expect(isNonFastForwardError(new Error("authentication failed"))).toBe(false); + expect(Commit.isNonFastForwardError(new Error("Some other error"))).toBe(false); }); }); From 6ff39377572206eaebc0f06d91d750c7bd36b9b5 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Thu, 18 Jun 2026 09:23:41 -0300 Subject: [PATCH 2/2] Add review guidelines for pull request reviews - Add `AGENTS.md` defining P0/P1 blocking criteria for reviews. - Document non-blocking exclusions and the required fields for every finding. --- AGENTS.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..6a8c386 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,33 @@ +## Review guidelines + +Reviews must be concise, high-signal, and limited to issues that should affect whether a pull request is merged. + +Only leave blocking review comments for concrete P0/P1 risks. + +P0/P1 means: + +- Security, privacy, data-loss, authentication, authorization, permission, or availability regressions. +- A likely production correctness bug with a concrete execution path introduced by the diff. +- A broken build, failing test, migration issue, or API contract break directly caused by the diff. +- A change that violates an existing documented invariant, schema, caller contract, or product requirement. + +Do not leave blocking comments for: + +- Hypothetical edge cases without a realistic user path. +- Inputs that are impossible under existing callers, schemas, UI constraints, API contracts, or validation layers. +- Style preferences, naming preferences, alternative designs, or speculative refactors. +- Missing defensive handling unless the pull request introduces a realistic failure path. +- "This could happen if..." concerns without evidence from the diff. +- Pre-existing issues not made worse by the pull request. +- Suggestions that would expand scope beyond the pull request's intent. + +Every finding must include: + +- The concrete failure path. +- Why it is P0 or P1. +- The exact changed line or smallest relevant range. +- The smallest practical fix. + +If a concern is real but non-blocking, omit it unless it is explicitly useful. If included, put it under "Non-blocking notes". + +If there are no P0/P1 findings, say: "No blocking findings."