address code rabbit comments

.github/workflows/dev-release-prep.yml

@@ -18,7 +18,7 @@ concurrency:
 
 jobs:
   prepare:
-    if: github.event.pull_request.merged == true && !startsWith(github.event.pull_request.head.ref, 'release/')
+    if: github.event.pull_request.merged == true && !startsWith(github.event.pull_request.head.ref, 'release/') && !startsWith(github.event.pull_request.head.ref, 'review/')
     runs-on: ubuntu-latest
     env:
       OPENCODE_API_KEY: ${{ secrets.OPENCODE_API_KEY }}

.github/workflows/review-response.yml

@@ -13,13 +13,15 @@ jobs:
       pull-requests: write
     env:
       OPENCODE_API_KEY: ${{ secrets.OPENCODE_API_KEY }}
+      GH_TOKEN: ${{ secrets.PR_AUTOMATION_TOKEN }}
     steps:
       - name: Checkout PR head
         uses: actions/checkout@v4
         with:
           fetch-depth: 0
           repository: ${{ github.event.pull_request.head.repo.full_name }}
           ref: ${{ github.event.pull_request.head.ref }}
+          token: ${{ env.GH_TOKEN }}
 
       - name: Verify OpenCode secret
         run: |
@@ -28,6 +30,13 @@ jobs:
             exit 1
           fi
 
+      - name: Validate PR automation token
+        run: |
+          if [ -z "$GH_TOKEN" ]; then
+            echo "PR_AUTOMATION_TOKEN is required to open PRs; org policy blocks GITHUB_TOKEN" >&2
+            exit 1
+          fi
+
       - name: Setup Node.js
         uses: actions/setup-node@v4
         with:
@@ -53,7 +62,7 @@ jobs:
 
       - name: Run review-response agent
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ env.GH_TOKEN }}
         run: |
           opencode run \
             --agent review-response \
@@ -73,7 +82,7 @@ jobs:
       - name: Commit and push
         if: steps.diff.outputs.has_changes == 'true'
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ env.GH_TOKEN }}
         run: |
           git config user.name "github-actions[bot]"
           git config user.email "github-actions[bot]@users.noreply.github.com"
@@ -84,7 +93,7 @@ jobs:
       - name: Open pull request
         if: steps.diff.outputs.has_changes == 'true'
         env:
-          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GH_TOKEN: ${{ env.GH_TOKEN }}
         run: |
           comment_url="${{ steps.context.outputs.comment_url }}"
           reviewer="${{ steps.context.outputs.reviewer }}"

lib/prompts/codex.ts

@@ -67,8 +67,13 @@
 	cacheFilePath: string,
 	etag?: string | undefined,
 	tag?: string | undefined,
-): string {
-	const fileContent = safeReadFile(cacheFilePath) || "";
+): string | null {
+	const fileContent = safeReadFile(cacheFilePath);
+	if (!fileContent) {
+		logWarn("Cached Codex instructions missing or empty; skipping session cache");
+		return null;
+	}
+
 	cacheSessionEntry(fileContent, etag, tag);
 	return fileContent;
 }
@@ -101,7 +106,11 @@
 	const response = await fetch(url, { headers });
 
 	if (response.status === 304 && cacheFileExists) {
-		return readCachedInstructions(cacheFilePath, cachedETag || undefined, latestTag);
+		const cachedContent = readCachedInstructions(cacheFilePath, cachedETag || undefined, latestTag);
+		if (cachedContent) {
+			return cachedContent;
+		}
+		throw new Error("Cached Codex instructions were unavailable after 304 response");
 	}
 
 	if (!response.ok) {
@@ -153,11 +162,15 @@
 
 		if (options.cacheFileExists) {
 			logWarn("Using cached instructions due to fetch failure");
-			return readCachedInstructions(
+			const cachedContent = readCachedInstructions(
 				options.cacheFilePath,
 				options.effectiveEtag || options.cachedETag || undefined,
 				options.cachedTag || undefined,
 			);
+			if (cachedContent) {
+				return cachedContent;
+			}
+			logWarn("Cached instructions unavailable; falling back to bundled instructions");
 		}
 
 		logWarn("Falling back to bundled instructions");
@@ -173,7 +186,7 @@
  * Rate limit protection: Only checks GitHub if cache is older than 15 minutes
  * @returns Codex instructions
  */
 export async function getCodexInstructions(): Promise<string> {
 	const sessionEntry = codexInstructionsCache.get("latest");
 	if (sessionEntry) {
 		recordCacheHit("codexInstructions");
@@ -196,7 +209,15 @@
 
 	const cacheFileExists = fileExistsAndNotEmpty(cacheFilePath);
 	if (cacheIsFresh(cachedTimestamp, cacheFileExists)) {
-		return readCachedInstructions(cacheFilePath, cachedETag || undefined, cachedTag || undefined);
+		const cachedContent = readCachedInstructions(
+			cacheFilePath,
+			cachedETag || undefined,
+			cachedTag || undefined,
+		);
+		if (cachedContent) {
+			return cachedContent;
+		}
+		logWarn("Cached Codex instructions were empty; attempting to refetch");
 	}
 
 	let latestTag: string | undefined;
@@ -207,7 +228,15 @@
 			error,
 		});
 		if (cacheFileExists) {
-			return readCachedInstructions(cacheFilePath, cachedETag || undefined, cachedTag || undefined);
+			const cachedContent = readCachedInstructions(
+				cacheFilePath,
+				cachedETag || undefined,
+				cachedTag || undefined,
+			);
+			if (cachedContent) {
+				return cachedContent;
+			}
+			logWarn("Cached instructions unavailable; falling back to bundled copy");
 		}
 		return loadBundledInstructions();
 	}

lib/request/compaction-helpers.ts

@@ -8,7 +8,6 @@ import {
 import type { CompactionDecision } from "../compaction/compaction-executor.js";
 import { filterInput } from "./input-filters.js";
 import type { InputItem, RequestBody } from "../types.js";
-import { cloneInputItems } from "../utils/clone.js";
 import { countConversationTurns } from "../utils/input-item-utils.js";
 
 export interface CompactionSettings {
@@ -24,15 +23,16 @@ export interface CompactionOptions {
 	preserveIds?: boolean;
 }
 
+/**
+ * Drop only the latest user message (e.g., a compaction command) while preserving any later assistant/tool items.
+ */
 function removeLastUserMessage(items: InputItem[]): InputItem[] {
-	const cloned = cloneInputItems(items);
-	for (let index = cloned.length - 1; index >= 0; index -= 1) {
-		if (cloned[index]?.role === "user") {
-			cloned.splice(index, 1);
-			break;
+	for (let index = items.length - 1; index >= 0; index -= 1) {
+		if (items[index]?.role === "user") {
+			return [...items.slice(0, index), ...items.slice(index + 1)];
 		}
 	}
-	return cloned;
+	return items;
 }
 
 function maybeBuildCompactionPrompt(
@@ -43,9 +43,7 @@ function maybeBuildCompactionPrompt(
 	if (!settings.enabled) {
 		return null;
 	}
-	const conversationSource = commandText
-		? removeLastUserMessage(originalInput)
-		: cloneInputItems(originalInput);
+	const conversationSource = commandText ? removeLastUserMessage(originalInput) : originalInput;
 	const turnCount = countConversationTurns(conversationSource);
 	let trigger: "command" | "auto" | null = null;
 	let reason: string | undefined;

lib/request/input-filters.ts

@@ -13,6 +13,8 @@ import type { InputItem, SessionContext } from "../types.js";
 import { extractTextFromItem } from "../utils/input-item-utils.js";
 import { logDebug } from "../logger.js";
 
+const TOOL_REMAP_MESSAGE_HASH = generateContentHash(TOOL_REMAP_MESSAGE);
+
 export function filterInput(
 	input: InputItem[] | undefined,
 	options: { preserveIds?: boolean } = {},
@@ -247,6 +249,17 @@ export function addToolRemapMessage(
 ): InputItem[] | undefined {
 	if (!hasTools || !Array.isArray(input)) return input;
 
+	const hasExistingToolRemap = input.some((item) => {
+		if (item?.type !== "message" || item?.role !== "developer") return false;
+		const contentText = extractTextFromItem(item);
+		if (!contentText) return false;
+		return generateContentHash(contentText) === TOOL_REMAP_MESSAGE_HASH;
+	});
+
+	if (hasExistingToolRemap) {
+		return input;
+	}
+
 	const toolRemapMessage: InputItem = {
 		type: "message",
 		role: "developer",

scripts/review-response-context.mjs

@@ -36,10 +36,10 @@ function main() {
 
 	const filePath = comment.path;
 	const reviewer = comment.user?.login ?? "unknown";
-	const branchSlug = `review/comment-${comment.id}`;
+	const baseRef = pr.base?.ref ?? "main";
+	const branchSlug = `review/${baseRef}-${comment.id}`;
 	const prNumber = pr.number;
 	const prTitle = pr.title ?? "";
-	const baseRef = pr.base?.ref ?? "main";
 	const baseSha = pr.base?.sha ?? "";
 	const headRef = pr.head?.ref ?? "";
 	const headSha = pr.head?.sha ?? "";

spec/review-response-token.md

@@ -0,0 +1,24 @@
+# Review-response workflow token alignment
+
+## Context
+
+- File: .github/workflows/review-response.yml (lines 1-106)
+- Behavior: Responds to PR review comments; checks out PR head and pushes auto-fix branch using default GITHUB_TOKEN.
+- Issue: Org policy blocks PR creation/push via default `GITHUB_TOKEN`; dev-release-prep uses `PR_AUTOMATION_TOKEN` validated explicitly.
+- Existing related workflow: .github/workflows/dev-release-prep.yml uses `PR_AUTOMATION_TOKEN` for PR creation and validates secret.
+
+## Requirements / Definition of Done
+
+- review-response workflow uses `secrets.PR_AUTOMATION_TOKEN` for all GitHub write operations (push, PR creation, gh api/cli) instead of default GITHUB_TOKEN.
+- Validate presence of PR_AUTOMATION_TOKEN early with a failure message mirroring dev-release-prep wording.
+- Keep OPENCODE_API_KEY handling unchanged.
+- Ensure checkout/push/pr steps reference the new token via env (GH_TOKEN and git auth) so branch push + PR creation succeed under org policy.
+- Review-response branches should be named `review/<base>-<review-id>`.
+- Release automation should treat `review/*` branches as non-release and avoid version bumps.
+- Tests/build unaffected (workflow-only change).
+
+## Open Questions / Notes
+
+- No other review-response workflows present.
+- PAT must have repo permissions; assumes secret already configured in repo/org.
+- Checkout of fork PRs may still require permission; pushing uses PAT.

spec/review-v0.3.5-fixes.md

@@ -0,0 +1,25 @@
+# Review v0.3.5 fixes
+
+## Scope
+
+- Handle null/empty cache reads in `lib/prompts/codex.ts` around readCachedInstructions caching logic
+- Remove redundant cloning in `lib/request/compaction-helpers.ts` (removeLastUserMessage, maybeBuildCompactionPrompt)
+- Prevent duplicate tool remap injection in `lib/request/input-filters.ts` addToolRemapMessage
+
+## Existing issues / PRs
+
+- None identified for this branch (review/v0.3.5).
+
+## Definition of done
+
+- safeReadFile null results do not get cached as empty content; fallback logic remains available for caller
+- Compaction helpers avoid unnecessary clones while preserving immutability semantics (original input reused unless truncated)
+- Tool remap message is only prepended once when tools are present; logic handles undefined/null safely
+- All relevant tests updated or added if behavior changes; existing suite passes locally if run
+
+## Requirements / notes
+
+- Only cache instructions when actual non-empty content is read; on null either warn and return null or allow existing fallback paths
+- removeLastUserMessage should find last user role index and slice; when commandText is falsy reuse originalInput directly in maybeBuildCompactionPrompt
+- addToolRemapMessage should fingerprint or compare TOOL_REMAP_MESSAGE and skip if already present (matching role/type/text)
+- Preserve existing function signatures and return types throughout

test/compaction-helpers.test.ts

@@ -0,0 +1,59 @@
+import { applyCompactionIfNeeded } from "../lib/request/compaction-helpers.js";
+import type { InputItem, RequestBody } from "../lib/types.js";
+
+describe("compaction helpers", () => {
+	it("drops only the last user command and keeps trailing items", () => {
+		const originalInput: InputItem[] = [
+			{ type: "message", role: "assistant", content: "previous response" },
+			{ type: "message", role: "user", content: "/codex-compact please" },
+			{ type: "message", role: "assistant", content: "trailing assistant" },
+		];
+		const body: RequestBody = { model: "gpt-5", input: [...originalInput] };
+
+		const decision = applyCompactionIfNeeded(body, {
+			settings: { enabled: true },
+			commandText: "codex-compact please",
+			originalInput,
+		});
+
+		expect(decision?.mode).toBe("command");
+		expect(decision?.serialization.transcript).toContain("previous response");
+		expect(decision?.serialization.transcript).toContain("trailing assistant");
+		expect(decision?.serialization.transcript).not.toContain("codex-compact please");
+
+		// Verify RequestBody mutations
+		expect(body.input).not.toEqual(originalInput);
+		expect(body.input?.some((item) => item.content === "/codex-compact please")).toBe(false);
+		expect((body as any).tools).toBeUndefined();
+		expect((body as any).tool_choice).toBeUndefined();
+		expect((body as any).parallel_tool_calls).toBeUndefined();
+	});
+
+	it("applies compaction when no user message exists", () => {
+		const originalInput: InputItem[] = [
+			{
+				type: "message",
+				role: "assistant",
+				content: "system-only follow-up",
+			},
+		];
+		const body: RequestBody = { model: "gpt-5", input: [...originalInput] };
+
+		const decision = applyCompactionIfNeeded(body, {
+			settings: { enabled: true },
+			commandText: "codex-compact",
+			originalInput,
+		});
+
+		expect(decision?.serialization.totalTurns).toBe(1);
+		expect(decision?.serialization.transcript).toContain("system-only follow-up");
+
+		// Verify RequestBody mutations
+		expect(body.input).toBeDefined();
+		expect(body.input?.length).toBeGreaterThan(0);
+		expect(body.input).not.toEqual(originalInput);
+		expect((body as any).tools).toBeUndefined();
+		expect((body as any).tool_choice).toBeUndefined();
+		expect((body as any).parallel_tool_calls).toBeUndefined();
+	});
+});