Skip to content

feat(agents): add gh-classify agent - #619

Closed
ascerra wants to merge 4 commits into
mainfrom
feat/add-gh-classify-agent
Closed

feat(agents): add gh-classify agent#619
ascerra wants to merge 4 commits into
mainfrom
feat/add-gh-classify-agent

Conversation

@ascerra

@ascerra ascerra commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Review notes

Self-review before open:

Area Result
Correctness Post-script tests cover dry-run, threshold skip, null category, validated-iteration preference
Security Project PAT not in sandbox env; issue content treated as untrusted; policy matches triage (gh + Vertex, no curl)
Scope Agent implementation only — no .fullsend registration in this PR
Conventions Matches scribe extraction file layout; docs indexed in docs/README.md

Test plan

  • bash scripts/post-gh-classify-test.sh passes locally
  • CI script-test / lint green
  • After merge: .fullsend PR to append agents: entry pinned to merge SHA and remove customized/*gh-classify* overlay

Port the GitHub issue classification agent from fullsend-ai/.fullsend
customized/ into the shared agents repo, matching the scribe extraction
pattern (agents#10). Keeps org category docs and the workflow in .fullsend.

Signed-off-by: Adam Scerra <ascerra@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ascerra
ascerra requested a review from a team as a code owner August 3, 2026 11:17
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 11:18 AM UTC · Ended 11:21 AM UTC
Commit: ef94d9b · View workflow run →

Signed-off-by: Adam Scerra <ascerra@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:22 AM UTC · Completed 11:38 AM UTC
Commit: c31e18d · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [role-slug-collision] harness/gh-classify.yaml:7role: triage and slug: fullsend-ai-triage collide with the existing triage agent (harness/triage.yaml:8-9). Every other agent in the repo uses a unique role/slug pair (except code/fix which share coder intentionally as closely related agents). The triage and classify agents are functionally distinct — triage assesses issue sufficiency and applies control labels, while classify assigns project board categories. This collision risks silent misdispatch if the harness resolves agents by role or slug.
    Remediation: Use distinct identifiers, e.g., role: classify and slug: fullsend-ai-classify.

  • [protected-path] agents/gh-classify.md, harness/gh-classify.yaml, policies/gh-classify.yaml, scripts/post-gh-classify.sh, scripts/pre-gh-classify.sh, scripts/post-gh-classify-test.sh, skills/issue-classification/SKILL.md — 7 files under protected paths (agents/, harness/, policies/, scripts/, skills/) are modified. The PR has no linked issue to authorize changes to governance and infrastructure files. Human approval is required for all protected-path changes.
    Remediation: Link an authorizing issue and obtain human maintainer review for protected-path changes.

Medium

  • [missing-authorization] agents/gh-classify.md — This PR adds a complete new agent with prompt, harness, policy, schema, scripts, skill, tests, and documentation — 13 files total. The PR body references #10 but does not link to an issue authorizing this port. Non-trivial changes require explicit authorization via a linked issue.

Low

  • [path-traversal] harness/gh-classify.yaml:22CLASSIFY_CATEGORIES_PATH is used as a host_files source path without path validation in this PR. This is a standard pattern across the repo (other agents use ${GOOGLE_APPLICATION_CREDENTIALS}, ${GCP_OIDC_TOKEN_FILE}, etc. in host_files.src the same way), and workflow dispatch inputs require authorized collaborator access.

  • [edge-case] scripts/post-gh-classify.sh:57 — The backward-compat fallback for dir in iteration-*/output comment states "glob order = naturally ascending iteration numbers," but bash glob expansion is lexicographic, not numeric. For iteration counts >= 10, iteration-10 sorts before iteration-2. Mitigated by max_iterations: 2 in the harness config.

  • [workflow-command-injection] scripts/pre-gh-classify.sh:58CLASSIFY_MODE and CLASSIFY_SOURCE_REPO are interpolated into ::notice:: workflow commands. Both are validated before emission (strict case statement and regex). Risk is negligible.

  • [content-injection] scripts/post-gh-classify.sh:593 — The agent's reasoning field flows into the Step Summary markdown table with partial sanitization (pipe, newline, backtick, angle bracket escaping, 100-char truncation). GitHub also applies HTML sanitization.

  • [content-injection] scripts/post-gh-classify.sh:583FILTER_CATEGORY is interpolated unsanitized into Step Summary markdown. Limited attack surface (authorized users only, GitHub HTML sanitization applies).

  • [token-scrubbing] scripts/post-gh-classify.sh:396 — In the set_project_field error handler, PROJECT_GH_TOKEN is scrubbed via sed regex. If the token contained sed metacharacters, the substitution could fail. In practice, GitHub tokens are alphanumeric.

  • [scope-creep] README.md:18, docs/README.md:13 — Both files add a Scribe row alongside gh-classify. Scribe already exists on main but was missing from these tables — a minor doc gap fix bundled with the gh-classify entries.

  • [naming-convention] agents/gh-classify.md:1 — The gh- prefix departs from the established naming convention; all other agents use unprefixed names (triage, scribe, code, fix, review, retro, prioritize).


Labels: PR adds a new agent with protected infrastructure files requiring human review


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

Review

Findings

High

  • [role-slug-collision] harness/gh-classify.yaml:7role: triage and slug: fullsend-ai-triage collide with the existing triage agent (harness/triage.yaml:8-9). Every other agent in the repo uses a unique role/slug pair (except code/fix which share coder intentionally as closely related agents). The triage and classify agents are functionally distinct — triage assesses issue sufficiency and applies control labels, while classify assigns project board categories. This collision risks silent misdispatch if the harness resolves agents by role or slug.
    Remediation: Use distinct identifiers, e.g., role: classify and slug: fullsend-ai-classify.

  • [protected-path] agents/gh-classify.md, harness/gh-classify.yaml, policies/gh-classify.yaml, scripts/post-gh-classify.sh, scripts/pre-gh-classify.sh, scripts/post-gh-classify-test.sh, skills/issue-classification/SKILL.md — 7 files under protected paths (agents/, harness/, policies/, scripts/, skills/) are modified. The PR has no linked issue to authorize changes to governance and infrastructure files. Human approval is required for all protected-path changes.
    Remediation: Link an authorizing issue and obtain human maintainer review for protected-path changes.

Low

  • [path-traversal] harness/gh-classify.yaml:18CLASSIFY_CATEGORIES_PATH is used as a host_files source path without path validation in this PR. This is a standard pattern across the repo (other agents use ${GOOGLE_APPLICATION_CREDENTIALS}, ${GCP_OIDC_TOKEN_FILE}, etc. in host_files.src the same way), and workflow dispatch inputs require authorized collaborator access.

  • [edge-case] scripts/post-gh-classify.sh:483 — The backward-compat fallback for dir in iteration-*/output comment states "glob order = naturally ascending iteration numbers," but bash glob expansion is lexicographic, not numeric. For iteration counts >= 10, iteration-10 sorts before iteration-2. Mitigated by max_iterations: 2 in the harness config.

  • [workflow-command-injection] scripts/pre-gh-classify.sh:40CLASSIFY_MODE and CLASSIFY_SOURCE_REPO are interpolated into ::notice:: workflow commands. Both are validated before emission (strict case statement and regex). Risk is negligible.

  • [content-injection] scripts/post-gh-classify.sh:533 — The agent's reasoning field flows into the Step Summary markdown table with partial sanitization (pipe, newline, backtick, angle bracket escaping, 100-char truncation). GitHub also applies HTML sanitization.

  • [content-injection] scripts/post-gh-classify.sh:521FILTER_CATEGORY is interpolated unsanitized into Step Summary markdown. Limited attack surface (authorized users only, GitHub HTML sanitization applies).

  • [token-scrubbing] scripts/post-gh-classify.sh:473 — In the set_project_field error handler, PROJECT_GH_TOKEN is scrubbed via sed regex. If the token contained sed metacharacters, the substitution could fail. In practice, GitHub tokens are alphanumeric.

  • [scope-creep] README.md:24, docs/README.md:98 — Both files add a Scribe row alongside gh-classify. Scribe already exists on main but was missing from these tables — a minor doc gap fix bundled with the gh-classify entries.

  • [naming-convention] agents/gh-classify.md:1 — The gh- prefix departs from the established naming convention; all other agents use unprefixed names (triage, scribe, code, fix, review, retro, prioritize).


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (2)

Review

Findings

High

  • [protected-path] agents/gh-classify.md, harness/gh-classify.yaml, policies/gh-classify.yaml, scripts/post-gh-classify.sh, scripts/pre-gh-classify.sh, scripts/post-gh-classify-test.sh, skills/issue-classification/SKILL.md — 7 files under protected paths (agents/, harness/, policies/, scripts/, skills/) are modified. The PR has no linked issue to authorize changes to governance and infrastructure files. Human approval is required for all protected-path changes.
    Remediation: Link an authorizing issue and obtain human maintainer review for protected-path changes.

  • [role-slug-collision] harness/gh-classify.yaml:7role: triage and slug: fullsend-ai-triage collide with the existing triage agent (harness/triage.yaml:8-9). Every other agent in the repo uses a unique role/slug pair. This collision risks silent misdispatch if the harness resolves agents by role or slug.
    Remediation: Use distinct identifiers, e.g., role: classify and slug: fullsend-ai-classify.

Medium

  • [missing-doc] docs/gh-classify.md — The documentation requires a CLASSIFY_CATEGORIES_PATH pointing to a categories document but never explains its expected format, structure, or provides an example. The skill file (skills/issue-classification/SKILL.md) partially describes the format (markdown with headings as category names) but end-user documentation should be self-contained.
    Remediation: Add a section to docs/gh-classify.md documenting the categories document format with a concrete example.

  • [edge-case] scripts/post-gh-classify.sh:411ALREADY_CLASSIFIED is computed as ALL_OPEN_COUNT - CANDIDATE_COUNT. A race condition between the pre-script's issue fetch and the post-script's execution could result in CANDIDATE_COUNT > ALL_OPEN_COUNT, producing a negative value that corrupts the summary report numbers.
    Remediation: Add a guard: (( ALREADY_CLASSIFIED < 0 )) && ALREADY_CLASSIFIED=0.

  • [stale-doc] README.md — The main repository README agent table lists 6 agents but omits gh-classify (added in this PR) and scribe (already exists in the repo). The docs/README.md is updated in this PR but the main README.md is not.
    Remediation: Add gh-classify to the README.md agent table. Consider also adding scribe (pre-existing omission).

Low

  • [missing-authorization] — This PR adds a complete new agent without a linked issue. Non-trivial changes benefit from explicit authorization via a linked issue.

  • [dead-code] scripts/post-gh-classify.sh:270 — Variable ACTIONS_TAKEN is assigned on lines 270, 292, and 297 but is never read. Likely an incomplete feature.

  • [test-inadequate] scripts/post-gh-classify-test.sh:28 — All four test cases run with CLASSIFY_DRY_RUN=true. The live-run path through set_project_field (GraphQL mutations, error handling) is not exercised.

  • [workflow-command-injection] scripts/pre-gh-classify.sh:40CLASSIFY_MODE is interpolated into a ::notice:: workflow command before the value is validated by the case statement at line 91. Risk is limited since CLASSIFY_MODE originates from workflow dispatch inputs by authorized users.

  • [content-injection] scripts/post-gh-classify.sh:533 — The agent's reasoning field flows into the GitHub Step Summary markdown table. Partial sanitization exists (pipe and newline escaping, 100-char truncation), and GitHub applies its own HTML sanitization, limiting practical risk.

  • [scope-boundary] docs/gh-classify.md:16 — Documentation mentions an "enrolled-repo shim on issues.opened" but does not explain enrollment or link to external documentation.

  • [naming-consistency] agents/gh-classify.md:1 — The gh- prefix departs from the established naming convention; all other agents use unprefixed names (triage, scribe, code, fix, review, retro, prioritize).

  • [script-naming-consistency] scripts/post-gh-classify.sh:2 — Header comment says post-classify.sh instead of post-gh-classify.sh.

  • [script-naming-consistency] scripts/pre-gh-classify.sh:2 — Header comment says pre-classify.sh instead of pre-gh-classify.sh.

  • [harness-field-ordering] harness/gh-classify.yaml:29 — Field ordering follows the minority pattern (pre_scriptvalidation_looppost_script). The majority of harness files use pre_scriptpost_scriptvalidation_loop.


Labels: PR adds a new agent (gh-classify) with supporting infrastructure


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the enhancement New feature or request label Aug 3, 2026
Document categories format, harden post/pre scripts, expand tests,
and sync README agent table. Leave triage role/slug intentional.

Signed-off-by: Adam Scerra <ascerra@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ascerra

ascerra commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the medium/low review findings in the latest push. Leaving the two high findings as-is:

Why the high findings don't apply here

  1. protected-path / missing linked issue — This PR is the authorizing change: extracting the existing .fullsend customized gh-classify agent into the shared agents repo (same pattern as scribe in feat(agents): add scribe agent #10). No separate tracking issue is needed for this migration; human maintainer review of the PR is the approval path.

  2. role-slug-collision (role: triage / slug: fullsend-ai-triage) — Intentional and noted. The .fullsend workflow still reuses the triage GitHub App for classify (issues + org projects). Changing role/slug would require mint/role enrollment and workflow credential changes that are out of scope for this extraction. Follow-up later if/when a dedicated classify app is created.

Fixed

  • Categories document format + enrollment wording in docs/gh-classify.md
  • ALREADY_CLASSIFIED negative guard + test
  • Remove unused ACTIONS_TAKEN
  • Validate CLASSIFY_MODE / repo before ::notice::
  • Stronger step-summary sanitization for reasoning
  • Script header names + harness field order
  • Live-run GraphQL path covered in unit tests
  • Root README.md agent table (scribe + gh-classify)

Not changing

  • gh- name prefix — preserves the existing customized agent / workflow identity in .fullsend

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:45 AM UTC · Completed 12:01 PM UTC
Commit: 0f40848 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional review findings (independent pass) — 9 items not previously flagged by the existing fullsend-ai-review bot pass, covering a candidate-set validation gap in the post-script, an error-handling bug under set -e, test-coverage gaps, and a few documentation/consistency issues. See inline comments.

Comment thread scripts/post-gh-classify.sh
Comment thread scripts/post-gh-classify.sh
Comment thread scripts/post-gh-classify.sh
Comment thread scripts/post-gh-classify.sh Outdated
Comment thread scripts/post-gh-classify.sh Outdated
Comment thread scripts/post-gh-classify.sh Outdated
Comment thread skills/issue-classification/SKILL.md Outdated
Comment thread agents/gh-classify.md
Comment thread skills/issue-classification/SKILL.md Outdated
Host candidate-set guard, set -e GraphQL resilience, per-repo categories
docs/skill fallback, Read/Write skill guidance, and expanded tests.

Signed-off-by: Adam Scerra <ascerra@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:53 PM UTC · Completed 2:09 PM UTC
Commit: 7de8120 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread harness/gh-classify.yaml
model: opus
image: ghcr.io/fullsend-ai/fullsend-sandbox:latest
policy: policies/gh-classify.yaml
role: triage

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] role-slug-collision

role: triage and slug: fullsend-ai-triage collide with the existing triage agent (harness/triage.yaml:8-9). Every other agent in the repo uses a unique role/slug pair (except code/fix which share coder intentionally as closely related agents). The triage and classify agents are functionally distinct — triage assesses issue sufficiency and applies control labels, while classify assigns project board categories. This collision risks silent misdispatch if the harness resolves agents by role or slug.

Suggested fix: Use distinct identifiers, e.g., role: classify and slug: fullsend-ai-classify.

Comment thread harness/gh-classify.yaml
- src: env/gh-classify.env
dest: /sandbox/workspace/.env.d/gh-classify.env
expand: true
- src: ${CLASSIFY_CATEGORIES_PATH}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] path-traversal

CLASSIFY_CATEGORIES_PATH is used as a host_files source path without path validation in this PR. This is a standard pattern across the repo (other agents use ${GOOGLE_APPLICATION_CREDENTIALS}, ${GCP_OIDC_TOKEN_FILE}, etc. in host_files.src the same way), and workflow dispatch inputs require authorized collaborator access.

exit 1
fi
else
# Backward compatibility: scan iteration-N/ subdirectories for the last

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] edge-case

The backward-compat fallback for dir in iteration-*/output comment states 'glob order = naturally ascending iteration numbers,' but bash glob expansion is lexicographic, not numeric. For iteration counts >= 10, iteration-10 sorts before iteration-2. Mitigated by max_iterations: 2 in the harness config.


# Log mode info (::notice:: only renders in GitHub Actions).
if [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then
echo "::notice::Classify agent — mode=${CLASSIFY_MODE}, repo=${CLASSIFY_SOURCE_REPO}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] workflow-command-injection

CLASSIFY_MODE and CLASSIFY_SOURCE_REPO are interpolated into ::notice:: workflow commands. Both are validated before emission (strict case statement and regex). Risk is negligible.

.[] | select(.status == "classified") |
"| #\(.issue_number) | \(.workstream_category // "—") | \((.confidence * 100) | floor)% | \(.category_action) |"
' "${REPORT_FILE}" 2>/dev/null || echo "| — | — | — | — |"
echo ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] content-injection

The agent's reasoning field flows into the Step Summary markdown table with partial sanitization (pipe, newline, backtick, angle bracket escaping, 100-char truncation). GitHub also applies HTML sanitization.

if [[ -n "${FILTER_CATEGORY}" ]]; then
echo ""
echo "**Filter:** ${FILTER_CATEGORY}"
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] content-injection

FILTER_CATEGORY is interpolated unsanitized into Step Summary markdown. Limited attack surface (authorized users only, GitHub HTML sanitization applies).

if [[ ${#CLASSIFIED_LINES[@]} -gt 0 ]]; then
echo "CLASSIFIED (${#CLASSIFIED_LINES[@]} issues)"
echo "------------------------------------------------------------"
for line in "${CLASSIFIED_LINES[@]}"; do

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] token-scrubbing

In the set_project_field error handler, PROJECT_GH_TOKEN is scrubbed via sed regex. If the token contained sed metacharacters, the substitution could fail. In practice, GitHub tokens are alphanumeric.

Comment thread README.md
| **Scribe** | Maps meeting notes to the GitHub issue backlog | Schedule, `/fs-scribe` |
| **GitHub Classify** | Classifies GitHub issues into project board categories | Workflow dispatch, issue shim |

See [`docs/`](docs/) for detailed documentation on each agent.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] scope-creep

Both README.md and docs/README.md add a Scribe row alongside gh-classify. Scribe already exists on main but was missing from these tables — a minor doc gap fix bundled with the gh-classify entries.

Comment thread agents/gh-classify.md
@@ -0,0 +1,58 @@
---

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] naming-convention

The gh- prefix departs from the established naming convention; all other agents use unprefixed names (triage, scribe, code, fix, review, retro, prioritize).

@fullsend-ai-review fullsend-ai-review Bot added the needs-human Agent loop needs human intervention label Aug 3, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review-only pass (deduped against existing inline comments/reviews). 9 additional findings covering: a role-permission gap that would 403 every project-board write in the documented default install path, a sandbox credential/network-policy exposure inconsistent with the PR's own security model and prior ADR guidance, a sandbox-side fallback that silently upgrades "unclassified" mode to "all" mode, dry-run mode not exercising config validation, missing timeouts on GraphQL calls, and Step Summary reporting gaps/mislabeling.

Comment thread harness/gh-classify.yaml
model: opus
image: ghcr.io/fullsend-ai/fullsend-sandbox:latest
policy: policies/gh-classify.yaml
role: triage

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] role: triage has no organization_projects permission — every project-board write (and read) will 403

gh-classify.yaml declares role: triage. In fullsend-ai/fullsend's internal/mintcore/github.go, canonicalRolePermissions["triage"] is {"contents":"read","issues":"write","metadata":"read"} — no organization_projects scope at all, read or write. By contrast the prioritize role, used by the structurally similar prioritize agent for the same kind of Projects V2 field write, explicitly includes "organization_projects":"write".

In scripts/pre-gh-classify.sh, the project-metadata GraphQL query (querying organization(login: $org) { projectV2(...) }) and the already-classified-issue query both run with PROJECT_GH_TOKEN="${GH_TOKEN}" (falls back to plain GH_TOKEN whenever CLASSIFY_PROJECT_TOKEN is unset — i.e. the default, documented same-org install path), and the code already has a designed fallback for when that query fails (PROJECT_ACCESS_OK="false" → "treating all issues as unclassified"). In scripts/post-gh-classify.sh, set_project_field()'s addProjectV2ItemById/updateProjectV2ItemFieldValue mutations also use the same PROJECT_GH_TOKEN="${CLASSIFY_PROJECT_TOKEN:-${GH_TOKEN}}.

Net effect: in the primary, documented same-org path (no CLASSIFY_PROJECT_TOKEN configured), every live run will fail to discover project metadata and every set_project_field call will 403 — the agent's core function (assigning a project-board category) cannot work as merged.

Failure scenario: Install gh-classify same-org with no CLASSIFY_PROJECT_TOKEN (the documented default). The pre-script's project-metadata GraphQL query returns a permission error (no organization_projects scope on the triage-role token), so project-meta.json ends up with null project_id/field_id. Every post-script set_project_field() call then hits the empty-ID guard and returns 1; the run's Step Summary shows every classified issue with an error and zero actual category writes ever happen.

Suggestion: Add a classify role to fullsend's canonicalRolePermissions with organization_projects: write (plus issues read/write), and register a matching GitHub App/slug, or reuse role: prioritize directly. This is a cross-repo dependency that must land before or alongside this PR.

Comment thread harness/gh-classify.yaml
CLASSIFY_MIN_CONFIDENCE: ${CLASSIFY_MIN_CONFIDENCE}
CLASSIFY_DRY_RUN: ${CLASSIFY_DRY_RUN}
sandbox:
GH_TOKEN: "${GH_TOKEN}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] Sandbox is given the same write-capable GH_TOKEN plus full read-write GitHub network access, contradicting the PR's own "writes never happen in the sandbox" security model

This line passes the identical ${GH_TOKEN} into both env.runner (used by pre/post scripts for issues:write + org projects writes) and env.sandbox. policies/gh-classify.yaml's github_api network policy (lines 37-49) grants access: read-write to api.github.com and github.com for the **/gh binary — the exact binary allowlisted in tools: Bash(gh,jq) for the sandboxed agent.

docs/gh-classify.md's security-model table and this PR's own self-review assert that project writes never happen inside the sandbox, but nothing at the credential or network layer enforces that — the sandboxed agent (which processes untrusted, attacker-influenced issue content per this PR's own threat framing) can call gh issue edit/close/comment or an arbitrary GraphQL mutation directly with the live token; the prompt's prose instruction ("NEVER modify issue content, labels, or state") is the only barrier.

This is precisely the "scoped token in sandbox" pattern that fullsend's own ADR 0017 (Credential Isolation for Sandboxed Agents) explicitly rejected — "any credential present in the sandbox must be assumed exfiltrable" — in favor of prefetch + post-process as "the expected model for any new agent." The immediately-preceding sibling scribe agent already follows that default: policies/scribe.yaml has no github_api network policy at all, getting all GitHub data via host-side pre-fetch. gh-classify's own pre-script already computes open-issues.json and project-meta.json on the host but does not mount them into the sandbox, so the skill has the sandboxed agent re-discover the same data live with the injected token instead. (Note: the pre-existing triage.yaml/policies/triage.yaml has the identical pattern already merged, so this PR did not invent the hole — but it reproduces it wholesale in brand-new files, and the PR's security self-review doesn't discuss the trade-off.)

Failure scenario: An issue body/comment contains a prompt-injection payload instructing the agent to run gh issue comment <n> --body ... or an equivalent GraphQL mutation using the injected GH_TOKEN. The github_api network policy permits read-write to api.github.com from the gh binary inside the sandbox, and the token itself carries issues:write — the write succeeds with no technical control blocking it, only the prompt's prose instruction which the injection is specifically trying to override.

Suggestion: Either (a) mount the pre-script's already-computed open-issues.json/project-meta.json/already-classified list into the sandbox as host_files (as scribe does) and drop GH_TOKEN plus the github_api network policy from env.sandbox entirely, or (b) if live sandbox reads must be kept, mint a distinct, read-only-scoped token for env.sandbox separate from the write-capable token used by env.runner/post-script, and scope the github_api network policy to read-only.


Then **exclude issues that are already classified** on the project
board. The pre-script identifies these on the host, but you are in a
sandbox and must discover them yourself. Query the project board to

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Sandbox-side already-classified discovery degrades the same way as the pre-script, silently turning "unclassified" mode into "all" mode every run

This step instructs the sandboxed agent to independently query the org Project board via GraphQL using GH_TOKEN to exclude already-classified issues in batch (unclassified/all) modes: "If the project query fails (permissions, network), log a warning and continue with the open-issue list you already fetched." Given the CRITICAL role-permission gap on harness/gh-classify.yaml:7 (role: triage has no organization_projects scope at all), this query will always fail in the sandbox too, exactly as it does in the pre-script. The fallback is graceful (no crash) and there is a host-side safety net (the post-script rejects out-of-candidate issue_numbers), so this doesn't cause incorrect writes — but it does mean unclassified mode silently behaves like all mode on every single run: the entire open-issue backlog gets re-evaluated (repeated LLM inference cost) and every classified issue's project-write attempt still fails per the permission-gap finding.

Failure scenario: Same-org install running in unclassified mode on a repo with hundreds of already-classified issues. Both the pre-script's and the sandbox's project-board queries fail due to the missing organization_projects permission; every run re-evaluates the full open-issue backlog instead of only the genuinely new/unclassified ones, multiplying inference cost and producing hundreds of failed project-write attempts per run.

Suggestion: Fix the root role-permission gap. Independently, surface a loud, visible warning (in the GitHub Step Summary, not just stdout/log text) whenever project access is unavailable, so a broken deployment silently reprocessing the whole backlog is visible to operators.

if [[ -n "${CATEGORY}" && "${CATEGORY}" != "null" ]]; then
PASSES_THRESHOLD=$(printf '%s >= %s\n' "${CONFIDENCE}" "${MIN_CONFIDENCE}" | bc -l 2>/dev/null || echo "0")
if [[ "${PASSES_THRESHOLD}" == "1" ]]; then
if [[ "${DRY_RUN}" == "true" ]]; then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Dry-run mode never exercises set_project_field's validation path, so a broken project/field config reports clean "would-set" results

In the per-issue loop, dry-run goes straight to ((CLASSIFIED++)); ISSUE_STATUS="classified"; CATEGORY_ACTION="would-set", while the live-run branch calls set_project_field, which contains ALL of the actual validation (PROJECT_ID/FIELD_ID presence check, and the category→option_id lookup against project-meta.json). So set_project_field is never invoked when CLASSIFY_DRY_RUN is true. A misconfigured CLASSIFY_PROJECT_NUMBER/CLASSIFY_FIELD_NAME, a project the token can't see, or a category name that doesn't match any project single-select option all report a clean "would-set" during dry-run preview, then silently produce error status the moment CLASSIFY_DRY_RUN flips to false on a live run. This defeats dry-run's documented purpose as a pre-flight config check (docs/gh-classify.md: "Dry-run mode previews decisions without writing to the project board"), and scripts/post-gh-classify-test.sh has no test exercising dry-run against an invalid/incomplete project-meta.json or an unmapped category name — all its dry-run tests use a fully valid project-meta.json.

Failure scenario: An operator sets CLASSIFY_PROJECT_NUMBER to the wrong project number and runs with CLASSIFY_DRY_RUN=true to preview. Every issue shows "would-set" with no warning. They flip to CLASSIFY_DRY_RUN=false expecting the same results, but every single classification now fails with "Failed to add or find #N on project" because PROJECT_ID never resolved — a misconfiguration invisible in preview.

Suggestion: In dry-run mode, still run the read-only parts of set_project_field (the PROJECT_ID/FIELD_ID presence check and the category→option_id lookup) so misconfiguration and unmapped category names surface as warnings during preview. Add a test fixture with an incomplete project-meta.json or an unknown category name run under CLASSIFY_DRY_RUN=true.

# project it returns the existing item. If the mutation fails for any
# reason, fall back to querying for the existing item.
local item_id
item_id=$(GH_TOKEN="${PROJECT_GH_TOKEN}" gh api graphql -f query='

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] GitHub API calls inside set_project_field have no timeout, unlike the pre-script's consistent 30s bound

Every gh api/gh api graphql call inside set_project_field() — the node_id lookup, the addProjectV2ItemById mutation (this line), the paginated item-search fallback, and the updateProjectV2ItemFieldValue mutation — runs with no timeout wrapper. By contrast, every equivalent call in scripts/pre-gh-classify.sh is consistently wrapped in timeout 30. Since set_project_field runs synchronously per classified issue inside the main loop (potentially many issues in all/unclassified mode), a single hung network call stalls the entire post-script until the harness-level timeout_minutes: 15 kills the whole job — discarding the Step Summary/report for every unprocessed issue, a much worse failure mode than the per-call bound already used elsewhere in this same PR.

Failure scenario: GitHub's GraphQL API experiences a slow response or hangs mid-request while processing issue #50 of 200 in all mode. With no per-call timeout, the post-script blocks indefinitely on that one call until the harness's 15-minute job timeout fires, killing the entire run — the 49 already-processed issues' results are still in memory but the report-write and Step Summary steps never execute because the script never reaches them.

Suggestion: Wrap the gh api / gh api graphql invocations in set_project_field with the same timeout 30 pattern used in pre-gh-classify.sh, so a single stuck call degrades to a per-issue error instead of consuming the whole run's time budget.

echo "| Issue | Category | Confidence | Action |"
echo "|------:|----------|:----------:|--------|"
jq -r '
.[] | select(.status == "classified") |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Out-of-candidate-set rejections (the prompt-injection/mode-drift defense) are invisible in the GitHub Step Summary

The host-side defense that rejects agent-returned issue_numbers outside the pre-script's authoritative candidate set works correctly and is recorded with status: "out-of-candidate" in classify-report.json, and is printed to stdout — but it never appears in the GITHUB_STEP_SUMMARY. The Step Summary's Metrics table has no "Outside candidates" row, and both of its jq-filtered detail tables (select(.status == "classified") here, and select(.status == "skipped") further down) exclude status == "out-of-candidate" rows entirely. A maintainer reviewing the run via the normal GitHub Actions Step Summary UI (as opposed to raw logs/artifacts) has no visibility that the agent attempted to act on an issue it was never authorized to touch — exactly the scenario this candidate-set check exists to catch.

Failure scenario: A prompt injection causes the agent to return a classification for an issue number outside the authoritative candidate set. The post-script correctly blocks the write, but a maintainer who only checks the GitHub Actions Step Summary UI (not raw logs or the JSON artifact) sees a clean run with no indication that a blocked injection/mode-drift attempt occurred.

Suggestion: Add an "Outside candidates" row to the Step Summary Metrics table, and/or a dedicated Step Summary table for status == "out-of-candidate" rows, so a blocked attempt surfaces where operators actually look.

echo "| Issue | Confidence | Reasoning |"
echo "|------:|:----------:|-----------|"
jq -r '
.[] | select(.status == "skipped") |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] "Skipped issues (below threshold)" Step Summary table conflates below-threshold, filter-mismatch, and unclassifiable results under one misleading heading

ISSUE_STATUS defaults to "skipped" and is only overridden to classified, error, or out-of-candidate; it is never overridden for the below-threshold, filter-mismatch, or unclassifiable (null-category) cases, even though CATEGORY_ACTION does distinguish them (below-threshold, filter-mismatch, unclassifiable). This table filters purely on status == "skipped", so it silently mixes in issues skipped due to a CLASSIFY_FILTER_CATEGORY mismatch or a confident null classification under a heading that specifically claims "below threshold" — misrepresenting agent behavior to anyone reading the summary. A confident null/filter-mismatch result looks identical in that table to a genuinely borderline low-confidence guess.

Failure scenario: A run with CLASSIFY_FILTER_CATEGORY="Bug fixes" set classifies several issues into other categories (filter-mismatch, high confidence) and a few issues as genuinely ambiguous (unclassifiable, low confidence). Both groups appear together in the Step Summary's "Skipped issues (below threshold)" table with no way to tell which is which from that view alone, even though the underlying report data distinguishes them.

Suggestion: Rename the table to something reason-agnostic (e.g. "Skipped issues") or split it by category_action (below-threshold, filter-mismatch, unclassifiable) the same way the stdout output already does with separate line arrays.


unclassified)
echo "Discovering unclassified issues via GitHub Project..."
PROJECT_NUMBER="${CLASSIFY_PROJECT_NUMBER:-1}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Hardcoded org-specific defaults (CLASSIFY_PROJECT_NUMBER=1, CLASSIFY_FIELD_NAME="Workstream Category") baked into a PR explicitly framed as a reusable, general-purpose agent

PROJECT_NUMBER="${CLASSIFY_PROJECT_NUMBER:-1}" / FIELD_NAME="${CLASSIFY_FIELD_NAME:-Workstream Category}" appear twice (here and again around lines 231-232) as defaults. The PR description explicitly frames docs/workstream-categories.md as org-specific and deliberately keeps it out of this shared repo ("Org-specific ... stay in .fullsend"), acknowledging the agent is meant to be reusable across organizations/installs — yet the project number and the literal field name "Workstream Category" are fullsend-ai's own board layout, hardcoded as defaults with no ADR/issue tying them to a cross-org convention other installs are expected to follow.

Failure scenario: A different org installs gh-classify without reading the fine print on CLASSIFY_PROJECT_NUMBER/CLASSIFY_FIELD_NAME, assumes the defaults are sensible general-purpose values (since they read like ordinary defaults rather than fullsend-ai-specific ones), and the agent silently targets project #1 with a field named "Workstream Category" that doesn't exist in their org's board — degrading to the same silent no-op-write failure mode as the permission-gap finding on harness/gh-classify.yaml:7, making the two indistinguishable to an operator.

Suggestion: Either make these required (no default) so every install must declare its own project number/field name, or clearly document in docs/gh-classify.md that these specific defaults reflect the fullsend-ai org's own board and other installs must override them.


# Normalize sentinel values used to satisfy fullsend runner_env validation.
# The runner rejects empty env vars, so the workflow provides non-empty
# sentinels for optional vars. Convert them back to empty here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] env.runner/env.sandbox sentinel-value contract depends entirely on a not-yet-written companion workflow, and is untested via the actual sentinel path

These lines normalize sentinel values (CLASSIFY_ISSUE_NUMBER == "0", CLASSIFY_FILTER_CATEGORY == "__all__", CLASSIFY_PROJECT_TOKEN == "__none__") with the comment "The runner rejects empty env vars, so the workflow provides non-empty sentinels for optional vars" — but harness/gh-classify.yaml's env.runner/env.sandbox blocks do plain ${VAR} interpolation with no :- default fallback, and the PR description explicitly defers .github/workflows/gh-classify.yml (the only place these sentinel values would actually be supplied) to a follow-up PR in .fullsend. scripts/post-gh-classify-test.sh contains no reference to 0, __all__, or __none__ as sentinel inputs — it only exercises the scripts with already-resolved real values, never through the sentinel-normalization path itself. So the correctness of this contract is entirely unverified within this PR and depends on a workflow file this PR does not include or test against.

Failure scenario: The follow-up .fullsend workflow PR uses a different placeholder (e.g. an empty string, or "none" instead of "none") for an optional var, or omits the sentinel for one field. Nothing in this repo's test suite or harness config would catch the mismatch — the pre/post scripts would treat the literal placeholder string as a real value (e.g. attempting to filter by category "all") with no test ever exercising that path.

Suggestion: Either add the :- defaults directly in harness/gh-classify.yaml so the sentinel contract is self-contained in this repo, or explicitly flag in the PR/docs that this harness cannot be safely registered until the companion .fullsend workflow (with matching sentinel values) lands, and add a test that runs the scripts through the sentinel values themselves (e.g. CLASSIFY_ISSUE_NUMBER=0, CLASSIFY_FILTER_CATEGORY=all) rather than only through already-resolved real values.

@ascerra

ascerra commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR as we have discussed that this gh-classify agent is not currently being used. We will have record of it in the git history if needed to be revived one day.

@ascerra ascerra closed this Aug 4, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:00 PM UTC · Completed 8:21 PM UTC
Commit: 7de8120 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #619feat(agents): add gh-classify agent

PR #619 was a substantial effort (1,686 lines, 13 new files) by ascerra to port the gh-classify agent from .fullsend into the shared agents repo. It went through 4 commits and 3 rounds of automated review before being closed without merging — the author determined the agent is not currently in use.

Workflow timeline

  1. Aug 3 11:17 — PR opened. Review dispatched (run 30808947370), then cancelled.
  2. Aug 3 11:21 — Author pushed pre-commit fixes. Review dispatched (run 30809197216), completed at 11:38 with CHANGES_REQUESTED — 11 findings (1 High, 1 Medium, 9 Low).
  3. Aug 3 11:44 — Author pushed fixes addressing most non-High findings. Review dispatched (run 30810739028), completed at 12:01 — still CHANGES_REQUESTED.
  4. Aug 3 12:20 — Human reviewer waynesun09 submitted first review pass: 9 findings including CRITICAL candidate-set validation gap and HIGH set -e dead-code bug.
  5. Aug 3 13:52 — Author pushed fixes for waynesun09's findings. Review dispatched (run 30820017877), completed at 14:09 — still CHANGES_REQUESTED.
  6. Aug 3 14:13waynesun09 submitted second review pass: 8 additional findings covering role-permission gaps, sandbox credential exposure, and missing timeouts.
  7. Aug 4 19:58 — Author closed the PR (agent not currently in use).

Review quality analysis

The automated review bot posted 11 inline findings. The human reviewer (waynesun09) independently posted 17 findings, including 3 CRITICAL and 3 HIGH issues the bot missed entirely.

What the bot caught: Predominantly syntactic, single-file issues — unused variables, naming inconsistencies, comment/filename mismatches, field ordering style, and one valid High finding (role-slug collision with the existing triage agent).

What the human caught that the bot missed:

  • CRITICAL: No host-side validation that agent-returned issue numbers match the authoritative candidate set (prompt injection surface). Required tracing data flow across 5 files.
  • CRITICAL: role: triage lacks organization_projects permission — every project-board write would 403. Required cross-repo lookup into fullsend's mintconfig.
  • CRITICAL: Sandbox receives a write-capable GH_TOKEN + unrestricted GitHub network access, contradicting the PR's documented security model.
  • HIGH: set -e makes the GraphQL error handler dead code (local rc=$? never executes after a failed command substitution). Required understanding bash execution semantics under set -euo pipefail.
  • HIGH: Sandbox GH_TOKEN permission failure silently degrades "unclassified" mode to "all" mode, causing the agent to overwrite existing project-board categories.

The gap was not about volume but about the class of reasoning: cross-file data flow tracing, cross-repo domain knowledge, bash execution semantics, and security architecture analysis vs. single-file syntactic pattern matching.

Evidence for existing open issues

  • Workflow trigger noise: 40 fullsend.yaml runs were triggered on this branch, including 25 pull_request_review events (15 cancelled) caused by the author replying individually to 20 bot inline comments. This provides additional evidence for fullsend-ai/fullsend issues #4984, #2994, #2991, and #1125. Notably, closed issue #2520 ("Filter COMMENTED pull_request_review events") was resolved in June but the problem persists, suggesting a regression or incomplete fix.
  • Cross-file reasoning: The bot missed 3 CRITICAL findings requiring multi-file data flow analysis, corroborating fullsend-ai/fullsend #1525 and fullsend-ai/agents #338.
  • Shell semantics: The bot missed the set -e dead-code pattern, corroborating fullsend-ai/agents #131 and #490.
  • Severity calibration: The bot's sole High finding (role-slug collision) was less impactful than 6 of the human's findings. This corroborates agents #545 and #412.
  • Low-value noise: ~45% of the bot's findings were cosmetic (header/filename mismatches, field ordering), consistent with agents #150.

Proposals filed

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

Labels

enhancement New feature or request needs-human Agent loop needs human intervention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants