Inline non-fast-forward check and add review guidelines

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."

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<Error, UserAction> {
     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)
       );
   }

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);
   });
 });