feat(github): add opt-in reviewMode for approve and request changes

[donnfelker]

Summary

Ticket: PT-618

• Adds github.reviewMode (comment | approve). A missing key stays comment, so existing runs still post a COMMENT review.
• approve requests changes when issues remain and approves a clean completed review. The same identity has to be able to clear its own change request, so there is no request-changes-only mode.
• An untouched prior issue is carried forward. A stale approval is dismissed before re-review. Incomplete runs never approve.
• Repo codegenie.toml may set only this GitHub key. CLI --review-mode and the Action review-mode input override it for one run and still require posting.

Test plan

• pnpm exec vitest run tests/review-verdict.test.ts tests/config-loader.test.ts tests/github-publisher.test.ts tests/github-action.test.ts tests/pipeline-phase7.test.ts
• Local approve on a clean PR: https://github.com/0xPolygon/ai-analytics/pull/120
• Local request-changes, fix, then re-approve: https://github.com/0xPolygon/ai-analytics/pull/121
• Confirm a workflow that does not set review-mode still posts COMMENT

[donnfelker]

Local approve-mode test

We tested this from a laptop against two pull requests Claude had opened in ai-analytics. The binary was this branch, not the published package. The model was OpenRouter's openai/gpt-5.6-luna at xhigh. The command was the same both times. Only the pull request number changed.

cd ~/source/polygon/ai-analytics
/Users/dfelker/source/polygon/codegenie/node_modules/.bin/tsx \
  /Users/dfelker/source/polygon/codegenie/src/cli/main.ts review \
  --pr 121 --post-github-comments --review-mode approve \
  --provider openrouter --model openai/gpt-5.6-luna:xhigh

--review-mode approve does both jobs. If the review finds a real issue, it requests changes. If a later run is clean, the same identity approves, which is the only way that identity can clear its own change request. There is no request-changes-only mode.

PR 120 was the clean case. It added a last-updated line to the README. The first run died in the model call and posted a comment, not an approval, because an incomplete review is not allowed to approve. The retry finished. At 20:18 UTC, donnfelker submitted Approved on commit a30b8bd. No findings.

PR 121 was the block-and-clear case. The title says do not merge. rangeForPreset was switched from UTC date getters to local ones. Codegenie reviewed that diff and, at 20:34 UTC, submitted Changes requested on web/lib/filters/date-range.ts. The review said the local calendar date was being passed into UTC boundaries, so the same instant could produce a different preset window. After the UTC getters were restored, the same command was run again. At 20:39 UTC, donnfelker submitted Approved on commit 716a5af. The open-issue marker on that review was empty. The earlier change request was cleared by the same reviewer.

So the local loop is: one flag, one identity, two outcomes. A clean README change gets an approval. A known timezone regression gets a change request, and the fix gets an approval from that same review.

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 10 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

0 confirmed findings retained from completed work. Unresolved questions require attention.

Coverage

Reviewed 62/70 hunks.
Excluded by configuration/planning: 8 hunks.
Coverage levels: deep 21, normal 28, light 13, skip 8.

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs Human Attention

• Does buildReviewOptions' conditional spread (only setting skipGithubInlineComments when truthy) correctly reflect commander's flag, i.e. is there no --no-skip-github-inline-comments variant whose explicit false must be forwarded?; Is --review-mode/--skip-github-inline-comments CLI parsing covered anywhere (no matches in tests/review-command.test.ts), e.g. the guard at review-command.ts:229 rejecting --review-mode without --post-github-comments and parseReviewMode rejecting unknown values?; Does the conditional spread of skipGithubInlineComments at src/cli/review-command.ts:184 correctly preserve an explicit false CLI/config value, i.e. is the override merged over config so that omitting the key falls back to config rather than silently dropping a user-specified false?; Where is parsed.options.skipGithubInlineComments produced (parseReviewCommand/buildCliOverrides) and can it be explicitly false versus undefined?; Are there CLI parse tests asserting that --review-mode approve without --post-github-comments (and without --pr) is rejected with invalid_args?; Given that the Action-side parseReviewMode returns a widened string while the CLI copy returns GitHubReviewMode, should the Action copy be retyped to GitHubReviewMode — i.e. does GitHubActionInputs.reviewMode's declared type currently permit assigning an arbitrary string without a compile error?; On the opts.skipInlineComments=true path, are inlineCandidates routed into demoted (and thus into appendDemotedFindings) or genuinely dropped, and if dropped, is losing those finding details from the posted review intentional?; Does runReview actually honor overrides.skipGithubInlineComments when publishing (i.e., is it threaded into the publisher's inline-comment path), and is there a default-on test pinning that approve mode keeps inline comments?; If the pipeline aborts or errors after dismissStaleApproval (review-runner.ts:187) but before maybePublishToGitHub, the prior approval is dismissed with no replacement review posted; is that the intended end state for failed runs?; If a run dismisses the stale approval at stage 2 (review-runner.ts:187) and then aborts (hard timeout, LLM failure, exception) before maybePublishToGitHub posts, the PR is left with neither the old approval nor a new verdict. Is that abandonment window acceptable, or should dismissal happen at posting time?

  • Files: src/cli/review-command.ts, src/config/schema.ts, src/github-action/entrypoint.ts, src/github/publisher.ts, src/pipeline/review-runner.ts, src/types.ts
  • Symbols: CommanderReviewOptions, GitHubActionInputs, GitHubReviewMode, RunReviewOverrides, buildCliOverrides, buildPostingBody
  • Reason: The packet only shows the optional field added to the RunReviewOverrides type; if runReview never reads it, callers setting the flag would silently still post inline comments. Commit title 'test(action): pin that approve mode keeps inline comments by default' suggests this flag gates inline posting. Related reasons: Packet reviewer could not resolve this question from the reviewed context. The added line only forwards the flag when truthy, so a CLI/parsed false can never override a config value of true. If parsed.options.skipGithubInlineComments can be explicitly false (e.g. a --no- style flag), approve-mode runs would still skip inline comments contrary to the user's request. Grouped from 10 related hints across 9 packets.

• When postWithRecovery demotes an APPROVE/REQUEST_CHANGES review to COMMENT on an own-PR 422, the already-finalized body keeps the verdict text ('Approved.' / 'Changes requested...') and the verdict=approve marker; is that intended for the recorded verdictFallback='own_pr' path?; Can a non-verdict 422 (e.g. invalid inline comment path) ever echo the phrase "own pull request" through error.message/stderr/stdout/responseBody, e.g. when the review body text itself is echoed back by gh, causing an unnecessary COMMENT demotion with record.verdictFallback="own_pr"?; Is the verdict marker appended at the end of the posted review body (after reviewBody) by the finalize callback in postWithRecovery/createReview?; When reviewMode="approve" and publishableCount===0 with summaryWhenNoFindings=false, is every resulting posting path guaranteed a non-empty body (selectPostedEvent must yield forcePost so forcedFallbackBody applies), or can shouldPostBody be false and leave the run with a posting plan but nothing posted?; Are record.reviewEvent / record.verdictFallback consumed anywhere outside src/github/publisher.ts (e.g. telemetry or action summary output), and does any consumer assume reviewEvent reflects the requested verdict rather than the demoted COMMENT event?

  • Files: src/github/github-client.ts, src/github/publisher.ts, src/pipeline/composer.ts, src/types.ts
  • Symbols: RunPostingRecord, dedupeRankAndComposeReview, forcedFallbackBody, formatVerdictMarker, isOwnPullRequestReview, postWithRecovery
  • Reason: Packet reviewer could not resolve this question from the reviewed context. Grouped from 5 related hints across 5 packets.

• Does anchorSettled treat a ComparedFileLines entry with patchMissing=true (file over GitHub's diff-size limit) as unsettled, as the new compareFiles comment claims?; If GitHub ever omits changes on a compare file entry that also has no patch, patchMissing becomes false and rebaseOntoHead drops the commit= origin while shifting by empty add/delete sets; is changes guaranteed present in the compare files payload?; Does any caller invoke compareFiles when the prior review commit equals the PR head (status "identical"), and is that accepted path covered by a test?; Do the stubbed gh payload shapes (pure rename with changes:0 and no patch, missing patch with changes>0) match real GitHub compare responses closely enough that patchMissing semantics are validated?

  • Files: src/github/github-client.ts, src/github/review-verdict.ts, tests/github-client.test.ts
  • Symbols: ComparedFileLines, GhCompareFile, anchorSettled, carriedOpenIssues, compareFiles, rebaseOntoHead
  • Reason: compareFiles accepts status "ahead" or "identical" and throws otherwise; the new tests only pin "ahead" (accept) and "diverged" (reject). If re-reviewing an unchanged head reaches compareFiles with identical SHAs, dropping "identical" from the guard would throw github_post_failed on a common path and the inspected tests would still pass. Related reasons: Packet reviewer could not resolve this question from the reviewed context. Grouped from 4 related hints across 3 packets.

• Does REPO_SAFE_REVIEW_KEYS in src/config/config-loader.ts include "github.reviewMode" so repo codegenie.toml can actually set it, and does the schema validate the "comment"|"approve" enum before defaultSources records the source path?; Does applyRepoConfigLayer (which runs after loadConfig and applies repo codegenie.toml via applyRawConfig with a freshly built defaultSources() map) overwrite a CLI-set github.reviewMode, and is a repo downgrade of --review-mode approve to comment the intended precedence?; Should the new test also pin that a repo-config reviewMode = "approve" does not clear an already-approved home/CLI value (filterRepoConfig drops the key entirely rather than forcing comment)?

  • Files: src/config/config-loader.ts, src/config/schema.ts, src/pipeline/review-runner.ts, tests/config-loader.test.ts
  • Symbols: REPO_SAFE_REVIEW_KEYS, applyCliOverrides, applyRawConfig, applyRepoConfigLayer, defaultSources, filterRepoConfig
  • Reason: applyCliOverrides records sources["github.reviewMode"]="cli", but applyRepoConfigLayer clones the already-CLI-resolved config, builds a local sources map from defaultSources(), and applyRawConfig sets config.github.reviewMode unconditionally when raw.github.reviewMode is defined (line 293-295). A repo file with reviewMode="comment" (the only repo-safe value) would then silently override an explicit CLI/Action approve and the cli source record is discarded. Declared intent says CLI/Action input overrides repo config for one run, while the new filterRepoConfig comment says repo may only lower the ceiling, so the intended precedence needs author confirmation. Related reasons: Packet reviewer could not resolve this question from the reviewed context. The packet only shows the new source-path entry; the PR states repo config may set this GitHub key, so the allow-list and enum validation live outside the shown hunk and were not inspectable here. Grouped from 3 related hints across 3 packets.

• In the new publisher tests, inline-comment fixtures (e.g. the c1 comment on review 11 at tests/github-publisher.test.ts:1136-1145) omit lineIsCurrent, so issuesFromReview classifies them as lineBasis "previous". Does the real GitHubClient.listOwnComments set lineIsCurrent=true for non-outdated comments, meaning these tests exercise the previous-basis rebase path rather than the production-typical current-basis path?; Does tests/github-publisher.test.ts exercise publisher.ts byOrigin grouping where two carried issues have different numbering commits (issue.commit set vs. source.commitId), so each group is compared from its own origin?

  • Files: src/github/github-client.ts, src/github/publisher.ts, src/github/review-verdict.ts, tests/github-publisher.test.ts, tests/review-verdict.test.ts
  • Symbols: carryForwardIssues, fakeGithub, issuesFromReview, listOwnComments, rebaseOntoHead
  • Reason: tests/review-verdict.test.ts only calls carryForwardIssues with origin equal to the issue's own commit, so a regression in publisher.ts:274-285 grouping (e.g. comparing every issue from source.commitId and ignoring issue.commit) would shift lines against the wrong compare window and still pass the unit tests. Related reasons: Packet reviewer could not resolve this question from the reviewed context. Grouped from 2 related hints across 2 packets.

Additional unresolved notes suppressed: 5.

Stats

• 🤖 Model: anthropic claude-opus-5 high
• 🧞 Codegenie: v0.7.0 (8003bb804d)
• Elapsed time: 8m 57s
• Git: 0xPolygon/codegenie from master to feat/github-review-mode (74f557b961)
• Review completeness: complete.
• Usage: model calls 150, tokens 4276055, cost $12.6228.
• Effective caps: tokens 8000000.
• Local context pressure: 22 degraded tool results, 13 degraded hunks, 5 unresolved notes suppressed.

No confirmed findings

No confirmed findings were retained. The limitations above prevent a clean conclusion.

— View Workflow Job

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 18 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

7 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 44/61 hunks.
Excluded by configuration/planning: 17 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Should carriedOpenIssues also recover open issues from a prior review whose REQUEST_CHANGES degraded to a COMMENT review (own-PR 422 fallback or carriedLookup.unknown), given it only reads the latest review when state === "CHANGES_REQUESTED" even though the body still carries a verdict marker with the open list?; Does GitHub surface the "can not approve your own pull request" 422 only after inline comment validation passes, making the post-loop (zero-comment) submission the first attempt that can see it?; Does the gh CLI error for an own-PR approve reliably expose the text "own pull request" in context.stderr/stdout/responseBody, given normalizeCreateReviewError replaces error.message with "GitHub review creation failed with HTTP 422"?; Are own-review inline comment fingerprints ever parsed as empty strings (duplicate-detector uses fingerprint: match[1] ?? ""), which issuesFromReview would accept and then dedupe into a single permanently-carried pseudo-issue?; Does GitHub's dismissals endpoint reject self-dismissal or non-admin dismissal with 422 rather than 403, making the 403-only tolerance branch ineffective in practice?; Is the carriedLookup.unknown → forced COMMENT guard (src/github/publisher.ts:152-154) exercised anywhere, i.e. a test where listOwnReviews rejects and the run must not APPROVE?; Does any caller above carriedOpenIssues (maybePublishToGitHub in src/github/publisher.ts) catch a synchronous URIError from issuesFromReview, or does it abort the GitHub posting step?; Can a prior codegenie review body realistically contain a malformed percent escape (edited by a collaborator, or body truncation), given the bot writes the marker itself?
• Is the new --review-mode flag (validation via parseReviewMode and the --post-github-comments requirement in resolveTarget) covered by any test, given tests/review-command.test.ts contains no reviewMode references?; If a third mode is added to githubReviewModeSchema in src/config/schema.ts, would the duplicated validators silently reject it (no compile-time check links them)?; Does the CLI guard at src/cli/review-command.ts:226 (--review-mode requires --post-github-comments) run before cli.reviewMode is assigned at line 335/336 on every invocation path, so an approve mode can never reach config without posting enabled?; Should the duplicated reviewMode validators be refactored to reuse githubReviewModeSchema as the single source of truth?; Does codegenieConfigSchema/githubReviewModeSchema define reviewMode with default "comment" so merged configs that omit the key match defaultConfig?; Does the GitHub Action input path ever need to distinguish an explicitly empty review-mode input from an unset one (entrypoint line 348 ignores whitespace-only values)?; Does any other case in tests/github-action.test.ts drive executeGitHubActionCommand with --review-mode comment and --post-inline-comments false and assert --review-mode is not forwarded?
• Is the schema entry for github.reviewMode (comment|approve, default comment) defined in src/config/schema.ts so the new source path resolves to a real key?; Is it acceptable for a repo-controlled codegenie.toml on the PR branch to escalate the bot from COMMENT to APPROVE/REQUEST_CHANGES when posting is enabled?; In the GitHub Action path, is codegenie.toml read from the PR head checkout (attacker-controlled) rather than the base branch, and does the action run with a token whose APPROVE review can satisfy branch protection?; Does rawConfigSchema strip unknown github keys before filterRepoConfig, so the warning loop can only ever report known-but-unsafe keys?
• Given the Action passes neither flag, does review-runner's guard silently skip the verdict for a repo-level github.reviewMode = "approve", and is that silent ignoring intended?; Should an explicit Action input review-mode: comment still override a repo config github.reviewMode = approve when post-inline-comments is false?
• Can a non-403 failure from github.dismissReview (e.g. 404/422 when the review is not dismissible, or auth errors) abort the whole review run before any review is produced, since dismissStaleApproval rethrows every non-403 error at stage 1?; If the run fails or hard-aborts after dismissStaleApproval has already dismissed the prior approval (dismissal happens at stage 1, before review work), the PR is left with no approval and no replacement review. Is that the intended trade-off?
• Additional unresolved notes suppressed: 13

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 13 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

6 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 50/68 hunks.
Excluded by configuration/planning: 18 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

Summary-only findings:

• 🔵 Medium: LEFT-anchored carried issues can never settle, so approve mode can never re-approve the PR (src/github/review-verdict.ts)
  Impact: A finding anchored on a deleted line (side: "LEFT") is recorded in the verdict marker by formatVerdictMarker. On a later run parseOpenList rebuilds it with lineBasis: "previous" (review-verdict.ts:200), and issuesFromReview also assigns "previous" when GitHub returns the inline comment as outdated (github-client.ts sets lineIsCurrent only when comment.line is a number). anchorSettled then takes the previous-basis branch at line 182, which returns true only for side === "RIGHT", so the LEFT issue never settles unless the file is added, removed or renamed. carryForwardIssues keeps it, publisher.ts:146-151 folds it into openIssueCount, selectPostedEvent returns REQUEST_CHANGES, and publisher.ts:168 re-writes the same entry into the new marker — repeating on every run even after the author fixed the issue, because the finding is no longer re-published and its fingerprint cannot clear the carry. In approve mode the PR is then stuck at CHANGES_REQUESTED: no head commit clears it, since the LEFT line lives in the PR base coordinate space that compare(previousCommit, headSha) never expresses. Only a human dismissal (or renaming/deleting the file) unblocks merges gated on codegenie approval, while RIGHT-side issues do settle via file.deletedLines. The LEFT special-case looks deliberate, so the permanent-carry consequence needs author confirmation.

  Verification and uncertainty: LEFT anchors are demonstrably reachable: src/llm/schemas.ts:18 allows side: "LEFT", src/pipeline/planner.ts:1597 and src/pipeline/pipeline-utils.ts:138 build them, and src/github/publisher.ts:361,383 posts inline comments with the anchor's LEFT side. Basis assignment is confirmed at review-verdict.ts:200 and in github-client.ts (outdated comments have line null, so lineIsCurrent stays undefined and issuesFromReview maps to "previous"). Non-settlement is review-verdict.ts:181-182. The self-sustaining loop is publisher.ts:146, :151, selectPostedEvent:31-33 and :168, with carriedOpenIssues returning empty only when the latest own review is APPROVED or DISMISSED. Impact is bounded to opt-in approve mode and to findings anchored on deleted lines. Two conditions remain open: if GitHub reported the comment with a numeric line on every later run it would route to the current-basis branch, though the marker copy still uses "previous" and deduplication prefers the comment entry, so the stuck case requires the comment to be outdated or missing; and it is unconfirmed whether the current base→head UnifiedDiff is in scope at the carriedOpenIssues call site, which leaves the feasibility and signature of the natural correction unverified. No remedy or regression test is recommended here for that reason — the proposals on file depend on that unverified plumbing and are retained as provenance.

  Remediation remains unverified; original proposals and assessments are retained below.

  Regression-test guidance remains unverified; original proposals and assessments are retained below.

  Evidence:

  src/github/review-verdict.ts (source)

  // src/github/review-verdict.ts
  function anchorSettled(issue: OpenReviewIssue, files: ComparedFileLines[] | undefined): boolean {
    ...
    // A marker line is numbered on the previous commit. Compare deleted lines, not new-head added lines.
    if (issue.lineBasis !== "current") {
      return issue.side === "RIGHT" && file.deletedLines.includes(issue.line); // LEFT can never settle
    }
    const lines = issue.side === "LEFT" ? file.deletedLines : file.addedLines;
    return lines.includes(issue.line);
  }
  
  function parseOpenList(raw: string): OpenReviewIssue[] {
    ...
    return [{ fingerprint, path: decodeURIComponent(encodedPath), line: Number(line), side, lineBasis: "previous" }];
  }

  src/github/publisher.ts (source)

  const carriedLookup = mode === "comment" ? { issues: [], unknown: false } : await carriedOpenIssues(github, resolved.pr.number, resolved.pr.headSha, published);
  const carried = carriedLookup.issues;
  let decision = selectPostedEvent({ mode, health: healthForResult(finalReview).status, openIssueCount: published.length + carried.length });
  ...
  const verdictMarker = mode !== "comment" && reviewBody.trim().length > 0
    ? formatVerdictMarker(mode, resolved.pr.headSha, [...issuesFromFindings(published), ...carried])
    : undefined;

  Carried issues feed openIssueCount (REQUEST_CHANGES when > 0) and are re-serialized into the new marker, making the unsettleable LEFT entry permanent.

  src/github/github-client.ts (source)

  const line = typeof comment.line === "number" ? comment.line : typeof comment.original_line === "number" ? comment.original_line : undefined;
  ...
  if (typeof comment.line === "number") { thread.lineIsCurrent = true; }

  Outdated comments (line null) yield lineIsCurrent undefined, so issuesFromReview assigns lineBasis "previous" - the branch that can never settle a LEFT anchor.

  Original assessments and supporting evidence (may overlap or disagree)

  Original source material is retained for audit. The current conclusion is above; superseded assessments are labeled where supplied. Attribution does not prove semantic equivalence.

  Original candidate metadata:

  • 94a3d9cf-u1-bd6db205: severity medium, confidence medium

  Original impact: 94a3d9cf-u1-bd6db205/failureMode

  A finding anchored on a deleted line (side "LEFT") is recorded in the verdict marker by formatVerdictMarker. On a later run, parseOpenList rebuilds it with lineBasis "previous" (review-verdict.ts:200), and issuesFromReview also assigns "previous" when GitHub returns the inline comment as outdated (github-client.ts sets lineIsCurrent only when comment.line is a number). anchorSettled then hits the previous-basis branch at line 182, which returns true only for side === "RIGHT", so the LEFT issue is never settled unless the file is added/removed/renamed. carryForwardIssues keeps it, publisher.ts:146-151 adds it to openIssueCount, selectPostedEvent returns REQUEST_CHANGES, and publisher.ts:168 re-writes the same entry into the new marker. The cycle repeats on every subsequent run, even after the author has fixed the issue (the finding is no longer re-published, so its fingerprint is not in currentFingerprints and cannot clear the carry).
  

  Original impact: 94a3d9cf-u1-bd6db205/whyThisMatters

  In `approve` mode the PR is permanently stuck at REQUEST_CHANGES once a single deleted-line (LEFT) finding has been published and later addressed: no head commit can clear it, because the LEFT line number lives in the PR base coordinate space that compare(previousCommit, headSha) never expresses. Only a human dismissing the review (or renaming/deleting the file) unblocks merges that gate on codegenie approval. RIGHT-side issues do settle via file.deletedLines, so the behavior is asymmetric and surprising.
  

  Original verification: 94a3d9cf-u1-bd6db205/verification

  Reachability of LEFT anchors: src/llm/schemas.ts:18 allows side "LEFT"; src/pipeline/planner.ts:1597 and src/pipeline/pipeline-utils.ts:138 build LEFT anchors; src/github/publisher.ts:361,383 posts inline comments with the anchor's LEFT side. Basis: src/github/review-verdict.ts:200 (parseOpenList hardcodes lineBasis "previous") and src/github/github-client.ts (thread.lineIsCurrent = true only when comment.line is a number; otherwise original_line is used and issuesFromReview maps to "previous"). Non-settlement: review-verdict.ts:181-182 returns `issue.side === "RIGHT" && file.deletedLines.includes(issue.line)`. Impact path: publisher.ts:146 (carriedOpenIssues), :151 (openIssueCount = published.length + carried.length), selectPostedEvent lines 31-33 (REQUEST_CHANGES), :168 (carried re-serialized into the next marker); carriedOpenIssues only returns empty when the latest own review is APPROVED/DISMISSED, so the carry is self-sustaining. Impact bound: opt-in approve mode only, and only for findings anchored to deleted lines.
  

  Original fix: 94a3d9cf-u1-bd6db205/suggestedFix

  Stop treating previous-basis LEFT anchors as permanently open in anchorSettled. A LEFT line number is in the PR base coordinate space, which compare(previousCommit, headSha) cannot express, so it must be checked against the current PR diff instead: settle the carried issue when the recorded deletion at `path:line` is no longer present as a deleted line in the head's base->head diff (the publisher already walks that UnifiedDiff for `delete` lines with oldLineNumber in deletedFileAnchorKeys). Keep it unsettled while that deletion still exists, so genuinely open deleted-line findings continue to force REQUEST_CHANGES.
  

  Suggestion assessment: unverified

  The remedy preserves the established requirement in principle: it only settles a carried LEFT issue when the recorded deletion has disappeared from the current base->head diff, mirroring the RIGHT rule (anchored line changed) rather than approving unconditionally, so a still-deleted line keeps forcing REQUEST_CHANGES. I could not verify that the current PR UnifiedDiff is in scope at the carriedOpenIssues/carryForwardIssues call site (publisher.ts:146); deletedFileAnchorKeys(diff) shows such a diff with LEFT oldLineNumber data exists in publisher, but its availability on this path and the resulting signature change remain unchecked, so feasibility is unconfirmed.

  Behavioral requirement (established): In approve mode a previously reported issue must keep producing REQUEST_CHANGES while it is still open, and the PR must become approvable again once the anchored code has changed (the rule anchorSettled already implements for RIGHT anchors).

  src/github/review-verdict.ts

  if (issue.lineBasis !== "current") {
      return issue.side === "RIGHT" && file.deletedLines.includes(issue.line);
    }
  

  Shows the asymmetric rule the fix must make symmetric without weakening the still-open case.

  src/github/publisher.ts

  function deletedFileAnchorKeys(diff: UnifiedDiff): Set<string> {
    ... if (line.kind === "delete" && line.oldLineNumber !== undefined) {
            keys.add(anchorKey({ path, line: line.oldLineNumber, side: "LEFT", hunkId: hunk.id }));
  

  Demonstrates the publisher has base-side (oldLineNumber) diff data of the kind the fix needs, but not that it is in scope at the carry-forward call site.

  Original test: 94a3d9cf-u1-bd6db205/suggestedTest

  Add a unit test for carryForwardIssues/anchorSettled with a previous-basis LEFT issue (fingerprint not in currentFingerprints) on a modified file, asserting both directions: (a) when the recorded deleted line is still deleted in the current base->head diff, the issue is still carried (openIssueCount > 0, REQUEST_CHANGES) - this rejects a blanket "always settle LEFT" shortcut; (b) when that deletion is gone from the current diff, the issue is dropped so selectPostedEvent({mode:"approve", health:"completed", openIssueCount:0}) returns APPROVE - this fails on today's code, which carries the issue forever. Keep the existing RIGHT previous-basis assertions unchanged.
  

  Suggestion assessment: unverified

  Case (b) fails on today's code (the LEFT previous-basis issue is carried forever) so it rejects the observed defect, and case (a) asserts a still-deleted LEFT line remains carried, which rejects the symptom-hiding shortcut of settling every LEFT previous-basis issue (or of dropping carried issues whenever the file was merely touched). It therefore accepts the selected remedy while rejecting weakened variants. It is marked unverified only because the exact input shape depends on the unconfirmed fix signature (what current-diff data is plumbed into carryForwardIssues), and I did not inspect the existing review-verdict test file to confirm sister-test conventions.

  Behavioral requirement (established): Approve mode must keep requesting changes while a reported deleted-line issue is still present in the diff, and must be able to approve again once that anchored deletion is gone.

  src/github/review-verdict.ts

  export function carryForwardIssues(prior, currentFingerprints, files) {
    return prior.filter((issue) => {
      if (currentFingerprints.has(issue.fingerprint)) { return false; }
      return anchorSettled(issue, files) !== true;
    });
  }
  

  carryForwardIssues and selectPostedEvent are exported, so the boundary is directly reachable from a unit test.

  src/github/publisher.ts

  openIssueCount: published.length + carried.length
  

  Links the carried-issue count asserted by the test to the REQUEST_CHANGES/APPROVE decision.

  Original test (before verification revision; provenance only): 94a3d9cf-u1-bd6db205/originalSuggestions/suggestedTest

  Add or update a regression test that exercises the referenced changed path.
  

  Suggestion assessment: unverified

  Behavioral requirement compatibility was not assessed.

  Original verification: 94a3d9cf-u1-bd6db205/proofAssessment

  Proof status: established

  review-verdict.ts:181-182 returns settled only for side "RIGHT" on the previous basis; parseOpenList (line 200) always emits lineBasis "previous", and github-client.ts sets lineIsCurrent only when comment.line is numeric, so outdated LEFT comments also arrive as "previous". LEFT anchors are demonstrably published (schemas.ts:18, planner.ts:1597, pipeline-utils.ts:138, publisher.ts:361/383 posting side: anchor.side). publisher.ts:146-151 counts carried issues into openIssueCount and selectPostedEvent lines 31-33 returns REQUEST_CHANGES for any count > 0; publisher.ts:168 re-persists carried entries into the next marker, and carriedOpenIssues only short-circuits when the latest own review is APPROVED/DISMISSED, so the carry self-sustains across runs.

  • Secondary assumption: Does GitHub ever report the codegenie LEFT comment with a numeric line on every subsequent run (lineIsCurrent true), which would route it to the current-basis branch instead? Even then the marker copy uses "previous", and deduplication prefers the comment entry, so the stuck case requires the comment to be outdated or missing.
  • Secondary assumption: Is the current base->head UnifiedDiff accessible at the carriedOpenIssues call site so the suggested remedy can be implemented there?

  Original evidence: 94a3d9cf-u1-bd6db205/evidence/changedCode

  src/github/review-verdict.ts

  // src/github/review-verdict.ts
  function anchorSettled(issue: OpenReviewIssue, files: ComparedFileLines[] | undefined): boolean {
    ...
    // A marker line is numbered on the previous commit. Compare deleted lines, not new-head added lines.
    if (issue.lineBasis !== "current") {
      return issue.side === "RIGHT" && file.deletedLines.includes(issue.line); // LEFT can never settle
    }
    const lines = issue.side === "LEFT" ? file.deletedLines : file.addedLines;
    return lines.includes(issue.line);
  }
  
  function parseOpenList(raw: string): OpenReviewIssue[] {
    ...
    return [{ fingerprint, path: decodeURIComponent(encodedPath), line: Number(line), side, lineBasis: "previous" }];
  }

  Original evidence: 94a3d9cf-u1-bd6db205/evidence/relatedCode/0

  src/github/publisher.ts

  const carriedLookup = mode === "comment" ? { issues: [], unknown: false } : await carriedOpenIssues(github, resolved.pr.number, resolved.pr.headSha, published);
  const carried = carriedLookup.issues;
  let decision = selectPostedEvent({ mode, health: healthForResult(finalReview).status, openIssueCount: published.length + carried.length });
  ...
  const verdictMarker = mode !== "comment" && reviewBody.trim().length > 0
    ? formatVerdictMarker(mode, resolved.pr.headSha, [...issuesFromFindings(published), ...carried])
    : undefined;

  Carried issues feed openIssueCount (REQUEST_CHANGES when > 0) and are re-serialized into the new marker, making the unsettleable LEFT entry permanent.
  

  Original evidence: 94a3d9cf-u1-bd6db205/evidence/relatedCode/1

  src/github/publisher.ts

  const leftSide = comments.filter((comment) => comment.anchor.side === "LEFT");
  ...
      side: anchor.side,

  Proves findings with side "LEFT" are actually published as inline review comments, so the unsettleable case is reachable.
  

  Original evidence: 94a3d9cf-u1-bd6db205/evidence/relatedCode/2

  src/github/github-client.ts

  const line = typeof comment.line === "number" ? comment.line : typeof comment.original_line === "number" ? comment.original_line : undefined;
  ...
  if (typeof comment.line === "number") { thread.lineIsCurrent = true; }

  Outdated comments (line null) yield lineIsCurrent undefined, so issuesFromReview assigns lineBasis "previous" - the branch that can never settle a LEFT anchor.
  

🙋 Needs human attention:

• Is config-only github.reviewMode="approve" expected to post a verdict from the Action when post-inline-comments=false (i.e. should the code be fixed), or is that combination intentionally unsupported so the action.yml wording should change instead?; When overrides.skipGithubInlineComments is set and all findings are inline-publication, does any caller (e.g. src/pipeline/review-runner.ts) still expect those findings to appear in the review body?; Is there any path where a REQUEST_CHANGES review is submitted with prepared.length > 0 and shouldPostBody false (empty body), causing GitHub's 422 'Body can't be blank' and losing the verdict marker for the next run?; In approve mode with zero publishable findings and an incomplete/partial run, is posting the forced-fallback COMMENT review (rather than skipping) the intended behavior, including the body content contributed via src/pipeline/composer.ts?; Is the new zero-work postingPlan in approve mode (config.github.reviewMode !== "comment") intended to post a review even when summaryWhenNoFindings is false?; Does tests/github-publisher.test.ts drive an approve-mode run with an empty review body so the forcePost-gated fallback in publisher.ts is exercised end-to-end?
• Does an invalid review-mode input value (e.g. "request-changes") fail the Action run with a clear invalid_args error, and is that behavior covered by github-action tests?; Does a CLI-side test assert that --review-mode <invalid> throws invalid_args (the action-side equivalent exists at tests/github-action.test.ts:1071)?; If github.reviewMode later gains a third value, will the duplicated accepted-value lists in src/cli/review-command.ts parseReviewMode and src/github-action/entrypoint.ts parseReviewMode be updated together (the action copy returns string, so a drift would not be caught by typecheck)?; Does any other test (e.g. tests/pipeline-phase7.test.ts or tests/github-publisher.test.ts) pin that approve mode with inline comments enabled does not emit --skip-github-inline-comments?; Do the toContain("--review-mode") / toContain("approve") assertions need adjacency checking, i.e. could argv ever place the value non-adjacently and break commander parsing?
• Should compareFiles paginate the GitHub compare endpoint (per_page/page), given the API caps the files array (~300 entries) for large diffs?; After review-runner dismisses a stale approval (src/pipeline/review-runner.ts:933-938), latestSubmittedReview() filters out DISMISSED reviews, so carriedOpenIssues() picks the next-older review (e.g. an earlier CHANGES_REQUESTED). Can already-resolved issues from that older review be re-carried and force REQUEST_CHANGES on a clean run?; carriedOpenIssues() calls github.listOwnComments(prNumber) a second time (maybePublishToGitHub already fetched the same list before calling it) and that call is outside the try/catch that produces unknown:true; is the duplicate fetch intentional, and should its failure also map to unknown:true rather than aborting publishing?; Does tests/github-publisher.test.ts already assert that carriedLookup.unknown downgrades APPROVE to COMMENT and that listOwnComments failures are tolerated?; Is the stale-approval dismissal wiring in src/pipeline/review-runner.ts (staleApproval -> github.dismissReview at ~lines 933-938) covered by any test that asserts dismissReview is actually invoked with the stale review id, or do all fakes only stub it to undefined?
• Is it intended that an untrusted fork/PR-head codegenie.toml with [github] reviewMode = "approve" is trusted to enable approve/request-changes posting when the Action runs with post-inline-comments and no explicit review-mode input?; Does filterRepoConfig's safe.github = { reviewMode } reconstruction drop other allowed github keys (e.g. summaryWhenNoFindings) from repo codegenie.toml?
• Does the mapping from GitHubReviewMode "approve" to the GitHubReviewEvent "REQUEST_CHANGES" branch correctly cover the incomplete-run case (never approve) in review-verdict.ts?; Is parseVerdictMarker/parseOpenList rejection of malformed markers (bad fingerprint, non-numeric line, missing side, absent marker) covered anywhere? The new suite only asserts the happy round trip.
• Additional unresolved notes suppressed: 8

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

Re the summary-only finding "LEFT-anchored carried issues can never settle": fixed in 9cad0ce. LEFT lines are numbered on the PR base, which the previous-to-head compare never covers. A carried LEFT issue (either line basis) now settles as soon as its file changes between the last review commit and the new head. If the problem is still there, the fresh review raises it again under the same fingerprint. If the file is untouched, the issue stays open. Covered in tests/review-verdict.test.ts.

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 13 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

4 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 59/68 hunks.
Excluded by configuration/planning: 9 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• With opts.skipInlineComments true in approve mode and a non-empty buildPostingBody result, are suppressed inline candidates added to demoted (or otherwise named) in the posted REQUEST_CHANGES body, given forcedFallbackBody only runs when reviewBody is empty?; carriedOpenIssues re-calls github.listOwnComments (publisher.ts:252) although maybePublishToGitHub already fetched the same list at line 133 and that result is in scope; is this an extra uncached gh API round-trip per approve-mode run?; Does dismissStaleApproval need the same gh client options (e.g. runGh/auth env) that publisher.ts passes, or is createGitHubClient(repoRoot) with defaults sufficient in GitHub Action runs?; Is it acceptable that the stale approval is dismissed at stage 1 even when the run later aborts, hard-timeouts, or maybePublishToGitHub throws (PR changed mid-run), leaving the PR with neither the old approval nor a new review?; Does the CLI --review-mode / Action review-mode override mutate config.github.reviewMode before runReview, so dismissStaleApproval's config.github.reviewMode !== "approve" gate sees the override?; Is posting a zero-work review body ("Nothing to review.") on every approve-mode run intended when github.summaryWhenNoFindings is false?; Does the publisher handle GitHub rejecting an APPROVE/REQUEST_CHANGES review on the bot's own PR (types.ts exposes verdictFallback: "own_pr", suggesting a fallback path) for every forcePost branch of selectPostedEvent?; When postWithRecovery exhausts retries and throws (status "failed"), record.reviewEvent keeps the intended event (e.g. APPROVE) set at init even though no review was created; is that acceptable for telemetry/report consumers that read reviewEvent without checking status?; Does any test assert that github.dismissReview is actually called when a stale APPROVED review exists on an older commit (src/pipeline/review-runner.ts:938)?; Does any test exercise the approve/request-changes reviewMode path end-to-end (listOwnReviews returning a stale approval, dismissReview being called, compareFiles driving LEFT carried issues) rather than only the empty-stub defaults added in fakeGithub?
• Do tests in tests/github-action.test.ts cover the empty INPUT_REVIEW_MODE case (always-appended --review-mode "" trimmed to undefined by parseGitHubActionArgs)?; With post-inline-comments true and an explicit --review-mode comment, does the entrypoint gate ever drop the flag such that a codegenie.toml-configured approve mode still takes effect?; Is dropping --review-mode when postInlineComments is false and reviewMode is "comment" the intended design?; Is the widened reviewMode?: string typing in src/github-action/entrypoint.ts intended rather than the shared GitHubReviewMode union?; Should the Action forward the posting flags in this case, or should action.yml's documented promise of a verdict for the config path be withdrawn instead?; Does any caller other than action.yml invoke codegenie github-action --review-mode with a value that is intentionally whitespace-only (which is silently ignored rather than rejected)?; Should src/github-action/entrypoint.ts:parseReviewMode return the shared GitHubReviewMode union (as src/cli/review-command.ts does) instead of string, so the two accepted-value lists cannot drift silently?
• Does the deploying GitHub Action workflow check out PR head/merge content (rather than the base ref), making the codegenie.toml read by loadConfig/applyRepoConfigLayer PR-author-controlled?; Is there an existing trust policy (docs or code) stating repo codegenie.toml is only honored from the base branch?; In the eval-runner path, applyRepoConfigLayer re-applies repo codegenie.toml (including the new github.reviewMode) on top of an already CLI-resolved config, so a repo-set reviewMode would win over --review-mode there; is that acceptable since evals never post GitHub reviews?; Does loadConfig surface an invalid repo-level github.reviewMode value as CodegenieError rather than silently dropping it (the new test's final assertion depends on parse failures being thrown, not warned)?
• Does src/github/github-client.ts compareFiles need pagination (per_page/page) for the compare endpoint, given GitHub returns at most 300 files per response, so files beyond that cap are absent from ComparedFileLines?; Is anchorSettled's LEFT rule (any added or deleted line in the file settles the issue) acceptable given that a carried LEFT issue is only re-raised when the current run independently re-detects it?; Is anchorSettled's file.status ("added"/"removed"/"renamed" -> settled) and patchMissing=true (-> not settled) path exercised by any test, and does leaving it uncovered allow a regression that silently drops or retains carried issues?
• Does any test cover the post-loop summary-only fallback (createReview after 3 failed attempts) with a non-COMMENT event, including an own-PR 422 at that point?; Does any posting path call finalizeReviewBody with a marker value different from the one already embedded by an earlier finalize pass (e.g. postWithRecovery fallback building a new body)?; If GitHub returns a self-review 422 whose body does not contain the literal phrase 'own pull request' (e.g. localized or reworded API message), isOwnPullRequestReview returns false and postWithRecovery keeps retrying with the APPROVE/REQUEST_CHANGES event, ending in the post-loop createReview that rethrows — is a hard posting failure the intended outcome in that case?
• Additional unresolved notes suppressed: 8

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 12 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

2 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 59/70 hunks.
Excluded by configuration/planning: 11 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• When review-mode=approve and post-inline-comments=false, does the publisher still post the verdict review body even if the body is empty (shouldPostBody false) so the Action's --skip-github-inline-comments combination is not a silent no-op?; When opts.skipInlineComments is true, duplicate detection still runs and skippedDuplicates is counted, so an approve-mode run with all findings suppressed inline can still report status 'skipped_all_duplicates' while reviewEvent is REQUEST_CHANGES; is that record combination expected by consumers of RunPostingRecord?; Does carriedOpenIssues need the same try/catch around github.listOwnComments that it has around listOwnReviews?; In approve mode with zero publishable findings and summaryWhenNoFindings=false, does publishReview always produce a non-empty body (forcedFallbackBody requires decision.forcePost) so the newly created posting plan does not end as a no-op/failed posting record?; Is the new RunReviewOverrides.skipGithubInlineComments actually consumed in runReview's GitHub publish path (and threaded from CLI/Action), or is it a declared-but-unused optional field?; On the zero-work short-circuit path (review-runner.ts ~line 1021) maybePublishToGitHub still runs while dismissStaleApproval is deliberately skipped; does the approve reviewMode publisher post a new APPROVE review for an exclusion-only push, contradicting the new comment's 'cannot earn a new approval' rationale?; Does setting coverage.partial = totalHunks > 0 in maybeZeroWork intentionally change default comment-mode output, so an exclusion-only push now renders the 'Review incomplete' health banner (renderReviewHealth) and the posting coverage disclosure (publisher.ts appendPostingCoverageDisclosure / shouldIncludeBase at line 432) that it previously did not?; Does any test assert that an exclusion-only zero-work run in reviewMode=approve posts COMMENT rather than APPROVE (i.e. covers maybeZeroWork's partial: totalHunks > 0)?; Does any test exercise carryForwardIssues with a non-empty currentFingerprints set (issue re-raised in the current run must not also be carried forward)?; Is the duplicated assertion at tests/review-verdict.test.ts:23 (identical to line 21) meant to cover a distinct case such as incomplete + openIssueCount > 0, or the forcePost flag for the incomplete branch?; Are anchorSettled's file.status === added/removed/renamed and patchMissing === true branches covered anywhere (no test fixture here sets status or patchMissing: true)?
• Does compareFiles need pagination for the compare endpoint, which returns at most 300 files per response and no files beyond that, so a large PR silently yields no ComparedFileLines entry for the omitted files?; Do tests/github-client.test.ts (or the other listed suites) cover the gh transport shapes — reviews list pagination, dismissals PUT path and body, and the compare URL — at a client level?; Is item.commitId being omitted when review.commit_id is falsy (and state defaulting to "") safe for latestSubmittedReview/carriedOpenIssues, which skip compare and carry all prior issues when commitId is undefined?; When compareFiles omits a file (pagination/>300 files) or reports patchMissing, carryForwardIssues never settles the issue; can an approve-mode PR then be stuck in permanent REQUEST_CHANGES even after the issue is fixed?; Does tests/review-verdict.test.ts (or tests/github-publisher.test.ts) cover a multi-generation carry-forward where a carried issue is re-encoded into a marker stamped with a newer commit?; Do GhPullReview.state values get normalized (e.g. uppercased/trimmed) before the publisher compares them against 'APPROVED'/'CHANGES_REQUESTED'? OwnPullRequestReview.state is an unconstrained string, so a case/format mismatch would silently skip dismissal of a stale approval.; In tests/github-publisher.test.ts:879 the override pr: { ...pr(), headSha: head }, headSha: head sets the same value pr() already returns ("h".repeat(40)), so the blocked/settled cases differ only in the compared fixture. Was a distinct head SHA (and therefore a different lineBasis path in anchorSettled) intended?
• Does the Action/CLI --review-mode override path still require posting when github.reviewMode is set only in repo codegenie.toml (i.e., config-sourced approve without --post-github-comments)?; Is --review-mode <value> value validation (parseReviewMode in buildCliOverrides, line 341) reached for every CLI path, given resolveTarget throws first when --post-github-comments is absent, so an invalid value like --review-mode bogus alone reports the posting-requirement error instead of the enum error?; Does a CLI --review-mode approve override have any effect (e.g. unexpected approval) when --post-github-comments is not passed, or is reviewMode only consumed on the posting path?; Does src/github-action/entrypoint.ts forward --review-mode to the review command without --post-github-comments when postInlineComments is false and reviewMode is "approve", and does src/cli/review-command.ts reject that combination as invalid_args?; Should src/github-action/entrypoint.ts:parseReviewMode be changed to return GitHubReviewMode to prevent future drift between the two accepted-value lists?
• Are summary-only findings (no anchor) intentionally excluded from the verdict marker's open list by issuesFromFindings?; When the own-PR 422 fallback downgrades the event to COMMENT, should the body's verdict marker still say mode=approve?; Does any test cover the post-loop summary-only fallback asserting record.reviewEvent equals the own-PR downgraded COMMENT event (and not the original APPROVE/REQUEST_CHANGES) after three failed attempts?; On re-finalize passes (demoteCommentsIntoBody and postWithRecovery fallback both re-run finalize on an already-finalized body), stripReviewBodyFooter only removes the trailing footer line, so a previously appended verdict marker stays inside stripped. Does sanitizeGitHubCommentBody strip HTML comments (as src/github-action/status-comment.ts:207-208 claims for its own marker), or can the body end up with two identical `` markers?; Does gh() populate context.stderr/responseBody with GitHub's 422 body text (e.g. "Can not approve your own pull request") for createReview failures? normalizeCreateReviewError in src/github/github-client.ts overwrites error.message with "GitHub review creation failed with HTTP 422", so isOwnPullRequestReview can only match via context.stderr/stdout/responseBody; if those are empty the own-PR fallback at publisher.ts:297 never triggers and an approve-mode run on a self-authored PR fails instead of degrading to COMMENT.
• Does repo-level codegenie.toml actually allow only lowering github.reviewMode to 'comment' (never raising to 'approve'), as README.md line 79 states, and does the Action 'review-mode' input / CLI --review-mode still require --post-github-comments?; Is the CLI example codegenie review --pr 123 --post-github-comments --review-mode approve valid given the README claim that only user config, --review-mode, or the Action input may set approve — i.e. is --review-mode accepted without any config key present?; Is it intended that a PR-author-controlled repo codegenie.toml can downgrade an org/user-config github.reviewMode = "approve" to "comment", suppressing the blocking REQUEST_CHANGES review (CLI/Action input still wins because it applies after the repo layer)?; Is the repo-config restriction in config-loader.ts (~lines 421-425) that accepts only github.reviewMode === "comment" from untrusted repo config intended, and does it error or silently drop "approve"?
• Additional unresolved notes suppressed: 7

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

codegenie review

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 13 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

3 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 63/70 hunks.
Excluded by configuration/planning: 7 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• When review-mode=approve and post-inline-comments=false, the Action now passes --post-github-comments; confirm the only publisher side effect added is the verdict review (publisher.ts prepared=[] via skipInlineComments) and not duplicate-thread/summary posting that users disabled via post-inline-comments=false.; When PublishOptions.skipInlineComments is true (prepared = [] at src/github/publisher.ts:138), does the value passed to issuesFromFindings(published)/selectPostedEvent still include all findings, or is published derived from prepared so a skip-inline run could approve a PR that still has findings?; Can overrides.skipGithubInlineComments be set while config.github.reviewMode is "comment" (CLI flag / Action input), or is it only wired with approve mode?; carriedOpenIssues calls github.listOwnComments(prNumber) a second time (the same list is already fetched earlier in maybePublishToGitHub for duplicate detection) and that call is not wrapped in try/catch unlike listOwnReviews; is the extra API call and its unguarded failure acceptable for approve mode?; Does a re-finalize pass (demoteCommentsIntoBody / postWithRecovery fallback) leave the prior verdict marker embedded in the body so the posted body contains two markers, and does parseVerdictMarker's first-match behavior then read the stale one?; If GitHub changes the self-review 422 wording (so /own pull request/ no longer matches), postWithRecovery throws instead of falling back to COMMENT; is a wording-independent signal (e.g. errors[].message resource/field or a dedicated code) available in the 422 payload to make the fallback more robust?; Is the verdict marker length bounded? finalizeReviewBody subtracts markerBlock.length from REVIEW_BODY_CAP, so a long open list (many carried issues with long paths) can make the cap argument negative or push the posted body past GitHub's limit.; Does importing createGitHubClient directly in review-runner.ts bypass the publisher abstraction/token-gating used by maybePublishToGitHub (e.g., dry-run or missing-token handling)?; After dismissStaleApproval() dismisses the prior APPROVED review at review-runner.ts:187, can carriedOpenIssues() in src/github/publisher.ts fall back to an older REQUEST_CHANGES review (latestSubmittedReview filters DISMISSED) and resurrect already-approved issues, turning the new verdict into REQUEST_CHANGES?; In the zero-work path with reviewMode=approve and an empty diff (totalHunks === 0, so coverage.partial=false and noFindings=true), maybeZeroWork posts a postingPlan without first dismissing a stale approval; does selectPostedEvent then post a fresh APPROVE that supersedes the stale one, or can it leave an approval pinned to an old commit?; In the zero-work path, when totalHunks === 0 (empty diff, or kept-file-less diffs whose files parse with no hunks such as pure renames/binary changes), coverage.partial stays false, so healthForResult yields a completed health and selectPostedEvent returns APPROVE with forcePost — should a run that reviewed nothing be able to post an APPROVE?; dismissStaleApproval runs at stage 2 before the rest of the pipeline; if the run later aborts (hard timeout/error) no replacement review is posted, leaving the PR with the prior approval removed. Is that the intended trade-off, or should dismissal be deferred to publish time?; Does github-client.ts set ExistingReviewThread.lineIsCurrent correctly (true only when the REST comment's line reflects the current diff, not when line is present but stale), given downstream carry-forward/anchoring consumers?; Does tests/review-verdict.test.ts cover anchorSettled/carryForwardIssues for a LEFT-side carried issue (settled via deletedLines) separately from RIGHT?; Should carryForwardIssues be tested with a non-empty currentFingerprints set so the re-raise dedupe branch (prior issue also reported this run) is protected?; Does any other inspected suite (e.g. tests/github-publisher.test.ts around lines 860-940) drive compareFiles with a file whose status is added/removed/renamed, covering the anchorSettled status branch that is also absent from these fixtures?
• Is the comment-only repo ceiling the intended, documented policy, given the PR summary says repo codegenie.toml may set this GitHub key?; Is this comment-only acceptance the documented repo-config policy in user-facing documentation?; In applyRepoConfigLayer (applied after CLI overrides in the PR-tree flow), a repo codegenie.toml with github.reviewMode = "comment" will overwrite a CLI/Action --review-mode approve. Is downgrading the CLI override intended, given the PR states CLI/Action override repo config?; Does tests/config-loader.test.ts assert that a repo codegenie.toml with github.reviewMode = "approve" is stripped (resolved mode stays "comment") and emits the github.reviewMode ignored warning, while "comment" is kept?; Did the pre-change sanitizer accept github.summaryWhenNoFindings from repo config or warn about it differently, i.e. is this a behavior change?; Does a repo codegenie.toml containing only non-reviewMode github keys (e.g. [github] summaryWhenNoFindings) still emit the per-key ignore warning now that warnTopLevelRepoSection was replaced by the custom github branch, and is that path asserted anywhere beyond this test?
• Does README.md:79's wording accurately describe this loader behavior (i.e., does the documented claim match the implemented restriction)?; Does githubReviewModeSchema reject non-literal values such as "APPROVE" or arbitrary strings, and is that validation applied before config-loader assigns config.github.reviewMode?; Is this restriction the intended repo-trust policy, reconciled with the PR summary's statement that repo codegenie.toml may set the GitHub reviewMode key?; Does config-loader.ts merge order guarantee that a repo codegenie.toml / CLI --review-mode value overrides defaultConfig.github.reviewMode rather than the default re-applying "comment"?; Does action.yml's review-mode input description state this restriction accurately?
• Does the Action-produced review-mode value actually flow through a re-validating path (CLI parse and/or schema parse) before it is assigned to github.reviewMode?; Should GitHubActionInputs.reviewMode be retyped to the GitHubReviewMode union (and the duplicated parser consolidated), considering the declaration site and its consumers?; At src/github-action/entrypoint.ts:227, --review-mode is forwarded when (postInlineComments || reviewMode==="approve"), but review-command.ts:229 rejects --review-mode unless --post-github-comments is set; confirm the arg list always includes --post-github-comments in those branches so approve runs do not fail with invalid_args.; Should the Action also forward --review-mode comment when post-inline-comments is true (currently it does), and is that combination covered anywhere?
• Does the GitHub review-creation API accept the always-present comments: [] plus a non-empty body for event APPROVE, as emitted by the widened createReview payload?; Does GitHub's createReview surface the own-PR 422 only after inline comment position errors are resolved, making the post-loop non-COMMENT fallback reachable?
• Additional unresolved notes suppressed: 8

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 10 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

2 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 63/70 hunks.
Excluded by configuration/planning: 7 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Does the CLI review command define --skip-github-inline-comments, which entrypoint.ts now emits when review-mode approve runs with post-inline-comments=false?; Should an invalid review-mode input (e.g. a typo) fail before the trigger gate in parseGitHubActionArgs, unlike model/models which are deliberately validated after the gate so unrelated comment events do not go red?; Does a --review-mode approve run without --post-github-comments produce the documented 'still require posting' error, or is the CLI override silently ignored?; Should a repo config github.reviewMode = "approve" be silently ignored when a run does not pass --post-github-comments, or should that surface a warning?; Does --review-mode approve without --post-github-comments surface an error, or is it silently inert for one run (PR summary says overrides 'still require posting')?; When github-action runs with post-inline-comments=false and review-mode=comment, the --review-mode flag is silently dropped (line 227) rather than erroring; is silently ignoring an explicit comment mode the intended Action behavior?; When the action input review-mode="comment" is combined with post-inline-comments=false, --review-mode is intentionally dropped; confirm no trusted local config path can then still resolve reviewMode="approve" and post a verdict (posting is skipped entirely, so this looks moot).; Does --review-mode comment with post-inline-comments=false but an explicit comment mode remain the intended no-verdict path in executeGitHubActionCommand (line 227 skips passing --review-mode)?; Should the Action's parseReviewMode reuse the shared GitHubReviewMode union type (src/types.ts) instead of its string-returning copy, so a future extension of the accepted set cannot diverge?; Does the config loader/default config populate github.reviewMode with 'comment' so the now-required field is always set for all CodegenieConfig construction sites?; Is GitHubReviewMode declared/exported in src/types.ts before use and limited to 'comment' | 'approve' as the PR body states?; Is GitHubReviewEvent ("COMMENT"|"APPROVE"|"REQUEST_CHANGES") mapped exhaustively from GitHubReviewMode ("comment"|"approve") at the publisher, so that an incomplete run can never map to APPROVE?; Is GitHubReviewEvent declared/exported in src/types.ts before RunPostingRecord uses it, and do all producers of RunPostingRecord set reviewEvent when a non-COMMENT event is submitted?; Should the new action test assert that "approve" directly follows "--review-mode" in the argv (currently only membership is checked, so a reordering/pairing regression in buildReviewArgv would still pass)?
• Does any existing test cover an approve-mode run where the newest own review is COMMENTED but an older CHANGES_REQUESTED review still holds open issues?; Can the verdict marker (open-issue list from issuesFromFindings + carried issues) grow large enough that REVIEW_BODY_CAP - footer.length - markerBlock.length - 2 goes below ~17, making capBody return only the truncation suffix and the final body exceed REVIEW_BODY_CAP?; Does FinalFinding.fingerprint always render as exactly 64 lowercase hex characters, so parseOpenList's /^[0-9a-f]{64}$/ test accepts every entry formatVerdictMarker writes?; Is carriedOpenIssues correct when latest.commitId === headSha (files stays undefined), given anchorSettled then treats every prior issue as unsettled and carries it forward forever on the same head?; Does any existing test cover the publisher path where the previous own review is COMMENTED but an older CHANGES_REQUESTED review still holds open issues?; In tests/github-publisher.test.ts:879 the settled case overrides headSha with "h".repeat(40), which equals the value resolved()/pr() already return; is the override intended to make the commit-difference explicit, or was a distinct head SHA intended so the test would also prove compareFiles is called with a changed head?; Should carryForwardIssues' file-status settle branch (status "added"/"removed"/"renamed" ⇒ anchor settled) and the non-empty currentFingerprints dedupe path get direct assertions, given no inspected test supplies a ComparedFileLines with status or a non-empty fingerprint set?
• When opts.skipInlineComments is true (src/github/publisher.ts:138), duplicate detection still runs and skippedDuplicates is computed from duplicateDecisions; does the resulting telemetry/body disclosure misreport skipped duplicates for runs that intentionally post no inline comments?; In approve mode with zero publishable findings and summaryWhenNoFindings=false, does buildPostingBody produce a non-empty body, or does the run rely on forcedFallbackBody("Approved.") via decision.forcePost?; Does the new staleApproval/createGitHubClient usage in runReview dismiss stale approvals only when posting is enabled and reviewMode is 'approve', and are its network/auth failures handled so a review run is not aborted?; Where in src/pipeline/review-runner.ts are createGitHubClient and staleApproval invoked, and is the GitHub token/permission precondition checked before invoking them?; When runReview throws after dismissStaleApproval (line 187) and before maybePublishToGitHub (line 359), the prior approval is already dismissed and no replacement review is posted; is leaving the PR unapproved on a failed/aborted run the intended contract?; Does staleApproval need coverage for PENDING/DISMISSED reviews (filtered in latestSubmittedReview) so an already-dismissed approval is not re-dismissed against the GitHub API?
• Does REPO_SAFE_REVIEW_KEYS in src/config/config-loader.ts list "github.reviewMode", and does src/config/schema.ts validate the comment|approve enum?; Should repo-config downgrade of github.reviewMode to "comment" also apply when applyRepoConfigLayer runs after a CLI override (currently only used by eval-runner)?; Should repo codegenie.toml be able to downgrade an explicit CLI/Action --review-mode approve to comment via applyRepoConfigLayer (eval path), where the repo layer is applied after CLI overrides?
• Does the new partial: totalHunks > 0 in the zero-work path change default comment-mode output, where excluded-only pushes now render the "Review incomplete" health banner and a "## Coverage" disclosure instead of plain "Nothing to review."?; Are binary / mode-only / rename-only files filtered out of diff.files before maybeZeroWork, or do they reach it as DiffFile entries with empty hunks (making totalHunks 0 while kept is empty)?; When a PR changes only files whose diffs produce no hunks (binary patches, mode/rename-only changes), maybeZeroWork sets partial: totalHunks > 0 = false, so health can be "completed" and selectPostedEvent returns APPROVE with zero reviewed content. Is that intended, and is it covered by any test?
• Additional unresolved notes suppressed: 5

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 16 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

4 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 59/70 hunks.
Excluded by configuration/planning: 11 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Does the compare range used by carriedOpenIssues (latest.commitId...headSha) ever exceed the GitHub compare 300-file response cap in practice for this project?; When the own-PR fallback demotes the event to COMMENT, does the posted body still carry the verdict marker produced for approve mode?; Can formatVerdictMarker produce a marker long enough that REVIEW_BODY_CAP - footer.length - markerBlock.length - 2 goes negative or very small (many carried/published issues, each encoded as fingerprint:path:line:side), and does capBody behave sanely for a non-positive limit (e.g. still emitting a truncation notice that pushes the final body over REVIEW_BODY_CAP)?; Can an unresolved summary-only (anchorless) finding be lost from the verdict marker so a later approve-mode run posts APPROVE while its own REQUEST_CHANGES still stands?; Does capBody tolerate a non-positive limit when a large verdict marker (many open issues, ~100 chars each) consumes most of REVIEW_BODY_CAP in finalizeReviewBody?; Does maybePublishToGitHub derive the posted event for the zero-work result via healthForResult/selectPostedEvent, so a totalHunks===0 zero-work result in approve mode submits APPROVE?; Are prior-review carried open issues counted for the zero-work posting plan (which would force REQUEST_CHANGES instead of APPROVE in the hunkless case when a prior change request stands)?; Does thread.lineIsCurrent stay unset (so lineBasis falls back to "previous") for every GitHub comment where only original_line is present, including outdated multi-line threads where line is null but start_line is set?; Is the carriedLookup.unknown -> COMMENT downgrade covered by any non-inspected suite (e.g. tests/pipeline-phase7.test.ts) that injects a rejecting listOwnReviews?; Should the second leg of the new test also assert the posted review event (expected REQUEST_CHANGES for a run with a confirmed finding in approve mode), not just that review 7 was dismissed?
• Does loadConfig resolve a repo-config github.reviewMode into the effective config correctly when --post-github-comments is absent (i.e. what value the resolved config carries and how it is used)?; Does runReview/ReviewOptions treat an absent skipGithubInlineComments key the same as false, and is parsed.options.skipGithubInlineComments ever a meaningful false that must override a config-level true?; Does src/github-action/entrypoint.ts allow a user-supplied review-passthrough containing --review-mode or --skip-github-inline-comments to reach the CLI without --post-github-comments being added, turning a previously working action config into an invalid_args failure?; Does the Action entrypoint ever emit --review-mode without --post-github-comments?; Should a whitespace-only --review-mode value (e.g. " ") be rejected as invalid instead of silently treated as unset, given parseBoolean/other flags are strict?; Are the newly imported createGitHubClient and staleApproval actually used in review-runner.ts, and does the stale-approval dismissal path await its async calls and handle rejections (e.g. missing token / API errors) so a failed dismissal does not reject unhandled or silently skip re-review?; Does review-runner.ts construct a GitHub client unconditionally (even when reviewMode is the default comment mode or no token is configured), which would change behavior for existing runs?
• Does applyRawConfig (src/config/config-loader.ts ~293-295) assign raw.github.reviewMode only after schema validation of the raw TOML value, so an arbitrary string cannot reach config.github.reviewMode?; Did repo-config github.summaryWhenNoFindings reach applyRawConfig before this change, i.e. is the newly warned-and-ignored key an unintended capability regression?; In the eval path, applyRepoConfigLayer applies repo config on top of an already CLI-resolved config, so a repo github.reviewMode = "comment" would override an explicit CLI/action approve there (a downgrade, consistent with the inline 'lower the ceiling' comment but not with the PR statement that CLI overrides repo). Is that ordering intended?; Is dropping repo-config github.reviewMode = "approve" with an ignored-key warning the intended UX, given the PR summary says repo codegenie.toml may set this GitHub key?
• Does GitHub's createReview report the "Can not approve your own pull request" 422 before or after inline-comment position 422s, i.e. can the own-PR rejection first surface on the post-loop summary-only call?; Does the GitHub client surface the own-PR validation message in one of message/stderr/stdout/responseBody for both 'approve your own pull request' and 'request changes on your own pull request' responses, so the /own pull request/iu match actually fires in production?
• When skipGithubInlineComments is set together with reviewMode=approve, does the publisher still settle/carry forward prior inline issues correctly (publisher.ts line 138 short-circuits inlineCandidates to [])?; On the zero-work early-return path (maybeZeroWork -> maybePublishToGitHub at review-runner.ts:~1021), can an approve-mode run post a new APPROVED review while a prior change request/approval for an older head SHA is intentionally left undismissed?
• Additional unresolved notes suppressed: 11

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

Re the summary-only finding "New gh transport methods (listOwnReviews, dismissReview, compareFiles) have no client-level tests": fixed in 9264f02. tests/github-client.test.ts now drives the real client through the fake runGh harness:

• listOwnReviews: a full page 1 mixing the viewer (different casing) with another author, then a short page 2. It asserts only viewer reviews are returned with id/state/commitId/body/submittedAt mapped, and that exactly two page requests are made.
• dismissReview: asserts the pulls/<n>/reviews/<id>/dismissals endpoint, --method PUT, the {message, event: "DISMISS"} body, and that a failing gh call rejects with CodegenieError.
• compareFiles: a patched file maps added and deleted lines, a patch-less file with changes > 0 sets patchMissing, and a renamed file maps previous_filename to previousPath.

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 15 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

4 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 62/70 hunks.
Excluded by configuration/planning: 8 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Is --review-mode approve without --post-github-comments rejected or ignored somewhere downstream, as the PR summary states overrides "still require posting"? parseReviewCommand sets cli.reviewMode independently of options.postGithubComments.; Is there a CLI flag that can explicitly disable inline comments (e.g. --no-skip-github-inline-comments) whose false value would be dropped by the conditional spread?; Is a repo-config github.reviewMode=approve without --post-github-comments intentionally silently ignored (no error), given the CLI flag is rejected in that case?; Does the CLI --review-mode override still require --post-github-comments (per PR summary "still require posting"), or can approve mode be requested without posting enabled?; Does applyEnvironment/applyCliOverrides handle an invalid --review-mode or env value before codegenieConfigSchema.safeParse, so users get a clear error rather than a generic 'resolved configuration is invalid'?; Should the Action-side parseReviewMode return GitHubReviewMode (as src/cli/review-command.ts does) instead of string, and should GitHubActionInputs.reviewMode be typed as the union rather than string?; Does the config loader/default-config construction always populate github.reviewMode (defaulting to 'comment') so the new required field cannot be undefined at runtime for existing codegenie.toml files?; Is GitHubReviewMode parsed/validated from codegenie.toml, CLI --review-mode, and the Action review-mode input, so an unknown string falls back to "comment" rather than being cast into the union?; Is the CLI --review-mode string validated against the comment|approve union before it is assigned to CliConfigOverrides.reviewMode (typed as GitHubReviewMode)?
• Does runReview's options type accept skipGithubInlineComments, and is omitting the key (rather than passing false) equivalent to disabled for config-provided values?; Is the new RunReviewOverrides.skipGithubInlineComments actually threaded into the GitHub publish path (publishReview/publisher options) and plumbed from the CLI/Action entry points, or is it declared but unread?; Does any caller set skipGithubInlineComments, and does the consuming code default it to false when undefined so existing runs keep posting inline comments?; If a run fails after dismissStaleApproval (line 187) but before publishing, the PR is left with its prior approval dismissed and no replacement review; is that the intended behavior for approve mode?; Is an empty diff (allFiles.length === 0, e.g. base already contains head) intended to produce an APPROVE review in reviewMode=approve?; Does dismissStaleApproval being skipped on the zero-work path (and on listOwnReviews/dismissReview failure) ever leave a stale APPROVE standing while a new APPROVE/COMMENT review is also posted for the new head?; Can a zero-work run have allFiles.length > 0 with totalHunks === 0 (e.g. binary, mode-only or rename-only diff entries that parse to zero hunks), so that partial: totalHunks > 0 stays false, healthForResult returns "completed", and selectPostedEvent emits APPROVE for a PR whose files were never reviewed?
• Does repo-level codegenie.toml actually accept [github] reviewMode = "comment" (lowering) while rejecting "approve", as README line 79 now states?; Does config loading actually reject (or silently ignore) github.reviewMode = "approve" from repo codegenie.toml, matching the README claim that repo config cannot set approve?; Does the config schema (src/config/schema.ts) declare github.reviewMode so the tracked source path corresponds to a real resolved value?; Is config.github.reviewMode guaranteed non-undefined after config loading (defaulted to "comment")? If it can be undefined at this point, !== "comment" is true and would force posting plans for every run with postGithubComments enabled.
• In compareFiles, patchMissing is false when GitHub omits patch and changes is absent/0 (e.g. binary or pure-rename entries); does anchorSettled then treat such a file as having no changed lines and settle carried issues on it?; Does compareFiles need a test for a renamed file (previous_filename -> previousPath) given anchorSettled matches carried issues on previousPath?; For a RIGHT issue with lineBasis "current" taken from an existing review comment (lineIsCurrent true), is comment.line numbered against the current PR head or against the head at the time the carried compare range starts, so that comparing against file.addedLines of the previous->head compare is correct?; When GitHub omits patch but reports changes: 0 (e.g. pure rename/mode change), compareFiles sets patchMissing=false with empty addedLines/deletedLines; anchorSettled then returns false for RIGHT/LEFT issues, so the issue stays open. Is that conservative outcome intended for all such status values not already short-circuited (added/removed/renamed)?
• Can a review body posted by approve mode be re-sanitized (sanitizeGitHubCommentBody strips //g) or re-truncated on a retry/fallback path in postWithRecovery/forcedFallbackBody before it reaches GitHub, which would delete the appended verdict marker and make a later carriedOpenIssues run see no prior open issues?; When normalizeCreateReviewError rewrites the message to GitHub review creation failed with HTTP 422, does error.context still retain stderr/stdout/responseBody containing GitHub's "Can not approve your own pull request" text so isOwnPullRequestReview can match?; Is there a test exercising an own-PR 422 whose phrase only appears in context.responseBody (not message), and one asserting an unrelated 422 is not misclassified as own_pr?
• Additional unresolved notes suppressed: 10

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 13 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

3 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 59/70 hunks.
Excluded by configuration/planning: 11 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Does the CLI --review-mode approve path combined with --skip-github-inline-comments (emitted by entrypoint when post-inline-comments is false) still post the APPROVE/REQUEST_CHANGES verdict review, as action.yml's post-inline-comments description promises?; Does the action.yml review-mode input (declared at line 31) default to an empty string, so an unset input yields INPUT_REVIEW_MODE="" and parseGitHubActionArgs leaves reviewMode undefined (comment mode preserved)?; When github.reviewMode="approve" comes only from repo codegenie.toml (no action review-mode input) and the action input post-inline-comments=false, the action emits neither --post-github-comments nor --review-mode, so no verdict is posted. Does that match the action.yml post-inline-comments description ("review-mode approve still posts a verdict when this is false")?; Does any test cover the action passing an empty INPUT_REVIEW_MODE (the YAML default) end-to-end so reviewMode stays undefined and no --review-mode is forwarded to the review command?; Is GitHubActionInputs.reviewMode in src/types.ts declared as plain string (rather than the GitHubReviewMode union), such that changing the Action-side parseReviewMode return type would tighten it?; Can config.github.reviewMode ever be undefined at composer time (e.g. a config object built in tests or by a non-default path), which would make reviewMode !== "comment" true and create a posting plan for no-findings runs that previously posted nothing?; Does the CLI --review-mode override reach config.github.reviewMode before runReview, so dismissStaleApproval (which reads only config.github.reviewMode, not an override field) fires for a one-run approve override on a repo configured as comment?; Is the dismissStaleApproval ordering (after maybeZeroWork) plus the new zero-work postingPlan consistent: an exclusion-only push keeps its old approval but still posts a COMMENT "Nothing to review." review with partial coverage disclosure — is that posting intended?; Is the comment-mode + inline-comments-enabled combination (reviewMode defined, postInlineComments true) covered anywhere, i.e. that --review-mode comment is forwarded with --post-github-comments and without --skip-github-inline-comments?; In the zero-work approve-mode path, coverage.partial is now totalHunks > 0 while runReview still finalizes the run as status "completed_full" (src/pipeline/review-runner.ts around lines 183-187 and 995). Does any consumer of the finalize status (exit code, run-stats, Action output) treat an exclusion-only run as fully covered even though the published verdict is COMMENT?
• Can gh return a review object with id present but user.login absent/differently cased such that listOwnReviews drops an own standing CHANGES_REQUESTED review (which would let approve mode approve over it)?; Does the src/pipeline/review-runner.ts consumer of OwnPullRequestReview.state also compare exactly against GitHub's uppercase states, and what happens there when listOwnReviews passes through its "" fallback?; Does github-client cache listOwnComments results so the second call cannot fail independently, and if it can fail, does the resulting rejection abort posting entirely in approve mode?; Is a carried issue's line ever rebased between review runs (e.g., in carriedOpenIssues or compareFiles consumers) before being re-serialized into the verdict marker?
• Are there CLI tests covering --review-mode bogus (invalid value rejection) and --review-mode approve without --post-github-comments, matching the documented override contract?; Is there a CLI parse test asserting that --review-mode / --skip-github-inline-comments without --post-github-comments throws invalid_args? A scoped search of tests/**/*.test.ts for those messages returned no hits (bounded, not exhaustive).; Does review-command.ts:229 (options.reviewMode !== undefined && !options.postGithubComments) reject --review-mode without posting, or does it silently drop it, and is parseReviewMode's rejection of invalid values covered by tests?
• Does GitHub always surface the own-PR 422 before comment-position 422s, which would make the post-loop verdict path unreachable with a non-COMMENT event?; Does the runGh error for a failed gh api ... /reviews POST carry the GitHub response text in context.stderr/context.stdout (or context.responseBody), given normalizeCreateReviewError overwrites error.message with 'GitHub review creation failed with HTTP 422'?; Does any test elsewhere in the suite (outside the inspected excerpt of tests/github-publisher.test.ts) cover the own-PR 422 fallback using an error produced by the real client path via normalizeCreateReviewError?
• In approve mode, can a zero-work run with totalHunks === 0 (empty diff, or a diff whose files parse to zero hunks such as binary/rename-only changes) publish an APPROVE review without reviewing anything, since partial stays false and healthForResult then reports "completed"?; Line 21 and line 23 of tests/review-verdict.test.ts assert the identical selectPostedEvent({mode:"approve",health:"completed",openIssueCount:0}) === APPROVE case twice; was one of them meant to pin a different state (e.g. unknown carried issues forcing COMMENT)?
• Additional unresolved notes suppressed: 8

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 13 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

2 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 61/70 hunks.
Excluded by configuration/planning: 9 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Should --skip-github-inline-comments also require --post-github-comments (like --review-mode does at review-command.ts:229), or is it silently ignored for non-posting runs?; Does the CLI --review-mode override still require posting (i.e., is it ignored or rejected when --post-github-comments is absent), as the PR summary states?; Does parsed.options.skipGithubInlineComments distinguish an explicit CLI false from an unset value, and does runReview/config treat a missing skipGithubInlineComments override as 'inherit config' rather than 'false'?; Does an empty --review-mode "" emitted by action.yml actually reach parseReviewMode in the Action entry command / src/cli/review-command.ts and fail validation, or is the empty value dropped or ignored before parsing?; When reviewMode is "approve" but postInlineComments is false, entrypoint.ts:227 passes --review-mode together with --skip-github-inline-comments; does review-command still post the APPROVE/REQUEST_CHANGES verdict review in that combination?; Is the Action entrypoint's wider string return type for parseReviewMode intentional?; Is the new RunReviewOverrides.skipGithubInlineComments actually threaded into the publisher call (and defaulted) inside runReview, or does it silently do nothing when callers set it?; If the run fails or aborts after dismissStaleApproval (review-runner.ts:187), the prior approval is already dismissed with no replacement review posted. Is that accepted behavior for approve mode?; Is there coverage asserting that approve mode with --post-inline-comments true omits --skip-github-inline-comments (so inline comments are not silently suppressed when both are requested)?
• When the prior review commit is not an ancestor of head (force-push), compare(base...head) resolves against the merge base, so addedLines/deletedLines can mark a still-present issue line as changed and anchorSettled drops the carried issue. Is relying on fingerprint re-detection in the current run sufficient to prevent approving over a still-open issue?; Does any test cover an approve-mode run whose listOwnReviews lookup fails and then a subsequent run, to prove the prior REQUEST_CHANGES issues are not lost via the emitted verdict marker?; Is the duplicate github.listOwnComments call (once in maybePublishToGitHub for duplicate detection, again inside carriedOpenIssues) intentional, and is an unguarded throw there acceptable in approve mode?; When the final summary-only post is demoted from APPROVE/REQUEST_CHANGES to COMMENT (own-PR rejection), the body still carries the verdict marker built from mode in maybePublishToGitHub. Does carriedOpenIssues (which skips COMMENTED reviews) or any downstream consumer misread that marker-bearing COMMENTED review?; In zero-work runs with totalHunks === 0 (empty diff) and reviewMode=approve, coverage.partial stays false so healthForResult yields "completed" and selectPostedEvent returns APPROVE with forcePost, posting an approval for a run that reviewed nothing. Is approving an empty-diff PR intended, given stale-approval dismissal is deliberately skipped on the zero-work path?; Does compareFiles' 300-file cap (no file pagination) ever produce a path whose absence is interpreted as settled rather than carried, e.g. via the status==="removed"/"renamed" shortcut interacting with a truncated file list?; Should fakeGithub's compareFiles stub ever return renamed files so the carried-issue rename-settling path (commit 9264f02) is exercised at the pipeline level, or is that covered only in tests/github-publisher.test.ts?; Lines 21 and 23 of tests/review-verdict.test.ts assert the identical approve-mode case twice; was one intended to cover a different input (e.g. mode "approve" with health "completed" and a non-zero openIssueCount plus forcePost assertion)?
• Does the config precedence implementation actually let repo codegenie.toml set [github] reviewMode only to 'comment' (lower), while user config/--review-mode/Action input may set 'approve', as README line 79 now claims?; Does the config loader/default-config construction set github.reviewMode (defaulting to "comment") for every CodegenieConfig literal, so the new required field cannot be undefined at runtime for configs parsed from repo codegenie.toml?; Does every consumer of ReviewCommandOptions.skipGithubInlineComments handle it as optional (undefined) rather than relying on an explicit false, and is GitHubReviewEvent's "REQUEST_CHANGES"/"APPROVE" variant handled exhaustively at the publisher switch?
• Can gh's 422 stdout/stderr for createReview ever echo the submitted review body (which could contain the phrase "own pull request" from a finding), causing a false own-PR classification and an unintended APPROVE->COMMENT demotion?; dismissReview's failure test only asserts rejects.toBeInstanceOf(CodegenieError); it does not pin the normalized error code/httpStatus produced by normalizeCreateReviewError, so a regression that reclassifies a 403 dismissal failure (e.g. into a different code consumed by the publisher fallback) would still pass.
• Is "github.reviewMode" also added to REPO_SAFE_REVIEW_KEYS and the config schema so a repo codegenie.toml setting is accepted rather than warned/ignored?; In the eval path, applyRepoConfigLayer re-applies repo codegenie.toml onto an already-CLI-resolved config; could a repo-set github.reviewMode="comment" override a CLI --review-mode there? (Evals do not post GitHub reviews, so impact looks nil.)
• Additional unresolved notes suppressed: 8

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

Re the two confirmed findings in the latest Codegenie status report (posted there but not as inline threads):

🔵 Medium: Failed prior-review lookup still writes a verdict marker that erases standing open issues. Fixed in 9c5405c.

• A run whose listOwnReviews lookup fails no longer writes a verdict marker.
• latestVerdictReview now only accepts reviews that carry a marker.
• Newer markerless CHANGES_REQUESTED reviews (from failed lookups) add their own inline issues through unrecordedChangeRequests. Each source review is settled against a compare from its own commit.
• Tests: no marker is written when the lookup fails. With an older marker change request, then a failed-lookup COMMENT, then a markerless change request with an inline issue, a clean approve run still requests changes and carries both issues into its marker.

⚪ Low: Carried issue line numbers are re-published against the new head commit without rebasing. Fixed in 9c5405c. carryForwardIssues now shifts each carried previous-basis RIGHT line through the compare's added and deleted lines, so the marker line matches its commit=. LEFT lines (numbered on the PR base), current-basis lines, files with no patch, and files absent from the compare keep their line. Tests cover: lines added and deleted above the anchor shift it, a change below it doesn't, and LEFT and patch-less issues stay put. One existing expectation changed from line 4 to 5 because the test inserts a line above the anchor.

ff562e1 gives the two-pipeline phase-7 test a 20s timeout. It was over the 5s default under local load.

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 19 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

2 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 57/70 hunks.
Excluded by configuration/planning: 13 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Does applyRawConfig (src/config/config-loader.ts:293) validate that raw.github.reviewMode is one of "comment"|"approve" before assigning it to config.github.reviewMode, or can an arbitrary TOML string reach GitHub publishing logic from a user/global config file?; Does REPO_SAFE_REVIEW_KEYS in src/config/config-loader.ts also include "github.reviewMode" so a repo codegenie.toml can set it, and does defaultSources/schema handle the new key consistently?; Does filterRepoConfig actually strip or downgrade an "approve" value for github.reviewMode (and does every repo-config path run through applyRepoConfigLayer), so a PR-author-controlled codegenie.toml can never set config.github.reviewMode to approve?; Should repo config be able to downgrade a CLI/Action --review-mode approve in the applyRepoConfigLayer path (eval-runner only today), given the PR states CLI/Action input overrides repo config?; Can a repo codegenie.toml github.reviewMode = "comment" override a CLI/Action --review-mode approve, since applyRepoConfigLayer runs after loadConfig's applyCliOverrides and filterRepoConfig admits the "comment" value?; Does the resolved CodegenieConfig always set github.reviewMode to "comment" when the key is absent, or can it remain undefined at src/pipeline/composer.ts:322?; Is creating a posting plan intended when health.status !== "completed" and there are no findings in approve mode (to dismiss a stale approval / keep a standing change request)?
• Is parseReviewMode's invalid_args rejection of unknown --review-mode values covered by tests (tests/review-command.test.ts)?; Does --review-mode approve without --post-github-comments (or on a non-PR target) silently set github.reviewMode with no posting, and is that the intended 'still require posting' behavior rather than an error?; Should the Action-side parseReviewMode return type and GitHubActionInputs.reviewMode in src/types.ts be narrowed to GitHubReviewMode for parity with the CLI?; Does the config loader/defaults object populate github.reviewMode (default 'comment') for every CodegenieConfig construction site, so the new required field cannot be undefined at runtime for configs built outside the loader (e.g. test fixtures or partial merges cast to CodegenieConfig)?; Does every construction site of ReviewCommandOptions and every consumer of skipGithubInlineComments treat undefined as 'do not skip inline comments', and is GitHubReviewEvent's REQUEST_CHANGES value mapped from reviewMode 'approve' correctly?
• When opts.skipInlineComments is true (CLI --skip-github-inline-comments), are findings with publication "inline" still represented in the posted review body, or are they dropped from all output?; Can any composer or post-composition path place a finding with publication === "suppressed" into finalReview.findings, making published.length nonzero and preventing approve mode from ever approving?; In the zero-work path with an empty diff (allFiles.length === 0 → totalHunks 0 → partial false), does approve mode post an APPROVE review that clears a standing change request without reviewing anything?; Is dismissStaleApproval deliberately skipped on the zero-work path (it is called only after maybeZeroWork returns undefined), so a stale approval on an exclusion-only push is intentionally retained?; Does any test exercise carryForwardIssues with a non-empty currentFingerprints set (re-found issue dropped), e.g. in tests/github-publisher.test.ts around publisher.ts:278?
• When the own-PR fallback demotes a verdict review to COMMENT, the body still carries the verdict marker and verdict wording (e.g. forcedFallbackBody "Changes requested."); is a COMMENTED review bearing an approve-mode marker intended?; Does the demotion/recovery path in postWithRecovery actually re-finalize an already-finalized body (containing the marker), producing two marker occurrences in the posted review body?; Can a non-own-PR 422 (e.g. inline-comment line rejection) carry the literal text "own pull request" in gh stderr/stdout or the echoed responseBody — for example via finding text copied into the error payload — causing isOwnPullRequestReview to silently demote an APPROVE/REQUEST_CHANGES verdict to COMMENT?
• Do the two earlier added approve-mode tests at tests/github-publisher.test.ts:789-826 ("approves a clean completed review only in approve mode", "requests changes for findings and approves a clean review in approve mode") pin the exact review event and marker, or do they only assert absence of throws?; Does any inspected test assert the posted review event for approve mode on a PR with remaining findings (expect REQUEST_CHANGES, not APPROVE) after a stale approval is dismissed?
• Additional unresolved notes suppressed: 14

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

Re the two confirmed findings in the latest Codegenie status report (no inline threads were posted):

🔵 Medium: Verdict marker drops anchorless open issues, so a later run can approve over them. Fixed in f3ecb51. issuesFromFindings now records an anchorless finding as a file-level marker entry (<fp>:<path>:0:RIGHT, line 0 means no line). The existing grammar parses it. The entry settles like a LEFT issue: once its file changes and the fingerprint isn't raised again. It is never line-shifted, and it renders as just the path. Test: approve run with an anchorless summary-only finding requests changes and writes the entry. Feeding that body back with the file untouched still requests changes. Control: with the file changed and the finding gone, the clean run approves.

🔵 Medium: Exclusion-only runs now report "Review incomplete" to every user, not just approve mode. Fixed in f3ecb51. maybeZeroWork is back to partial: false. The approve-mode guard moved to the publisher: it never approves when totalHunks > 0 && reviewedHunks === 0, posting COMMENT instead, alongside the existing unknown-lookup downgrade. Tests: the exclusion-only pipeline run has coverage.partial === false and still posts COMMENT in approve mode, and a publisher-level test covers the reviewed-nothing guard.

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 9 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

2 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 60/70 hunks.
Excluded by configuration/planning: 10 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• Is a PR-author-controlled repo codegenie.toml lowering github.reviewMode to "comment" acceptable as a way to suppress the verdict/REQUEST_CHANGES post (composer skips the posting plan when reviewMode === "comment" and there are no publishable findings)?; Is there an existing test (tests/github-publisher.test.ts or elsewhere) covering carriedOpenIssues when the prior review commit is not an ancestor of head (force-push/rebase)?; When the final summary-only post is demoted from APPROVE/REQUEST_CHANGES to COMMENT (publisher.ts:374-378), the body still carries the verdict marker built in maybePublishToGitHub; does latestVerdictReview treating that COMMENTED review as the recorded verdict state ever mis-seed carried issues on a later run?; Could a non-own-PR 422 (e.g. invalid inline comment position) echo the phrase "own pull request" back in responseBody/stderr and falsely trigger the COMMENT demotion, silently dropping an APPROVE/REQUEST_CHANGES verdict?; Is patchMissing: patch === undefined && (file.changes ?? 0) > 0 in github-client.compareFiles correct for a file GitHub reports with patch omitted and changes undefined/0 (e.g. binary or very large file)? In that case patchMissing stays false with empty addedLines/deletedLines, so anchorSettled/rebaseOntoHead treat it as a parsed-but-unchanged file rather than an unknown patch.; Should compareFiles surface an explicit truncation signal when GitHub returns the 300-file cap, rather than relying on absent files keeping carried issues open?; fakeGithub.compareFiles ignores its (baseCommit, headSha) arguments and returns the same ComparedFileLines[] for every source review, so no inspected test proves carriedOpenIssues settles each source review against its own commit's compare. Is there another test (e.g. in tests/review-verdict.test.ts) that pins the per-commit compare keying?; Does tests/github-publisher.test.ts (or another inspected test) exercise carriedOpenIssues' use of latestVerdictReview/unrecordedChangeRequests, i.e. that a markerless CHANGES_REQUESTED review newer than the last marker-bearing review still blocks approval?
• When opts.skipInlineComments is true, inline-publication findings are neither posted inline nor added to demoted, so buildPostingBody only emits the summary (and includeInlineSummary is false). Is dropping per-finding detail for those findings the intended contract of CLI --skip-github-inline-comments?; With opts.skipInlineComments in comment mode, does the publisher then post nothing at all (decision.forcePost false and no inline comments), silently dropping inline-publication findings?; Is the extra COMMENT review body intended for approve-mode runs with summaryWhenNoFindings=false and zero publishable findings on an incomplete run?; Is the new RunReviewOverrides.skipGithubInlineComments flag actually threaded into the GitHub publisher call path in runReview, or is it accepted and ignored?; If a run aborts or throws after dismissStaleApproval (line 187) but before maybePublishToGitHub, the prior approval is already dismissed and no replacement review is posted; is leaving the PR unapproved on a crashed run the intended behavior?; Does a PR whose diff contains only binary or mode-only files really reach maybeZeroWork with coverage.totalHunks === 0 (kept empty), and does publisher then emit APPROVE?; Should the stale-approval-kept scenario also assert the posted review body/event of the second (reviewable-work) run is REQUEST_CHANGES, and that dismissal happens before the new review is created?
• Does parseReviewMode reject unknown --review-mode values with an invalid_args CodegenieError (rather than passing an arbitrary string into CliConfigOverrides.reviewMode), and is that rejection covered by a test? tests/review-command.test.ts contains no match for "review-mode"/"reviewMode".; Does parseReviewCommand surface an invalid --review-mode value (parseReviewMode) when the flag is used with --post-github-comments, i.e. is buildCliOverrides reached after resolveTarget for all target modes?; Does a CLI --review-mode approve override without --post-github-comments behave as the PR states (override still requires posting), or is the override silently ignored?; Is the duplicated parseReviewMode in entrypoint.ts intentionally typed as string rather than GitHubReviewMode (CLI version), given inputs.reviewMode is declared string?
• Does the resolved config layering actually apply the repo-level [github] reviewMode = "comment" value (rather than it being overwritten or dropped at merge time), as README line 79 states, and is that covered by tests/config-loader.test.ts?; Does the repo-layer ordering guarantee that a repo-set github.reviewMode="comment" cannot be re-raised to "approve" by a later layer other than CLI/Action (which is the documented override)?; Can applyRepoConfigLayer (used only by eval-runner) re-apply a repo-config github.reviewMode="comment" after CLI overrides and downgrade an approve run? Eval paths appear not to publish GitHub reviews, so impact looks nil.; Is there an env-variable layer (e.g. CODEGENIE_* ) that can set github.reviewMode, and if so is it also constrained like repo config? The new test only covers default, repo-config, home-config and CLI layers.
• When github.reviewMode="approve" comes only from repo codegenie.toml (no --review-mode input) and the action runs with post-inline-comments=false, no --post-github-comments is forwarded, so the configured approve verdict is silently not posted. Is that the intended "still require posting" rule?; Should an explicit action input review-mode=comment with post-inline-comments=true be asserted to forward "--review-mode comment" (currently untested branch of the inputs.postInlineComments || postsVerdict condition)?
• Additional unresolved notes suppressed: 4

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

Re the two confirmed findings in the latest Codegenie status report (no inline threads were posted):

🔵 Medium: compareFiles uses three-dot compare, so after a force-push still-open issues look settled. Fixed in d88d32e. compareFiles now reads the compare response's status and throws unless it is ahead or identical. carriedOpenIssues already maps a failed compare to files = undefined, so every carried issue stays open and the run requests changes. Fast-forward compares settle as before (covered by the existing "settles a touched one" test). Tests: a client test where a diverged compare rejects, and a publisher test where an unavailable compare keeps the carried issue open with REQUEST_CHANGES.

🔵 Medium: Approve mode auto-approves hunk-less pushes (binary/mode-only diffs). Fixed in d88d32e, taking the strict reading: the publisher now refuses APPROVE whenever reviewedHunks === 0, dropping the totalHunks > 0 condition. Exclusion-only, binary or mode-only, and empty pushes all post COMMENT in approve mode. Test: a zero-hunk completed run in approve mode posts COMMENT, and it fails on the old guard.

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 18 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

1 confirmed finding retained from completed work. Unresolved questions require attention.

Reviewed 56/70 hunks.
Excluded by configuration/planning: 14 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• After the own-PR 422 fallback re-finalizes the body via demoteCommentsIntoBody, the already-embedded verdict marker is not stripped by stripReviewBodyFooter, so the posted body can contain the same marker twice; is duplicate-marker output acceptable to parseVerdictMarker consumers and body-cap accounting?; Can formatVerdictMarker produce a marker long enough (roughly >60,000 chars, i.e. ~1000 open issues) that the negative budget makes capBody return only the truncation suffix while the full marker plus footer still ship a body over GitHub's size limit?; When reviewMode="approve", publishableCount===0 and summaryWhenNoFindings is false, is the composed reviewBody ever empty so the publisher must rely on forcedFallbackBody/forcePost to post a valid review?
• When skipInlineComments is true, skippedDuplicates still counts duplicate decisions for comments that were never going to be posted — does any telemetry/summary consumer report a misleading skipped-duplicate count?; If the pipeline aborts or fails after line 187 (e.g. budget stop, hard abort, or a posting error in maybePublishToGitHub), the prior approval has already been dismissed but no replacement review is posted. Is leaving the PR in a dismissed-approval, no-new-review state the intended behavior for failed runs in approve mode?; Is the carryForwardIssues dedupe branch (currentFingerprints.has(issue.fingerprint)) exercised anywhere? tests/review-verdict.test.ts always passes new Set(), so a regression that stops dropping re-raised prior issues would duplicate them in publisher.ts line 281's issues list and in openIssueCount.
• Does REPO_SAFE_REVIEW_KEYS in src/config/config-loader.ts include "github.reviewMode" so a repo codegenie.toml can actually set it, and does the schema validate the comment|approve enum before it is recorded as a default source path?; In applyRepoConfigLayer (used only by src/evals/eval-runner.ts), repo config is applied on top of an already CLI-resolved config, so a repo-config reviewMode="comment" would override a CLI --review-mode=approve. Is that inversion of the documented CLI-wins precedence intended for the eval path?
• Does tests/github-publisher.test.ts cover the final summary-only loop demoting a verdict event to COMMENT on an own-PR 422 after three comment-422 attempts (asserting verdictFallback="own_pr", reviewEvent="COMMENT", and a single retry)?; Does a non-own-PR 422 (e.g. an inline-comment position rejection) in practice carry text containing "own pull request" in stdout/stderr/responseBody, and would that demote an intended APPROVE/REQUEST_CHANGES verdict to COMMENT?
• Is omitting --post-github-comments (so no verdict is posted) the intended precedence when post-inline-comments=false and reviewMode comes only from user config, given README states user config may enable approve?; Does any non-tests harness (e.g. action.yml wiring or a CLI-level test) assert that approve mode with inline comments enabled still emits inline comments, which would make the entrypoint-level gap already covered?
• Additional unresolved notes suppressed: 13

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

Re the confirmed finding in the latest Codegenie status report, "No test covers approve mode with inline comments enabled, so an unconditional skip flag would pass": fixed in b5c92c2. New tests/github-action.test.ts case runs --review-mode approve with post-inline-comments at its default and asserts --post-github-comments, --review-mode approve (adjacent), and no --skip-github-inline-comments. Mutation-checked: weakening the gate to postsVerdict ? ["--skip-github-inline-comments"] : [] fails this test, and the existing post-inline-comments false case covers the other side.

[donnfelker]

codegenie review

[github-actions]

🧞 Codegenie Review

Warning

Review completed with unresolved questions. 11 question(s) remain unresolved; absence of a confirmed finding does not establish safety.

2 confirmed findings retained from completed work. Unresolved questions require attention.

Reviewed 58/70 hunks.
Excluded by configuration/planning: 12 hunks.

Coverage disclosure:

• specs/project/architecture.md: configured skip rule
• specs/project/components/repository_and_github.md: configured skip rule
• specs/project/functional_spec.md: configured skip rule

🙋 Needs human attention:

• When opts.skipInlineComments is true in comment mode (CLI --skip-github-inline-comments) and all findings are inline-published with github.summaryWhenNoFindings=false, prepared is [] and buildPostingBody returns "" (inline findings are neither in summaryOnlyFindings nor demoted), so the run takes the skipped_no_findings path and posts nothing. Is silently posting nothing intended for comment mode, or should inline findings be demoted into the body when inline posting is skipped?; carriedOpenIssues calls github.listOwnComments(prNumber) a second time (the publisher already called it at line 134) without a try/catch, so a transient failure there aborts the whole publish with github_post_failed instead of degrading to the documented unknown -> COMMENT path. Is that acceptable, and is the duplicate API call intended?; Should the final summary-only post rewrite the body (verdict marker / "Changes requested" prose) when it demotes the event to COMMENT, or is keeping the REQUEST_CHANGES marker in a COMMENTED review intended so the next run still carries the issues forward?; Does any test cover a carried issue whose compare lookup fails (compareFiles rejects a diverged compare) and the marker written on the next run, asserting the recorded line/commit pair stays consistent?; Is skipGithubInlineComments actually consumed in runReview (and threaded to the GitHub publisher) so that inline comments are suppressed when the override is set?; If a run dismisses a stale approval at line 187 but then fails before publishing (error/incomplete path), the PR is left with no approval and no new review; is that the intended trade-off stated in the PR body ("A stale approval is dismissed before re-review")?; dismissStaleApproval dismisses the prior approval at stage 2, before maybePublishToGitHub re-checks head/base SHAs and posts. If the run then aborts (hard timeout) or the publisher throws 'PR changed while review was running', the PR is left with no approval and no new review. Is that acceptable, or should dismissal be deferred to posting time?; Does any added approve-mode test assert the dismissal of a stale prior APPROVED review (github.dismissReview) before re-review, as claimed in the PR summary? fakeGithub stubs dismissReview as a no-op with no capture, so that path appears unasserted in the inspected range.; Is carryForwardIssues with files === undefined (compare rejected after a force-push) pinned anywhere as a unit case, or only via the publisher-level test at tests/github-publisher.test.ts:994?
• Is parseReviewMode actually invoked on the CLI input before cli.reviewMode reaches config resolution (call-site ordering in src/cli/review-command.ts and src/config/config-loader.ts)?; Should GitHubActionInputs.reviewMode be typed as the GitHubReviewMode union instead of string, so that the forwarded --review-mode value is type-checked at the action boundary rather than only re-validated by the CLI's parseReviewMode?; Is there a shared source of truth elsewhere (e.g. githubReviewModeSchema or an exported mode constant) that these two allow-lists are expected to track, and what prevents drift if a third mode is added?; Does the review CLI also reject an invalid --review-mode value, or does validation only exist in parseGitHubActionArgs (src/github-action/entrypoint.ts parseReviewMode)?
• Does repo-level codegenie.toml actually accept [github] reviewMode = "comment" (lowering) while rejecting "approve", as README line 79 states, and does the CLI flag name --review-mode plus Action input review-mode match the implementation?; Does the README sample block containing the new [github] section document the repo codegenie.toml or the user-level config, and does the config loader reject or ignore reviewMode = "approve" from repo config?; Is "comment" the correct documented default value and are "comment"/"approve" the only accepted enum values in the loader schema?; Does action.yml declare a review-mode input (with a default) so that ${{ inputs.review-mode }} is non-empty, and does the CLI accept an empty --review-mode "" value without erroring?; Do sibling args in the same block (e.g. --post-inline-comments, --depth) rely on non-empty action input defaults, confirming the unconditional append pattern is safe only when a default exists?
• Does defaultConfig in src/config/schema.ts actually set github.reviewMode, so the now-required key is always present?; Is the fatal config_error acceptable for an invalid github.reviewMode value in a PR-author-controlled repo codegenie.toml, or should it warn and ignore?; Can a resolved CodegenieConfig ever reach dedupeRankAndComposeReview with github.reviewMode undefined (e.g. hand-built config objects that bypass the resolved schema), where reviewMode !== "comment" would wrongly force a posting plan?
• Can a viewer-authored PENDING review (no submitted_at) reach the other approve-mode decision paths fed by listOwnReviews — latestSubmittedReview's own filter and the carried-issue source selection in publisher.ts?; Is OwnPullRequestReview.commitId guaranteed to equal the commit recorded in that review's own verdict marker? If GitHub reports a different review commit_id, marker lines are settled against a compare window that does not match their basis.
• Additional unresolved notes suppressed: 6

— codegenie v0.7.0 (8003bb804d) · View Workflow Job

[donnfelker]

Re the two confirmed findings in the latest Codegenie status report (no inline threads were posted):

🔵 Medium: Un-rebased carried line is re-published under the new head commit in the verdict marker. Fixed in 74f557b.

• Marker entries take an optional fifth field, <fp>:<path>:<line>:<side>:<commit>, naming the commit an un-rebased line is numbered on. parseOpenList only reads the first four fields, so older readers ignore it.
• When the compare is unavailable or the patch is missing, carryForwardIssues stamps the line with its origin commit. carriedOpenIssues groups issues by issue.commit ?? review.commitId and settles each group against a compare from that commit.
• A line that does move onto head drops the field.
• Tests: the two-run scenario from the finding (run A on a diverged compare writes :4:RIGHT:<C1>, and run B compares from C1, not run A's head, so a coincidental deletion of line 4 doesn't settle it: REQUEST_CHANGES), plus marker round-trip and old-format parsing.

🔵 Medium: Carried issues shared by two prior reviews are settled against the oldest review's compare window. Fixed in 74f557b. Sources are now processed newest-first, so a fingerprint raised by both the marker review (C1) and a newer markerless change request (C2) is settled against C2..head. Test: line 1 changed in C1..C2 but not since C2, and the run still requests changes with the issue listed.

Both new publisher tests and the verdict test fail on the previous commit.

[donnfelker]

codegenie review

[donnfelker]

@pkieltyka, asking for your input on this decision (written with the help of Claude because of the various layers):

Decision: where approve mode can be turned on

Summary

This PR adds reviewMode, which has two values: "comment" and "approve". In approve mode, the bot can approve a PR or request changes.

Codegenie reads reviewMode from three places. A higher number overrides a lower number.

Order	Place	Who controls it	Values it can set
1	User config file on the machine that runs the review	The person or CI runner	"comment" or "approve"
2	Repo codegenie.toml	Anyone who can change the repo, which includes a PR author	"comment" only
3	--review-mode flag (the Action review-mode input sends this flag)	The person or workflow that runs the command	"comment" or "approve"

Commit aa0c06f makes the repo codegenie.toml ignore reviewMode = "approve". Codegenie writes a warning and keeps the value at "comment".

Why: the risks if the repo file can set "approve"

Codegenie reads codegenie.toml from the code that it reviews. A PR can change that file. Thus, a PR author controls that file.

1. Self-approval. A PR author adds reviewMode = "approve" to codegenie.toml in their own PR. The bot reviews the PR. If the bot finds no issues, it approves the PR. The author caused the approval of their own code.

2. Branch protection bypass. A bot approval can count as a required review. Thus, the author can merge code that no person approved. If the bot misses an issue, no person sees the code before the merge.

3. Operator policy override. An operator sets the Action to comment only. A PR can then turn on approve mode for the bot through the repo file. This includes PRs from forks when the workflow checks out the PR head. The operator did not agree to give the bot approval power, but the PR contents gave it.

Result of the fix

• To turn on approve mode in GitHub Actions, set review-mode: approve in the workflow file.
• On the command line, use --review-mode approve.
• The repo codegenie.toml can only keep the mode at "comment".

Question

Do you agree that the repo codegenie.toml must not turn on approve mode? If you know a use case that needs it, tell us.