Skip to content

feat(skills): add aicr-triage skill for project board triage - #1911

Merged
njhensley merged 1 commit into
NVIDIA:mainfrom
yuanchen8911:add-triage-skill
Jul 27, 2026
Merged

feat(skills): add aicr-triage skill for project board triage#1911
njhensley merged 1 commit into
NVIDIA:mainfrom
yuanchen8911:add-triage-skill

Conversation

@yuanchen8911

@yuanchen8911 yuanchen8911 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds aicr-triage as a shared agent skill under .agents/skills/ for triaging the NVIDIA AICR project board (project 248), and lists it in the Available Skills catalog in docs/contributor/skills.md.

Motivation / Context

Mark asked contributors to have their agents classify new issues on the project board. This skill makes that workflow available to every agent in the repo (Claude and Codex) via explicit invocation or context-triggered triage.

It is a port of the personal triage-issues skill. Earlier revisions of this PR grew to 680 lines chasing edge cases surfaced in review; the branch has been rewritten to a 384-line version that implements the same workflow with an explicit, narrow failure model.

Fixes: N/A
Related: N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Build/CI/tooling

Component(s) Affected

  • CLI (cmd/aicr, pkg/cli)
  • API server (cmd/aicrd, pkg/server)
  • Recipe engine / data (pkg/recipe)
  • Bundlers (pkg/bundler, pkg/component/*)
  • Collectors / snapshotter (pkg/collector, pkg/snapshotter)
  • Validator (pkg/validator)
  • Core libraries (pkg/errors, pkg/k8s)
  • Docs/examples (docs/, examples/)
  • Other: .agents/skills/ (agent skill definitions), docs/contributor/skills.md

Implementation Notes

Workflow. Resolve the project and its field/option IDs, pull the board, slice to active (non-Done) issues, classify each one, present the recommendations, confirm per bucket, apply, then re-pull and report.

A verdict writes every field whose correct value differs from the current one — the bucket names below label the shape of that difference for reporting and confirmation, they do not cap which fields are written. An issue at Ready + P2 that belongs at Backlog + P1 gets both edits under one Demote verdict.

Buckets, in precedence order — each issue gets one verdict. Manual Review outranks them all and is terminal: an item goes there whenever an exact, executable write cannot be named for it, and is never offered for confirmation or written.

  1. Close — superseded, duplicate, or fully captured by a child
  2. Incomplete — Status set without Priority or the reverse; fills the missing field and corrects the populated one when the Demote or Promote rules apply
  3. Demote Ready → Backlog — blocked, umbrella, or roadmap items
  4. Promote P2 → P1 — active blockers and in-review items shipping soon
  5. First-time classification — unclassified items get Status and Priority
  6. No change — reported only

Stalled is a flag rather than a bucket: an In progress item untouched for more than 30 days is surfaced in its own table. It neither creates nor suppresses a bucket verdict — a stalled item can still be a legitimate Close or Incomplete, and being listed as stalled does not withdraw it from selection; it is the Stalled table that carries no action, not the issues in it. Staleness comes from the issue's updatedAt, which is approximate — a bot or a triage comment resets it — and the skill says so.

Field and option IDs are resolved every run via gh project view and gh project field-list. Projects v2 regenerates single-select option IDs when an option is deleted and re-added, so hardcoded IDs go stale silently.

The board argument is carried, not re-defaulted. Step 1 resolves owner/num from the skill arg, and Step 2 repeats those resolved values rather than re-applying NVIDIA/248. Re-defaulting would make an alternate-board run analyze the AICR board, and since an approved closure uses the repository from that dump, it could close an issue on a board the user never asked to triage.

The named-issue path searches the full board dump, not the active slice, so an issue that is on the board but marked Done is reported as such rather than as missing, and it stops rather than silently reclassifying it. The lookup filters on content.type == "Issue" as well as number: issues and pull requests share one number sequence, the board holds both, and gh issue view accepts a PR number without complaint, so a number-only match could walk a pull request into issue classification.

Reads are batched per repository, not per issue. The board dump already carries each item's title, labels, and body; only open/closed state and updatedAt are missing, and one gh issue list call supplies both for every open issue in a repo. On the live board that is one call instead of 83, and it doubles as the closed-but-not-Done check. The snapshot filename is derived from the repo so a multi-repo board cannot overwrite itself, and a page at the --limit ceiling stops the run rather than reporting later open issues as closed. Full comment threads are still fetched per issue, but only for the verdicts that require them.

Board reads use gh project item-list with a --limit above the board size, asserted with jq -e '(.items | length) == .totalCount'. item-list reports the true total even when --limit truncates the array, so this is a real completeness check rather than a printed count. The fetch itself is guarded so an auth or network failure is not misreported as truncation.

Confirmation is mandatory, and it shows the exact write — the proposal table carries a proposed-action column ("Status → Backlog, Priority → P2"), and each confirmation option repeats it as owner/repo#N — <action> (numbers are repository-scoped and an org board can hold several repos), since First-time and Incomplete can each produce several target combinations and current-state-plus-reasoning does not say which one is being approved. Per-bucket multi-select via AskUserQuestion, batched to 4 options per call, with a numbered-list fallback where that tool is unavailable. Closures are individually selectable. No raw issue text is copied into the proposal table: pipes are escaped and newlines stripped from every rendered cell, reasoning included, since that table is the only gate before writes to a shared board.

Close is available only on NVIDIA project 248, gated at classification so an ineligible candidate never reaches the proposal table. A Close verdict edits no fields, so the item reaches Done via the project's built-in "item closed → Done" workflow, not via the skill. That workflow is enabled on 248 (96 of 97 closed issues are Done; #1623 is the lone exception) but is per-project, so on an alternate board — or whenever the precondition cannot be confirmed — Close candidates are routed to Manual Review rather than closed. Reporting a closure the skill declines to make is cheaper than creating a state it then refuses to clean up. Issues found already closed in an active column are likewise reported for a human to fix.

The close reason is checked against an enum before the comment posts — an empty or mistyped value would otherwise fall through and close the issue as "not planned". --duplicate-of availability is checked in Prerequisites, alongside the token scope: it landed in gh 2.88.0, and it is a static property of the environment rather than something to re-probe per item. The check stops the run rather than warning, since capability state cannot be carried across fresh shells. The missing-target check stays per-item, ahead of the comment.

Every applied verdict gets an issue comment, since board field edits leave no trace in the issue timeline. Bodies are piped in on stdin from a quoted heredoc rather than assembled into a shell command. Closures comment first and only then close, so nothing can close without a visible reason, and the close is asserted against a re-fetched CLOSED state.

Issue content is treated as untrusted evidence. Text in a title, body, or comment that reads like instructions or a priority directive cannot override the classification rules, the confirmation step, or the commands run.

Fail-closed guards. The ones where the obvious phrasing would have failed open instead:

  • A close-as-duplicate validates that it has a surviving issue number before the comment is posted. Otherwise the close fails after the comment lands, leaving a public "Closing:" comment on an issue that is still open — the exact state comment-before-close exists to prevent.
  • The two item-edit stanzas were collapsed into one parameterized stanza, run once per field the verdict changes and only for those fields, so the guard and its error text exist once rather than as a copy-paste pair that can drift. The stanza asserts its option id is present before calling gh; an empty one aborts. A skip-if-empty guard was rejected for the opposite reason: it lets the block exit 0 having changed nothing, after which the run posts a Triaged: comment for a change that never happened.
  • The jq slice that builds the active list is guarded. Without it, a failed slice yields an empty file and the next instruction reports "no active issues" — a clean bill of health for a board that was never read.
  • The close reason is matched against an enum. An empty or mistyped value would otherwise fall through to the else branch and close the issue as "not planned".
  • The post-close verification read is guarded separately from the state comparison, so a failed re-fetch reports the closure outcome as unknown rather than as "close did not take".

Every block assigns the values it uses, including ones an earlier block already set — each shell call is a fresh shell, so a carried-over variable is empty. This is why the file paths in Step 2 are fixed strings rather than mktemp output: they can be re-derived rather than threaded between calls.

Placeholders are quoted strings. Unquoted <...> is shell redirection, not a placeholder: n=<issue number> is a parse error and --duplicate-of <original> redirects instead of passing an argument.

Failure model, stated plainly

Concurrency and connectivity are out of scope. There is no locking, no pre-write re-validation, no retry, and no reconstruction of ambiguous outcomes.

  • Any nonzero gh exit in the apply step stops the run, and the report separates three states: applied, outcome unknown (a timeout can land after GitHub accepted the write), and not attempted. Closures are verified in the full board dump, since the active slice drops Done items and a successful closure would vanish from it. Read-phase failures differ by scope: a failed per-issue comment fetch routes that item to Manual Review and triage continues, while a failed repository-level state fetch stops the run, since without it no item's state is known.
  • A concurrent edit made between the board read and the write is silently overwritten.
  • The Incomplete bucket repairs a one-field partial write left by an aborted run. It does not repair an overwritten decision.

The invariant is narrow, and stated as narrowly as it holds: an observed nonzero exit stops every subsequent write, and a closure is only reached after its comment posted successfully. That is all. It does not claim every resulting board state is repairable — a concurrent overwrite is not, which is why it is listed above as a consequence rather than a handled case.

Deliberate deviations from triage-issues

No-change ACK comments are dropped. The source skill comments on every triaged issue including no-change verdicts. Applied changes and closures still get comments; issues with no state change appear in the triage report only. This avoids a run producing dozens of comments and removes the timestamp-marker machinery that made stalled detection disproportionately complex. Flagging it here because it was requested directly in review — say so if ACKs are a required workflow behavior rather than a preference carried over from the original.

The project-URL argument is dropped. owner/number is accepted; no URL parsing is documented.

Testing

make qualify

Two Markdown files, so make qualify is the repository gate rather than functional verification of the skill. The behavior was checked directly against the live board, read-only:

  • gh project item-list 248 --owner NVIDIA --limit 400 returns 205/205 items; the jq -e completeness assertion exits 0 there and exits 1 at --limit 5, confirming it detects truncation rather than reporting a count
  • The active slice returns 83 non-Done issues, and each entry carries the PVTI_* item id, status, priority, labels, and content.{number,repository,title,body} the classification steps rely on
  • The named-issue jq filter selects on type and number: a Done issue is returned so its state can be reported (not filtered out), a board pull request with the same number is excluded, and an absent number returns empty
  • The duplicate feature-detect, and the separated close-verification guard, were each exercised: a failed verification read now reports the closure outcome as unknown rather than as "close did not take"
  • The quoted-heredoc comment form was checked in both directions: the body passes through intact, and the || guard fires on a nonzero exit
  • Every bash block was extracted and run through bash -n and ShellCheck (-s bash); all 9 parse and are clean at warning level. bash -n caught three blocks that did not parse: unquoted <...> placeholders are shell redirection, so n=<issue number> and --duplicate-of <original> were parse errors. All placeholders are now quoted strings.
  • The fail-closed guards were exercised directly: an empty option aborts with exit 1 rather than editing or silently skipping; a duplicate close with no surviving issue number aborts before the comment is posted, and a populated one proceeds; and the close-reason enum accepts duplicate and not planned while rejecting an empty or mistyped value.

No mutating command was run against the board.

make qualify passes: unit tests with -race, the coverage gate, shell tests, e2e, go vet, and lint-go. Two spurious failures were ruled out along the way and are worth recording, since neither is caused by this change and both are easy to mis-read as one:

  • A sandboxed run died on mktemp: Operation not permitted, taking 6 shell tests with it. The same tests pass outside the sandbox.
  • lint-go reported 109 golangci-lint issues whose file paths all pointed at other local worktrees (/private/tmp/aicr/review-b4b39d10d/...), i.e. a poisoned shared GOCACHE. Re-running with GOCACHE and GOLANGCI_LINT_CACHE set to fresh directories gives 0 issues. This PR contains no Go files, so it cannot produce a type-based lint finding.

Risk Assessment

  • Low — Isolated change, well-tested, easy to revert
  • Medium — Touches multiple components or has broader impact
  • High — Breaking change, affects critical paths, or complex rollout

Rollout notes: Adds two Markdown files and no code. The skill mutates a shared board and closes issues, but only after explicit per-bucket confirmation, so the blast radius is bounded by what a reviewer approves in the moment.

Checklist

  • Tests pass locally (make test with -race)
  • Linter passes (make lint) — 0 issues with isolated caches; see Testing
  • I did not skip/disable tests to make CI green
  • I added/updated tests for new functionality
  • I updated docs if user-facing behavior changed
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S) — GPG signing info

@yuanchen8911
yuanchen8911 requested a review from a team as a code owner July 24, 2026 18:08
@yuanchen8911 yuanchen8911 added the theme/ci-dx CI pipelines, developer experience, and build tooling label Jul 24, 2026
@yuanchen8911
yuanchen8911 requested a review from mchmarny July 24, 2026 18:12
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

Adds the aicr-triage skill with instructions for project discovery, issue classification, confirmation, sequential updates, closure handling, verification, and reporting. The contributor skills documentation now lists the skill and provides invocation examples.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: mchmarny

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: adding the aicr-triage skill for project board triage.
Description check ✅ Passed The description is directly related to the skill addition and docs update, matching the changeset.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/aicr-triage/skill.md:
- Around line 1-12: Rename the skill file from skill.md to SKILL.md so it
matches the repository’s required skill filename and remains discoverable on
case-sensitive filesystems.
- Around line 99-103: Update the issue lookup command in the skill instructions
to use the board item’s repository via content.repository.nameWithOwner instead
of the hardcoded NVIDIA/aicr value, while continuing to use content.number for
the issue number.
- Around line 4-11: The advertised behavior in the skill description does not
match the implemented workflow. Update the `/aicr-triage` skill by either adding
discovery and apply paths for P2-to-P1 promotion, Ready-to-Backlog demotion, and
stale-issue closure—including their priority option IDs—or narrowing the
description to the currently supported unclassified-item flow and its fixed P2
update behavior.
- Around line 33-36: Update the specific-issue triage path in the skill
instructions to look up the project item for the named repository issue before
classification. Resolve both the project item ID and its current Status by
querying the project using the repository and issue number, then pass those
values into Step 5 so the requested board edits can be applied.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 9c0784ce-6820-416b-a15f-d44df5fec1fa

📥 Commits

Reviewing files that changed from the base of the PR and between 41a80bf and ac002f5.

📒 Files selected for processing (1)
  • .agents/skills/aicr-triage/skill.md

Comment thread .agents/skills/aicr-triage/skill.md Outdated
Comment thread .agents/skills/aicr-triage/skill.md Outdated
Comment thread .agents/skills/aicr-triage/skill.md Outdated
mchmarny
mchmarny previously approved these changes Jul 24, 2026
@yuanchen8911
yuanchen8911 marked this pull request as draft July 24, 2026 18:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/aicr-triage/skill.md:
- Around line 95-102: The named-issue path must explicitly handle both existing
and missing project items: state whether an item with an existing Status is
reclassified, and when no matching board item exists, report “issue is not on
project” rather than “No unclassified issues found.” Update the surrounding Step
2 guidance to cover named issues separately from the unclassified-item scan,
using the existing item id, Status, and issue details.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 346d3986-0859-4cb4-a602-7ff410eadbeb

📥 Commits

Reviewing files that changed from the base of the PR and between ac002f5 and b2a8e1c.

📒 Files selected for processing (1)
  • .agents/skills/aicr-triage/skill.md

Comment thread .agents/skills/aicr-triage/SKILL.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/aicr-triage/SKILL.md:
- Around line 144-149: Update the triage workflow around the Status and Priority
gh project item-edit operations to stop immediately when either edit fails,
record which field was successfully updated, and avoid reporting the item as
completed. Before including an item in the final summary, verify that both
Status and Priority match the intended values; otherwise report the partial or
failed state explicitly.
- Around line 27-29: Update the priority guidance table in SKILL.md to define
distinct criteria for selecting P0 versus P1 before referencing IDs 79628723 and
0a877460. Specify the escalation threshold and impact characteristics for each
priority, while preserving the existing P2 guidance and identifiers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 87dc42d8-ec30-4b8a-9d01-01004ce513f7

📥 Commits

Reviewing files that changed from the base of the PR and between b2a8e1c and e51031a.

📒 Files selected for processing (1)
  • .agents/skills/aicr-triage/SKILL.md

Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/aicr-triage/SKILL.md:
- Around line 94-98: Update the specific-issue board query and the
existing-classification branch in the issue triage workflow to retrieve and
display both Status and Priority before asking whether to reclassify. Preserve
the no-board-item and unclassified-item behavior, and use the fetched values
when showing the current classification.
- Around line 94-98: The named-issue handling in the triage workflow must run
before the empty unclassified-items early return. Update the surrounding flow in
SKILL.md so named invocations perform the board lookup and handle missing or
already-classified items, while the “No unclassified issues found” stop applies
only when no issue number was provided.
- Line 94: Update the Step 1 GraphQL lookup for a specified issue number to also
match content.repository.nameWithOwner against the target repository, rather
than filtering only by content.number. If the repository cannot be determined or
multiple items remain, reject the request as ambiguous before proceeding to Step
5, while preserving retrieval of the board item id and current Status.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 7cf82e82-3d3d-45eb-9f2b-474eae65c08a

📥 Commits

Reviewing files that changed from the base of the PR and between e51031a and b5165d5.

📒 Files selected for processing (1)
  • .agents/skills/aicr-triage/SKILL.md

Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated

@ArangoGutierrez ArangoGutierrez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found 1 issue.

  1. Skill placed in .agents/skills/aicr-triage/SKILL.md instead of .claude/skills/aicr-triage/SKILL.md. All 6 existing project skills live under .claude/skills/, and that is the directory Claude Code scans for skill discovery. The .agents/skills/ path does not exist in the repo, so this skill will never auto-load. (.agents/skills/aicr-triage/SKILL.md:1)

Comment thread .agents/skills/aicr-triage/SKILL.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/aicr-triage/SKILL.md:
- Around line 65-67: Update the named-issue classification path to read and
display the fetched priority alongside the existing status. Thread priority.name
through the named-item data handling and include it in the proposal table before
reclassification confirmation, preserving the current status behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a429a8b1-4b5b-4f12-b295-737c1dae7be1

📥 Commits

Reviewing files that changed from the base of the PR and between b5165d5 and ac7ce55.

📒 Files selected for processing (2)
  • .agents/skills/aicr-triage/SKILL.md
  • docs/contributor/skills.md

Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
@yuanchen8911
yuanchen8911 marked this pull request as ready for review July 24, 2026 19:45
@yuanchen8911
yuanchen8911 requested a review from mchmarny July 24, 2026 19:45
@github-actions github-actions Bot added size/XL and removed size/M labels Jul 24, 2026

@mchmarny mchmarny 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.

Solid port — but worth noting up front that this is a port of an existing personal skill (~/.claude/skills/triage-issues/), and the port both gained and lost behaviors relative to its source.

Gained (real improvements, keep these): GraphQL pagination past the 200-item gh project item-list cap; set -o pipefail + empty-stdin rejection; GraphQL errors-checked-before-data; prompt-injection scoping of issue content; named-single-issue targeting; the project OAuth-scope prerequisite; an explicit P0 definition.

Lost — the three comments below are each a behavior the source skill has that this version drops, not new suggestions:

  1. Source: "IDs differ per project — re-fetch every run, never hardcode."
  2. Source Step 7: every triaged issue gets a gh issue comment, including no-change verdicts.
  3. Source Step 4: three buckets (promote P2→P1, demote Ready→Backlog, close), not just first-time classification.

A fourth minor one: the source confirms via AskUserQuestion with one multi-select per bucket so the user can accept a subset; this version's plain-text "Proceed?" is all-or-nothing. Fine if AskUserQuestion isn't available on the Codex path — worth saying so explicitly.

Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
- If the user requests changes to specific rows: update those rows, re-show the table, and ask again.
- If the user says no: stop without making any changes.

### 5. Apply classification

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.

Board field edits don't appear in the issue timeline, so an assignee or watcher has no signal that triage happened. The source skill mandates a comment on every triaged issue, explicitly including no-change verdicts, for exactly this reason.

Suggest adding to this step:

gh issue comment "$n" -R "$repo" \
  --body "Triaged: set to <Status> / <Priority>. <1-2 sentences of reasoning>"

…plus a no-change variant for the reclassify path in Step 1, where the verdict is "current placement is correct". Skip only when a triage comment from the same session already exists on the issue.

@yuanchen8911 yuanchen8911 Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Superseded by 94e1c25. The branch was rewritten and this reply no longer described it: there is no timestamp marker and no no-change ACK comment. Applied field changes and closures still get an issue comment. See the reply below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mark, this revision intentionally omits no-change ACK comments. Applied field changes and closures still receive issue comments, while correctly placed issues appear only in the triage report.

This avoids dozens of low-signal comments and removes the timestamp-marker machinery needed because comments reset updatedAt. If ACKs are a required workflow behavior, I’ll restore them; otherwise I’d like to resolve this as an intentional simplification.

Comment thread .agents/skills/aicr-triage/SKILL.md Outdated

@ArangoGutierrez ArangoGutierrez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Second pass, six findings. The Step 1 Python ones were reproduced by extracting the snippet from the file and running it against fixtures.

  1. Project 248 is an org-level board, so matching on content.number alone can select items from different repositories: with the same number in two repos this returns both, while the branch below only handles zero items or "the single returned item". The query already carries repository.nameWithOwner for Step 2, so filtering on it here too would disambiguate. (.agents/skills/aicr-triage/SKILL.md:116)
  2. Step 1's GraphQL board read also requires read:project, so a default gh auth login token (repo, read:org, gist) fails on the very first command rather than at Step 5. Moving this check ahead of Step 1 would match where it actually bites. (.agents/skills/aicr-triage/SKILL.md:33)
  3. Bulk selection keys only on Status, so an item left with Status set and Priority empty is never re-selected and the run reports "No unclassified issues found". That is exactly the state Step 5's partial-failure stop can leave behind, so the recovery path cannot see its own failure mode. Selecting on missing Status or missing Priority would let a retry pick it up. (.agents/skills/aicr-triage/SKILL.md:120)
  4. .get('data', {}) returns None when the key is present with a JSON null, which is how GraphQL represents an unresolvable object, so this raises AttributeError instead of printing the intended "unexpected GraphQL response" diagnostic just below. The (x or {}) idiom already used at line 116 handles that shape. (.agents/skills/aicr-triage/SKILL.md:107)
  5. export TARGET_ISSUE=123 and unset TARGET_ISSUE are both live commands in this one block, so running it as written always ends in bulk mode and the named-issue path never triggers. Commenting out one of the two branches would keep both usable. (.agents/skills/aicr-triage/SKILL.md:58)
  6. Issue titles are rendered into this table verbatim, and a title containing a pipe or a newline can reflow the rows the user is approving. Since this table is the only gate before writes to a shared board, escaping | and stripping newlines when building it would keep the confirmation trustworthy. (.agents/skills/aicr-triage/SKILL.md:164)

Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
Comment thread .agents/skills/aicr-triage/SKILL.md Outdated
@yuanchen8911

Copy link
Copy Markdown
Contributor Author

Thanks — after reconciling the provenance, I had initially treated aicr-triage as an intentionally narrower workflow, but the intended source is triage-issues. The missing behaviors are porting gaps rather than separate scope.

Will update this PR into a faithful shared-agent port:

  1. Resolve project, field, and option IDs dynamically with gh project view and gh project field-list.
  2. Restore the full active-board workflow: promote P2→P1, demote Ready→Backlog, and close superseded, duplicate, or fully captured issues. Stalled items will be surfaced for review rather than automatically closed.
  3. Record every approved triage verdict on the issue, including confirmed no-change ACKs.
  4. Use structured per-bucket selection when the host provides an interactive selection tool, with an explicit numbered-subset fallback on other agent paths.

Will retain the improvements added during this PR: pagination beyond 200 items, pipefail and fail-closed GraphQL parsing, prompt-injection boundaries, named-issue targeting, first-time classification, partial-update handling, and post-write verification.

@github-actions github-actions Bot added size/L and removed size/M labels Jul 24, 2026
@yuanchen8911 yuanchen8911 changed the title feat(skills): add aicr-triage skill for issue board classification feat(skills): add aicr-triage skill for project board triage Jul 24, 2026
@yuanchen8911
yuanchen8911 force-pushed the add-triage-skill branch 12 times, most recently from cb4bfca to afaa9e0 Compare July 25, 2026 03:08
@yuanchen8911
yuanchen8911 marked this pull request as draft July 25, 2026 03:16
@yuanchen8911 yuanchen8911 changed the title feat(skills): add aicr-triage skill for project board triage WIP: feat(skills): add aicr-triage skill for project board triage Jul 25, 2026
@yuanchen8911
yuanchen8911 force-pushed the add-triage-skill branch 2 times, most recently from 2001666 to 620c224 Compare July 26, 2026 19:37
@github-actions github-actions Bot added size/L and removed size/XL labels Jul 26, 2026
@yuanchen8911

yuanchen8911 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Major revision — the skill was rewritten around simplification. Merged as ff0301b53.

@mchmarny @ArangoGutierrez, thanks for the reviews.

@yuanchen8911

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/aicr-triage/SKILL.md:
- Around line 72-75: Update the temporary-file setup near owner and num in
.agents/skills/aicr-triage/SKILL.md:72-75 to create a unique per-run directory
with restrictive permissions, then derive ITEMS and ACTIVE paths within it
instead of using fixed filenames. Update the repository snapshot paths in
.agents/skills/aicr-triage/SKILL.md:136-140 to use that same directory and
remove it after verification completes.
- Around line 368-373: Update the Step 2 “no active issues” early-return
guidance so it applies only to initial discovery, not the Step 8 verification
flow. Ensure Step 8 always checks changed closures in the full `$ITEMS` dump,
even when `$ACTIVE` is empty, and confirms their Status is Done or reports
Manual Review.
- Around line 285-287: Update the workflow’s approved-item application step to
re-fetch each item’s current Status and Priority immediately before editing,
then compare them with the confirmation-table values. Route any item whose
values changed to Manual Review and skip its write; apply edits only when both
values still match, while preserving the existing reporting behavior.
- Around line 51-66: Update the metadata reads in the project setup flow around
gh project view and gh project field-list to fail closed, using strict command
failure handling so either command aborts the run immediately. Ensure project,
field, and option IDs are captured or reused only after both reads succeed,
while preserving the existing missing-metadata validation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: d981fa61-fc59-47bb-98c4-fb037624543e

📥 Commits

Reviewing files that changed from the base of the PR and between 7e0f4a1 and 94e1c25.

📒 Files selected for processing (2)
  • .agents/skills/aicr-triage/SKILL.md
  • docs/contributor/skills.md

Comment thread .agents/skills/aicr-triage/SKILL.md
Comment thread .agents/skills/aicr-triage/SKILL.md
Comment thread .agents/skills/aicr-triage/SKILL.md
Comment thread .agents/skills/aicr-triage/SKILL.md
@yuanchen8911

Copy link
Copy Markdown
Contributor Author

Related: #1920 tracks a gap found while reviewing this PR — the repo defines "stale" three ways (stale.yaml at 90/120, CONTRIBUTING.md at 60/74, and this skill's 30-day informational "Stalled" table), and the skill's Close bucket does not consult the lifecycle/frozen / priority/critical labels that exist to prevent closure.

The docs half is fixed in #1921. The skill half is deliberately not in scope here — it would add behavior to a change scoped to simplification — and is left for a follow-up once this merges.

Adds a shared agent skill for triaging the NVIDIA AICR project board
(project 248): classify new issues, promote P2 to P1, demote Ready to
Backlog, and propose closures, applying only user-confirmed changes.

Ported from the personal triage-issues skill, keeping its structure and
adding first-time and incomplete classification buckets. A verdict writes
every field whose correct value differs from the current one; the bucket
name labels that difference rather than capping it.

Concurrency and connectivity failures are out of scope. The invariant is
narrow: a nonzero gh exit in the apply step stops every subsequent write,
and a closure is only reached after its comment posted successfully. Read
failures differ by scope -- a failed per-issue comment fetch routes that
item to Manual Review and triage continues, while a failed repository-level
state fetch stops the run, since without it no item's state is known. A
one-field partial write from an aborted run is repaired by the
incomplete-classification bucket next run; a concurrent overwrite is not.

Signed-off-by: Yuan Chen <yuanchen97@gmail.com>

@ArangoGutierrez ArangoGutierrez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Careful skill — the failure contract in Step 7, the quoted heredocs, and the comment-before-close ordering are all the right calls. Four things, the first one worth a look.

  1. Step 6 asks the user to approve the exact field write (owner/repo#N — Status → Backlog, Priority → P2), but Step 7 then posts a public comment under their identity whose body the agent authors at apply time and never showed them. The skill's invariant is scoped to fields and closures — "no field is edited and no issue is closed without this confirmation" — so the comment slips through it. For a skill whose whole premise is confirm-before-mutate, showing the comment body in the Step 5 table (or including it in the Step 6 option text) would close the gap. (.agents/skills/aicr-triage/SKILL.md:314)
  2. gh auth status is run without an assertion, and the guidance "needs 'project' in Token scopes" is satisfied textually by the read-only read:project scope. Steps 1-2 read fine under read:project, so the run gets all the way through classification and user confirmation before gh project item-edit fails on the first write — landing in the "outcome unknown" state Step 7 works hard to avoid. Grepping for the exact project scope token would fail it up front. (.agents/skills/aicr-triage/SKILL.md:29)
  3. Step 8 re-runs the Step 2 fetch right after Step 7 and requires a closed issue's Status to already read Done, but Step 4 notes that transition comes from the board's built-in "item closed → Status: Done" workflow, which is asynchronous. A closure that actually succeeded can therefore show up under Manual Review on a fast re-fetch. Re-reading just the closed items after a short delay, or treating not-yet-Done as pending rather than manual, would avoid the false positive. (.agents/skills/aicr-triage/SKILL.md:374)
  4. $ITEMS and $ACTIVE are fixed basenames carrying no board identity even though owner/num are run parameters, and the "One named issue" block re-reads $ITEMS in a fresh shell without re-fetching it. $TMPDIR also resolves differently sandboxed vs unsandboxed, so a dump left by an earlier run against a different board can be picked up silently. Folding $owner/$num into the filename the way the per-repo $OPEN path already does would make a stale read impossible. (.agents/skills/aicr-triage/SKILL.md:75)

|| { echo "$label edit failed for #$n — outcome unknown; stop and report"; exit 1; }
```

**Comment on every applied verdict** — board field edits leave no trace in the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Step 6 asks the user to approve the exact field write (owner/repo#N — Status → Backlog, Priority → P2), but Step 7 then posts a public comment under their identity whose body the agent authors at apply time and never showed them. The skill's invariant is scoped to fields and closures — "no field is edited and no issue is closed without this confirmation" — so the comment slips through it. For a skill whose whole premise is confirm-before-mutate, showing the comment body in the Step 5 table (or including it in the Step 6 option text) would close the gap.

OAuth scope. Check first — otherwise the very first command fails:

```bash
gh auth status # needs "project" in Token scopes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

gh auth status is run without an assertion, and the guidance "needs 'project' in Token scopes" is satisfied textually by the read-only read:project scope. Steps 1-2 read fine under read:project, so the run gets all the way through classification and user confirmation before gh project item-edit fails on the first write — landing in the "outcome unknown" state Step 7 works hard to avoid. Grepping for the exact project scope token would fail it up front.


### 8. Verify and report

Re-run the Step 2 fetch and confirm the outcome for every changed item. Field

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Step 8 re-runs the Step 2 fetch right after Step 7 and requires a closed issue's Status to already read Done, but Step 4 notes that transition comes from the board's built-in "item closed → Status: Done" workflow, which is asynchronous. A closure that actually succeeded can therefore show up under Manual Review on a fast re-fetch. Re-reading just the closed items after a short delay, or treating not-yet-Done as pending rather than manual, would avoid the false positive.

```bash
# From Step 1, NOT the defaults: re-applying them would triage the wrong board.
owner="<owner>"; num="<project-number>"
ITEMS="${TMPDIR:-/tmp}/aicr-triage-items.json"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$ITEMS and $ACTIVE are fixed basenames carrying no board identity even though owner/num are run parameters, and the "One named issue" block re-reads $ITEMS in a fresh shell without re-fetching it. $TMPDIR also resolves differently sandboxed vs unsandboxed, so a dump left by an earlier run against a different board can be picked up silently. Folding $owner/$num into the filename the way the per-repo $OPEN path already does would make a stale read impossible.

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

Labels

area/docs size/L theme/ci-dx CI pipelines, developer experience, and build tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants