Skip to content

feat(code): auto-assign code-agent PRs to a human owner#284

Merged
ifireball merged 6 commits into
fullsend-ai:mainfrom
ifireball:feat/1880-auto-assign-pr-assignee
Jul 20, 2026
Merged

feat(code): auto-assign code-agent PRs to a human owner#284
ifireball merged 6 commits into
fullsend-ai:mainfrom
ifireball:feat/1880-auto-assign-pr-assignee

Conversation

@ifireball

Copy link
Copy Markdown
Member

Summary

  • After post-code creates or reuses a PR, best-effort assign it to a human using: latest human /fs-code commenter on the issue → first human issue assignee → human issue author
  • Detect the /fs-code invoker via the issue comments API (no workflow TRIGGER_SOURCE plumbing)
  • Filter bots/GitHub Apps; skip when the PR already has assignees; log and continue on gh pr edit failure

Implements fullsend-ai/fullsend#1880.

Test plan

  • bash scripts/pr-assignee-test.sh
  • make script-build && make check-bundle
  • make script-test
  • CI green on this PR

Made with Cursor

Resolve assignee from the latest human /fs-code comment on the issue,
then fall back to the first human issue assignee or author, so bot-authored
PRs get clear review ownership without workflow TRIGGER_SOURCE plumbing.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ifireball
ifireball requested a review from a team as a code owner July 19, 2026 07:49
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Review · Started 7:50 AM UTC
Commit: b824b9b · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Auto-assign code-agent PRs to a human assignee

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Resolve a human PR owner after post-code creates or reuses a PR.
• Determine owner via latest human /fs-code commenter, then issue assignee, then author.
• Add a focused shell test suite and wire it into make script-test.
Diagram

graph TD
  A["post-code"] --> B["maybe_assign_pr()"] --> C["pr-assignee.lib.sh"] --> D["gh CLI"] --> E{{"GitHub API"}}
  E --> F[("Issue/PR context")] --> C

  subgraph Legend
    direction LR
    _s["Script"] ~~~ _d[("Data") ] ~~~ _e{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use GraphQL timeline query for `/fs-code` invoker
  • ➕ Single query can fetch latest matching comment + author in one round-trip
  • ➕ Avoids REST pagination across large comment histories
  • ➖ More complex query and auth/shape handling than the current REST+paginate approach
  • ➖ Harder to debug in shell scripts compared to straightforward REST calls
2. Propagate trigger user via workflow input/env ("TRIGGER_SOURCE plumbing")
  • ➕ No extra API calls; deterministic actor identity
  • ➕ Avoids relying on comment parsing and potential ambiguity
  • ➖ Requires workflow/schema changes across repos and callers
  • ➖ Doesn’t help when PRs are reused/updated outside the original trigger context
3. Assign at PR creation time only (`gh pr create --assignee`)
  • ➕ Simpler mental model; assignment happens at the point of creation
  • ➕ Avoids a follow-up edit call in the success path
  • ➖ Doesn’t cover the "reuse existing PR" path without additional handling
  • ➖ Still needs the same candidate-resolution logic

Recommendation: The PR’s approach is a good fit for the existing post-code flow: it works for both new and reused PRs and avoids workflow plumbing. If comment volumes become an issue, consider the GraphQL alternative to reduce pagination and improve performance, but the current best-effort REST approach is reasonable for now.

Files changed (5) +544 / -0

Enhancement (3) +370 / -0
pr-assignee.lib.shAdd library to resolve and best-effort assign a human PR owner +180/-0

Add library to resolve and best-effort assign a human PR owner

• Introduces assignee resolution using GitHub issue comments and metadata with precedence: latest human '/fs-code' commenter → first human issue assignee → human issue author. Implements bot/App filtering, REST pagination for comments, and guarded assignment that skips already-assigned PRs and logs failures without failing the run.

scripts/lib/pr-assignee.lib.sh

post-code.shIntegrate PR auto-assignment into bundled post-code script +185/-0

Integrate PR auto-assignment into bundled post-code script

• Bundles the new 'pr-assignee' library into the built 'post-code.sh' and invokes 'maybe_assign_pr' after both the "existing PR" path and the "new PR created" path. Keeps assignment best-effort and non-fatal to the overall automation.

scripts/post-code.sh

post-code.src.shSource PR-assignee library and call it after PR creation/update +5/-0

Source PR-assignee library and call it after PR creation/update

• Sources 'scripts/lib/pr-assignee.lib.sh' alongside existing failure-report utilities and calls 'maybe_assign_pr' for both reused and newly created PRs, matching the behavior of the bundled script.

scripts/post-code.src.sh

Tests (1) +173 / -0
pr-assignee-test.shAdd unit-style shell tests for assignee resolution precedence and filtering +173/-0

Add unit-style shell tests for assignee resolution precedence and filtering

• Adds a standalone bash test script that validates '/fs-code' detection, "latest invoker wins" behavior, bot/dependabot filtering, and skip conditions when PRs already have assignees. Skips when running in bundled test mode to align with existing script test patterns.

scripts/pr-assignee-test.sh

Other (1) +1 / -0
MakefileRun PR-assignee tests in 'script-test' target +1/-0

Run PR-assignee tests in 'script-test' target

• Adds 'scripts/pr-assignee-test.sh' to the 'script-test' recipe so the new assignee resolution logic is exercised in local/CI test runs.

Makefile

Rename maybe_assign_pr's local so the bundled post-code.sh no longer
trips shellcheck SC2153 against post-failure-report's PR_NUMBER.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 19, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 7:52 AM UTC · Ended 7:56 AM UTC
Commit: 3d48dce · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Jul 19, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

Context used
✅ Compliance rules (platform): 55 rules
✅ Skills: 4 invoked
  code-review
  code-implementation
  pr-review
  docs-review

Grey Divider


Action required

1. ::warning:: echoes unsanitized vars 📜 Skill insight ⛨ Security
Description
The new maybe_assign_pr code emits GitHub Actions workflow commands (e.g., ::warning::) with
interpolated variables that are not fully sanitized, enabling workflow-command injection via control
chars/ANSI/newlines in assign_err (and unsanitized interpolation in the non-gha_echo path). This
violates the requirement to sanitize every interpolated value for ::, %0A/%0D, ANSI escapes, and
control characters before emitting workflow commands.
Code

scripts/lib/pr-assignee.lib.sh[R143-177]

+  if ! existing_count="$(gh pr view "${pr_number}" --repo "${REPO_FULL_NAME}" \
+    --json assignees --jq '.assignees | length' 2>/dev/null)"; then
+    if declare -F gha_echo >/dev/null 2>&1; then
+      gha_echo warning "Could not read assignees for PR #${pr_number} — skipping assignment"
+    else
+      echo "::warning::Could not read assignees for PR #${pr_number} — skipping assignment"
+    fi
+    return 0
+  fi
+  if [[ "${existing_count}" != "0" ]]; then
+    echo "PR #${pr_number} already has assignees — skipping assignment"
+    return 0
+  fi
+
+  local assignee
+  assignee="$(resolve_pr_assignee || true)"
+  if [[ -z "${assignee}" ]]; then
+    echo "No human assignee candidate — leaving PR #${pr_number} unassigned"
+    return 0
+  fi
+
+  echo "Assigning PR #${pr_number} to ${assignee}..."
+  local assign_err
+  assign_err="$(gh pr edit "${pr_number}" --repo "${REPO_FULL_NAME}" \
+    --add-assignee "${assignee}" 2>&1)" || {
+    if declare -F gha_echo >/dev/null 2>&1; then
+      gha_echo warning "Failed to assign PR #${pr_number} to ${assignee} — continuing"
+      if [[ -n "${assign_err}" ]]; then
+        gha_echo warning "${assign_err}"
+      fi
+    else
+      echo "::warning::Failed to assign PR #${pr_number} to ${assignee} — continuing"
+      if [[ -n "${assign_err}" ]]; then
+        echo "::warning::${assign_err//::/ }"
+      fi
Relevance

⭐⭐⭐ High

Sanitizing GHA workflow commands has prior acceptance (sanitize '::' etc.); see accepted PR184 and
partial PR172.

PR-#184
PR-#172

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1538382 requires sanitizing *all* interpolated variables in GHA workflow commands
for ::, %0A/%0D, ANSI escapes, and control characters. The new code emits ::warning::... lines
(and passes raw assign_err to gha_echo) without the required sanitization, and the current
gha_echo sanitizer only strips :: and %0A/%0D (not ANSI/control characters).

scripts/lib/pr-assignee.lib.sh[143-177]
scripts/lib/post-failure-report.lib.sh[25-33]
scripts/lib/post-failure-report.lib.sh[65-70]
Skill: pr-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`maybe_assign_pr` emits GitHub Actions workflow commands (`::warning::...`) with interpolated values that are not sanitized to the required standard (must handle `::`, `%0A/%0D`, ANSI escapes, and control characters). In particular, the fallback `echo "::warning::..."` path does not sanitize `pr_number`/`assignee`, and the `assign_err` line only strips `::` (and the `gha_echo` sanitizer currently does not address ANSI/control chars).

## Issue Context
This code runs in CI contexts where GHA workflow commands are parsed by the runner; unsanitized values can inject additional commands.

## Fix Focus Areas
- scripts/lib/pr-assignee.lib.sh[143-177]
- scripts/lib/post-failure-report.lib.sh[25-33]
- scripts/lib/post-failure-report.lib.sh[65-70]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Protected scripts/ paths modified ✗ Dismissed 📜 Skill insight § Compliance
Description
This PR modifies files under scripts/, which is a protected governance/infrastructure path
requiring explicit human review and must not be auto-approved. Even with a linked issue, a
protected-path finding is required to ensure correct review controls are applied.
Code

scripts/post-code.src.sh[R42-46]

SCRIPT_DIR_POST="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# shellcheck source=lib/post-failure-report.lib.sh
source "${SCRIPT_DIR_POST}/lib/post-failure-report.lib.sh"
+# shellcheck source=lib/pr-assignee.lib.sh
+source "${SCRIPT_DIR_POST}/lib/pr-assignee.lib.sh"
Relevance

⭐⭐ Medium

Repo has protected-path findings for skills/.github (PR59/PR184), but no clear precedent that
scripts/ is protected.

PR-#59
PR-#184

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1538392 lists scripts/ as a protected path and requires raising a finding
whenever such files are modified. The diff adds sourcing of a new library within
scripts/post-code.src.sh, demonstrating modification under the protected directory.

scripts/post-code.src.sh[42-46]
Skill: pr-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Protected governance/infrastructure paths were modified; this PR must not be auto-approved and requires explicit human review.

## Issue Context
Protected paths include `scripts/` per the compliance checklist.

## Fix Focus Areas
- scripts/post-code.src.sh[42-46]
- scripts/lib/pr-assignee.lib.sh[1-180]
- scripts/post-code.sh[393-574]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Uncaught jq errors ✓ Resolved 🐞 Bug ☼ Reliability
Description
resolve_pr_assignee_from_context uses jq filters that can error on missing/null fields (notably
.assignees[].login) without suppressing stderr or normalizing the input, which can produce noisy
workflow logs and cause the assignee-candidate lookup to be skipped unexpectedly. This is
inconsistent with find_fs_code_invoker’s 2>/dev/null || true best-effort parsing and can reduce
assignment reliability under atypical API responses.
Code

scripts/lib/pr-assignee.lib.sh[R91-106]

+  local human_assignee
+  human_assignee="$(echo "${issue_json}" | jq -r '
+    [.assignees[].login | select(
+      (startswith("app/") | not) and
+      (test("\\[bot\\]$") | not) and
+      (. != "dependabot")
+    )] | .[0] // empty
+  ')"
+  if [[ -n "${human_assignee}" ]]; then
+    echo "${human_assignee}"
+    return 0
+  fi
+
+  local author_login
+  author_login="$(echo "${issue_json}" | jq -r '.author.login // empty')"
+  if is_human_github_user "${author_login}"; then
Relevance

⭐⭐⭐ High

Team favors best-effort jq parsing; PR56/PR94 discussions added stderr suppression/null-guards to
avoid jq aborts.

PR-#56
PR-#94

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The assignee/author extraction jq calls do not suppress errors or guard null iteration, while the
comment invoker jq path explicitly suppresses jq errors; this makes the issue JSON parsing path less
best-effort and more likely to produce noisy failures under unexpected shapes.

scripts/lib/pr-assignee.lib.sh[91-106]
scripts/lib/pr-assignee.lib.sh[51-65]
scripts/lib/pr-assignee.lib.sh[7-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`resolve_pr_assignee_from_context` runs jq over `issue_json` using `.assignees[].login` and `.author.login` without any null-guards or stderr suppression. If `issue_json` is malformed or has an unexpected shape (e.g., `assignees` missing/null), jq can emit errors to stderr and the assignee extraction will fail noisily, reducing best-effort assignment quality.

### Issue Context
This library already treats comment parsing as best-effort by suppressing jq errors (`2>/dev/null || true`). The issue-assignee/author parsing should follow the same pattern.

### Fix Focus Areas
- scripts/lib/pr-assignee.lib.sh[91-106]

### Suggested change
- Update the jq program to tolerate missing/null fields, e.g.:
 - use `(.assignees // [])[]? | .login?` instead of `.assignees[].login`
 - use `.author.login? // empty`
- Suppress jq stderr and fail-soft, e.g. append `2>/dev/null || true` to both jq invocations so unexpected input results in `""` (no candidate) rather than noisy errors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread scripts/lib/pr-assignee.lib.sh Outdated
Comment thread scripts/post-code.src.sh
Comment thread scripts/lib/pr-assignee.lib.sh
Route assignment warnings through gha_echo (no raw ::warning:: fallback)
and make issue assignee/author jq parsing null-safe and fail-soft.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:57 AM UTC · Completed 8:08 AM UTC
Commit: c744f89 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review

Re-review of e7f5deb (prior review at 395f0f1, provenance: app-verified). Changes since prior review: harness/code.yaml, scripts/lib/pr-assignee.lib.sh, scripts/post-code.sh.

Prior findings: [protected-path] (medium) unchanged — scripts/ and harness/ still modified; [defense-in-depth] (low) resolved — regex guard ^[a-zA-Z0-9_-]+$ now validates assignee format before gh pr edit; [docs-gap] (low) resolved — harness/code.yaml comment now includes "assign human".

Clean, well-structured feature addition. The library correctly implements the precedence chain from fullsend-ai/fullsend#1880 with proper bot filtering, idempotency guard, best-effort semantics, and defense-in-depth input validation. Test coverage is comprehensive (190 lines, 18 test cases covering null bodies, CRLF, leading whitespace, and bot filtering).

Findings

Medium

  • [protected-path] scripts/lib/pr-assignee.lib.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/post-code-test.sh, scripts/pr-assignee-test.sh, harness/code.yaml — PR modifies files under scripts/ and harness/, protected paths. The linked issue (fullsend-ai/fullsend#1880) explicitly authorizes adding assignment logic to post-code.sh and shell tests for the resolver. Human approval is always required for protected-path changes, regardless of context. (Severity anchored from prior review at 395f0f1.)

Low

  • [docs-gap] skills/code-implementation/SKILL.md line 247 — Documentation states "the post-script will push the branch and create the PR" but does not mention the new PR assignment step.

    • Remediation: Update to: "the post-script will push the branch, create the PR, and assign it to a human owner"
  • [docs-gap] skills/code-implementation/SKILL.md line 770 — Documentation states "The post-script handles pushing, PR creation, and failure reporting" but does not mention PR assignment.

    • Remediation: Update to: "The post-script handles pushing, PR creation, PR assignment, and failure reporting."
Previous run

Review

Re-review of 395f0f1 (prior review at 2d0b158, provenance: app-verified). Changes since prior review: docs/code.md, scripts/lib/pr-assignee.lib.sh, scripts/post-code-test.sh, scripts/post-code.sh, scripts/pr-assignee-test.sh.

All prior findings addressed: [null-body-crash] (medium) resolved — jq first_word now handles null body correctly; [dual-impl-divergence] (low) resolved — duplicate bash function removed, only jq path remains; [missing-drift-guard] (low) resolved — maybe_assign_pr check added to post-code-test.sh; [docs-gap] (low) partially resolved — docs/code.md now describes assignment behavior, but harness/code.yaml comment is stale.

Clean, well-structured feature addition. The library follows all established codebase conventions (idempotency guard, shellcheck directive, private function prefix, test patterns). The precedence chain, bot filtering, and best-effort semantics are correctly implemented. Integration at both the existing-PR and new-PR paths is correct.

Findings

Medium

  • [protected-path] scripts/lib/pr-assignee.lib.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/post-code-test.sh, scripts/pr-assignee-test.sh — PR modifies files under scripts/, a protected path. The linked issue (fullsend-ai/fullsend#1880) explicitly authorizes adding assignment logic to post-code.sh and shell tests for the resolver. Human approval is always required for protected-path changes, regardless of context. (Severity anchored from prior review at 2d0b158.)

Low

  • [defense-in-depth] scripts/lib/pr-assignee.lib.sh line 170 — The assignee variable, derived from external GitHub API JSON via jq, is passed to gh pr edit --add-assignee "${assignee}" without format validation. Double-quoting prevents shell injection and GitHub constrains usernames to [a-zA-Z0-9-], but a lightweight regex guard (consistent with the branch-name validation pattern in post-code.src.sh) would add defense-in-depth.

    • Remediation: Add if [[ ! "${assignee}" =~ ^[a-zA-Z0-9-]+$ ]]; then _pr_assignee_warn "Unexpected assignee format: skipping"; return 0; fi
  • [docs-gap] harness/code.yaml line 7 — The post_script responsibilities comment lists "protected-path check, secret scan, push branch, create PR" but does not mention PR assignee resolution, which is now part of the post-script flow.

    • Remediation: Update comment to include "assign PR".
Previous run (2)

Review

Re-review of 2d0b158 (prior review at c744f89, provenance: app-verified). New commit since prior review: 2d0b158 (merge conflict resolution with gitleaks-install).

Clean, well-structured feature addition. The library follows all established codebase conventions (idempotency guard, shellcheck directive, private function prefixes, test patterns). The precedence chain, bot filtering, and best-effort semantics are correctly implemented. Integration at both the existing-PR and new-PR paths is correct.

Findings

Medium

  • [null-body-crash] scripts/lib/pr-assignee.lib.sh line 60 — The jq first_word function in find_fs_code_invoker crashes when processing a comment with body: null (deleted or minimized comments). The chain (. // "") | split("\n") yields [] on an empty string, so [][0] is null, and null | gsub("\r$"; "") errors with "null cannot be matched." Since the entire jq expression is wrapped in 2>/dev/null || true, the error is silently suppressed and find_fs_code_invoker returns empty — even when a valid /fs-code comment exists later in the array. This causes silent fallthrough to the assignee/author path, potentially assigning the wrong person.

    • Remediation: Guard against null in the first_word def: ((. // "") | split("\n")[0] // "" | gsub("\r$"; "") | split(" ")[0])
  • [protected-path] scripts/lib/pr-assignee.lib.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/pr-assignee-test.sh — PR modifies files under scripts/, a protected path. The linked issue (fullsend-ai/fullsend#1880) explicitly authorizes adding assignment logic to post-code.sh and shell tests for the resolver. Human approval is always required for protected-path changes, regardless of context. (Severity anchored from prior review at c744f89.)

Low

  • [dual-impl-divergence] scripts/lib/pr-assignee.lib.sh line 37 — comment_is_fs_code() (bash, uses awk '{print $1}') and the jq first_word in find_fs_code_invoker() disagree on leading-whitespace handling: " /fs-code" matches in bash but not in jq. Only the jq path is used in production, but tests only exercise the bash function. If either implementation changes independently, the tests could pass while production behavior diverges.

    • Remediation: Either move comment_is_fs_code to the test file as a test helper (it is only called there), or add tests that exercise the jq path directly.
  • [missing-drift-guard] scripts/post-code-test.sh — The bundled-script function checks (lines 20–33) verify gha_echo, post_fail_to_issue, and install_gitleaks from other bundled libs, but do not check for maybe_assign_pr from the new pr-assignee.lib.sh. This breaks the existing drift-guard pattern.

    • Remediation: Add: if ! grep -q 'maybe_assign_pr' "${POST_SCRIPT}"; then echo 'FAIL: bundled-script-has-pr-assignee'; ...
  • [docs-gap] docs/code.md — The post-script responsibilities description (line 15) and capabilities section do not mention PR auto-assignment. The feature works without this update, but the doc is now incomplete.


Labels: PR modifies the code agent post-script to add PR auto-assignment.

Previous run (3)

Review

Findings

Medium

  • [protected-path] scripts/lib/pr-assignee.lib.sh, scripts/post-code.sh, scripts/post-code.src.sh, scripts/pr-assignee-test.sh — PR modifies files under scripts/, a protected path. The linked issue (fullsend-ai/fullsend#1880) explicitly authorizes adding assignment logic to post-code.sh and shell tests for the resolver. Human approval is always required for protected-path changes, regardless of context.

Labels: PR modifies the code agent post-script to add PR auto-assignment.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment code-agent labels Jul 19, 2026
@ifireball

Copy link
Copy Markdown
Member Author

CI is green on c744f89 (test, script-test, commit-lint, DCO).

Addressed Qodo findings in follow-up commits (shellcheck SC2153 rename; sanitize assignment warnings via gha_echo; null-safe jq). Fullsend review’s only finding is the expected protected-path note for scripts/ — that needs human approval.

@ifireball ifireball self-assigned this Jul 19, 2026
Keep both gitleaks-install and pr-assignee sources in post-code.src.sh
and regenerate the bundled post-code.sh.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:24 AM UTC · Completed 6:41 AM UTC
Commit: 2d0b158 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Null/minimized comment bodies no longer abort the whole invoker scan.
Trim leading whitespace in jq to match dispatch/awk, test the production
path directly, add a maybe_assign_pr bundle drift guard, and document
post-script assignment in docs/code.md.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:45 AM UTC · Completed 7:00 AM UTC
Commit: 395f0f1 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Add a lightweight GitHub-login regex guard and update the code harness
comment to mention PR assignment.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:03 AM UTC · Completed 7:24 AM UTC
Commit: e7f5deb · View workflow run →

@ifireball
ifireball added this pull request to the merge queue Jul 20, 2026
Merged via the queue into fullsend-ai:main with commit ca518d9 Jul 20, 2026
14 checks passed
@ifireball
ifireball deleted the feat/1880-auto-assign-pr-assignee branch July 20, 2026 20:32
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:35 PM UTC · Completed 8:54 PM UTC
Commit: e7f5deb · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #284 — auto-assign code-agent PRs to a human owner

Timeline

Human-authored PR by ifireball (made with Cursor, not the code agent) implementing fullsend-ai/fullsend#1880. 8 files changed (+577/-2), adding a new pr-assignee.lib.sh library (183 lines) with 18 test cases.

July 19: PR opened (07:49). Three review dispatches fired in quick succession — first two cancelled by concurrency group as the author pushed rapid fixes for Qodo bot findings. Third review (run 29678947790, commit c744f89, ~12 min) succeeded, finding only [protected-path] (medium) and [docs-gap] (low). Author confirmed CI green at 08:13.

July 20: Merge-conflict resolution commit 2d0b158. Fourth review (run 29721525543, ~18 min) found [null-body-crash] (medium) and [dual-impl-divergence] (low) — on code identical to what the third review examined. Author fixed in 395f0f1. Fifth review (run 29722511802, ~17 min) found [defense-in-depth] (low). Author fixed in e7f5deb. Sixth review (run 29723371873, ~21 min) confirmed all findings resolved. Human rh-hemartin approved at 11:40 (empty body, no additional findings). Merged at 20:33.

What went well

  • Review agent caught 3 real bugs across iterations: a medium-severity jq crash on null comment bodies, a dual-implementation divergence, and a missing input-format validation guard. All were genuine quality issues.
  • Complementary tooling: Qodo found security issues (unsanitized GHA workflow commands) and reliability issues (uncaught jq errors on assignee/author extraction). The fullsend review agent found different issues (null-body edge case, design divergence, defense-in-depth). Neither overlapped — good coverage breadth.
  • Fast author response: All review findings were fixed within minutes of being posted, with inline replies confirming each fix.
  • Protected-path detection worked correctly: The review agent flagged scripts/ as a protected path requiring human approval, applied requires-manual-review, and did not auto-approve.

First-pass review quality gap

The most notable finding: the first successful review (run #3, commit c744f89) found only 1 finding on a brand-new 183-line shell library. The second review (run #4, commit 2d0b158) found 4 additional real issues on byte-identical code in pr-assignee.lib.sh (the intervening commit only touched gitleaks-install files). This is a verified case of non-deterministic first-pass shallowness.

The missed [null-body-crash] bug was a classic data-processing edge case: jq first_word function crashed when comment.body was null because null | gsub("\r$"; "") errors, and the outer 2>/dev/null || true silently swallowed the error, causing a valid /fs-code comment later in the array to be skipped. This is a jq-specific pitfall distinct from the bash pitfalls covered by agents#131.

Evidence for existing issues

Workflow noise

30 fullsend.yaml runs triggered for this single PR; only 7 dispatched actual work (6 review + 1 retro). The remaining 23 were skipped, cancelled, or matched no stage. This is evidence for fullsend#1362 (filter bot-triggered label events) and fullsend#1271 (filter bot-triggered review events).

Unresolved low-severity finding

The [docs-gap] finding (update skills/code-implementation/SKILL.md to mention PR assignment) remained unresolved at merge time. The human approved without addressing it — likely an intentional trade-off for a low-severity finding, but the docs gap now exists.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-agent requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants