fix: address PR #20 review findings and scope reviewer to diff only

- workflow-run.ts: retry checkConclusionJobCompleted once on API error,
  then proceed instead of dropping the ingest on continued failure
- comment.ts: findExistingComment now throws on paginate error so
  upsertComment aborts rather than falling through to create a duplicate
- run.ts: only overwrite resolvedWorkflowName when runData.workflowName
  is non-empty, preserving the companion workflow name as fallback
- workflow-run.ts: anchor issue-branch regex to full name (^agent/issue-N$)
  to prevent substring matches in longer branch names
- run.ts: prefer resolvedTriggerType over resolvedTriggerEvent in
  buildTriggerRef so manual trigger_number without trigger_event doesn't
  mislabel PRs as generic #N issues
- README.md: make turns field conditional in printf snippet so an unset
  TURNS var doesn't write invalid JSON into the artifact
- agent-review.md: remove bash tools (no full-file reads), instruct
  reviewer to only comment on lines present in the diff

Author: adamhenson

.github/workflows/agent-review.lock.yml

@@ -19,7 +19,6 @@ permissions:
   pull-requests: read
 
 tools:
-  bash: ["cat", "head", "tail", "grep", "wc", "ls", "find"]
   github:
     toolsets: [repos, pull_requests]
 
@@ -41,19 +40,21 @@ Review PR #{{ github.event.pull_request.number }}: "{{ github.event.pull_request
 
 ## Review Process
 
-1. **Fetch the PR diff** using the GitHub tool.
-2. **Review for:**
+1. **Fetch the PR diff** using the GitHub tool (`get-pull-request-diff` or equivalent).
+2. **Review only the changed lines** (`+` lines in the diff). Do not fetch or read full file contents — your entire review must be scoped to lines present in the diff.
+3. **Review for:**
    - TypeScript type safety issues (strict mode — no `any`, no unchecked nulls)
    - Code style: JSDoc block above every function, alphabetical params, explicit return types
    - Logic errors or bugs
    - Missing test coverage for new logic
-3. **Post inline review comments** on specific lines where you find issues.
-4. **Submit your review:**
+4. **Post inline review comments** only on lines that appear in the diff.
+5. **Submit your review:**
    - No issues: use `noop` with a brief summary.
    - Issues found: COMMENT with specific, actionable feedback.
 
 ## Important
 
+- **Only comment on lines in the diff.** Do not flag pre-existing code that was not touched by this PR.
 - Be concise and constructive.
 - Don't nitpick formatting — Biome handles that.
 - Focus on correctness and type safety.

.github/workflows/agent-review.md

@@ -288,8 +288,12 @@ If you're not using gh-aw, add these two steps to your agent job to write and up
 - name: Write token counts
   if: always()
   run: |
-    printf '{"input_tokens":%s,"output_tokens":%s,"cache_read_tokens":%s,"cache_write_tokens":%s,"turns":%s}\n' \
-      "$INPUT_TOKENS" "$OUTPUT_TOKENS" "$CACHE_READ_TOKENS" "$CACHE_WRITE_TOKENS" "$TURNS" \
+    turns_field=""
+    if [ -n "$TURNS" ]; then
+      turns_field=",\"turns\":$TURNS"
+    fi
+    printf '{"input_tokens":%s,"output_tokens":%s,"cache_read_tokens":%s,"cache_write_tokens":%s%s}\n' \
+      "$INPUT_TOKENS" "$OUTPUT_TOKENS" "$CACHE_READ_TOKENS" "$CACHE_WRITE_TOKENS" "$turns_field" \
       > /tmp/agent-tokens.json
 
 - uses: actions/upload-artifact@v4

README.md

@@ -302,21 +302,17 @@ async function findExistingComment({
   repo: string;
   issueOrPrNumber: number;
 }): Promise<{ id: number; body: string } | null> {
-  try {
-    const gh = octokit as ReturnType<typeof import('@actions/github').getOctokit>;
-    const comments = await gh.paginate(gh.rest.issues.listComments, {
-      owner,
-      repo,
-      issue_number: issueOrPrNumber,
-      per_page: 100,
-    });
+  const gh = octokit as ReturnType<typeof import('@actions/github').getOctokit>;
+  const comments = await gh.paginate(gh.rest.issues.listComments, {
+    owner,
+    repo,
+    issue_number: issueOrPrNumber,
+    per_page: 100,
+  });
 
-    const existing = comments.find((c) => c.body?.includes(COMMENT_MARKER));
-    if (!existing) return null;
-    return { id: existing.id, body: existing.body ?? '' };
-  } catch {
-    return null;
-  }
+  const existing = comments.find((c) => c.body?.includes(COMMENT_MARKER));
+  if (!existing) return null;
+  return { id: existing.id, body: existing.body ?? '' };
 }
 
 /**

dist/index.js

@@ -99,9 +99,10 @@ export async function run(): Promise<void> {
       if (!inputs.triggerEvent) resolvedTriggerEvent = runData.triggerEvent;
       resolvedTriggerRef = runData.triggerRef;
       resolvedTriggerType = runData.triggerType;
-      // Always use the agent workflow's name from the run data — ctx.workflowName is the
-      // companion workflow and would misattribute if used as a fallback here.
-      resolvedWorkflowName = runData.workflowName;
+      // Prefer the agent workflow's name from the run data over ctx.workflowName (which is the
+      // companion workflow). Only overwrite when non-empty so a failed getWorkflowRun() doesn't
+      // blank out the name entirely.
+      if (runData.workflowName) resolvedWorkflowName = runData.workflowName;
       workflowRunTokens = runData.tokens;
       artifactTurns = runData.artifactTurns;
     }
@@ -138,11 +139,17 @@ export async function run(): Promise<void> {
   // Fall back to resolvedTriggerRef from resolveTrigger (companion workflow_run mode — it knows
   // whether a PR was found regardless of the triggering event name, e.g. issue_comment on a PR).
   // Last resort: buildTriggerRef from the event name and number.
+  // Prefer resolvedTriggerType (set by resolveTrigger in workflow_run mode — already knows
+  // PR vs issue) over resolvedTriggerEvent when building the ref label, so a manual
+  // trigger_number without trigger_event doesn't default to the generic '#N' form for PRs.
   const triggerRef =
     ctx.triggerRef ??
     resolvedTriggerRef ??
     (resolvedTriggerNumber !== null
-      ? buildTriggerRef({ eventName: resolvedTriggerEvent, number: resolvedTriggerNumber })
+      ? buildTriggerRef({
+          eventName: resolvedTriggerType ?? resolvedTriggerEvent,
+          number: resolvedTriggerNumber,
+        })
       : null);
 
   // resolvedTriggerType is only set in companion workflow_run mode (from resolveTrigger).

src/comment.ts

@@ -180,7 +180,7 @@ async function checkConclusionJobCompleted({
   /** Workflow run ID */
   workflowRunId: number;
 }): Promise<boolean> {
-  try {
+  const attemptCheck = async (): Promise<boolean> => {
     const { data } = await octokit.rest.actions.listJobsForWorkflowRun({
       owner,
       repo,
@@ -198,12 +198,24 @@ async function checkConclusionJobCompleted({
     }
     core.info(`AgentMeter: conclusion job completed (${conclusionJob.conclusion}) — proceeding.`);
     return true;
-  } catch (error) {
-    // Fail closed on API errors — proceeding on a failed gate check risks double-ingest.
-    // A missing conclusion job (non-gh-aw workflow) is handled above as a successful
-    // API call that returns no matching job, not as an exception.
-    core.warning(`AgentMeter: could not check conclusion job status: ${error}. Skipping.`);
-    return false;
+  };
+
+  try {
+    return await attemptCheck();
+  } catch (firstError) {
+    core.warning(
+      `AgentMeter: could not check conclusion job status (attempt 1): ${firstError}. Retrying…`
+    );
+    try {
+      return await attemptCheck();
+    } catch (secondError) {
+      // Both attempts failed — proceed rather than silently dropping the ingest. A transient
+      // API hiccup or token-scope issue should not cause permanent data loss.
+      core.warning(
+        `AgentMeter: could not check conclusion job status (attempt 2): ${secondError}. Proceeding.`
+      );
+      return true;
+    }
   }
 }
 
@@ -334,9 +346,9 @@ async function resolveTrigger({
     }
   }
 
-  // gh-aw issue branches are named agent/issue-N — require the full prefix to
-  // avoid mismatching unrelated branches like feature/fix-issue-12-auth.
-  const issueMatch = headBranch.match(/\bagent\/issue-(\d+)\b/);
+  // gh-aw issue branches are named agent/issue-N — anchor to the full branch name to
+  // avoid substring matches in longer branch names like feature/agent/issue-12-fix.
+  const issueMatch = headBranch.match(/^agent\/issue-(\d+)$/);
   if (issueMatch?.[1]) {
     const num = parseInt(issueMatch[1], 10);
     return {