fix(skill): ban untracked deferrals in /review skill#568
fix(skill): ban untracked deferrals in /review skill#568carlos-alm wants to merge 42 commits intomainfrom
Conversation
…housekeep Four recurring maintenance routines as Claude Code skills: - /deps-audit: vulnerability scanning, staleness, unused deps, license checks - /bench-check: benchmark regression detection against saved baselines - /test-health: flaky test detection, dead tests, coverage gap analysis - /housekeep: clean worktrees, dirt files, sync main, prune branches
…line - Replace 2>/dev/null with output=$(... 2>&1) + exit_code check on all four benchmark invocations so error messages are captured and recorded - Add division-by-zero guard in Phase 3: when baseline == 0, mark delta as "N/A — baseline was zero" (informational only, not a regression) - Add git add + git commit step in Phase 5 so the baseline file is actually committed after each save, matching the documented rule
- After reverting package.json + package-lock.json on --fix test failure, also run `npm ci` to resync node_modules/ with the restored lock file; without this the manifest is reverted but installed packages are not - Add explanatory comment on @anthropic-ai/tokenizer skip-list entry clarifying it is a peer dependency of @anthropic-ai/sdk and may be required at runtime without an explicit import in our code
…erion - Phase 5 (Update Codegraph): add source-repo guard that skips the self-update logic when running inside the codegraph source repo; comparing the dev version to the published release and running npm install is a no-op since codegraph is not one of its own deps - Phase 1b stale-worktree criterion: replace "created more than 7 days ago" (not determinable via git worktree list) with "last commit on the branch is more than 7 days old AND branch has no commits ahead of origin/main", using `git log -1 --format=%ci <branch>`
…at/maintenance-skills
When Phase 0 stash push is a no-op (manifests unchanged), Phase 7 was calling stash drop/pop on the wrong entry. Track STASH_CREATED exit code and branch on it: use git checkout when no stash exists.
…ent corruption Replace hardcoded /tmp/test-health-runs/ with mktemp -d so parallel sessions get isolated directories. Add cleanup at end of analysis.
…plicit commit paths Add 4th verdict path for --save-baseline when baseline already exists. Replace corrupted em-dash character in N/A string. Change commit command to use explicit file paths per project convention.
…ress Phase 5 was listed as "2 of 7 complete" with outdated pre-Phase 3 file paths. Updated to reflect actual state: 32 of 269 source modules migrated (~12%). Steps 5.3-5.5 now list exact migrated/remaining files with verified counts (5.3=8, 5.4=54, 5.5=175, total=237 JS-only files). Added note about 14 stale .js counterparts of already-migrated .ts files needing deletion.
When STASH_CREATED=1 and tests pass, the npm audit fix changes are good — no action needed. Previously it ran git checkout to discard them, which undid the successful fix.
git diff --quiet ignores untracked files, so on the first run when baseline.json and history.ndjson are newly created, the commit was skipped. Stage first with git add, then check with --cached.
Branch deletion now asks for user confirmation before each delete, consistent with worktree removal in Phase 1c.
- bench-check: add timeout 300 wrappers to all 4 benchmark invocations with exit code 124 check for timeout detection - bench-check: add explicit COMPARE_ONLY guard at Phase 5 entry - housekeep: fix grep portability — use grep -cE instead of GNU \| syntax - test-health: add timeout 180 wrapper in flaky detection loop - test-health: fix find command -o precedence with grouping parentheses
When both stat variants (GNU and BSD) fail, $size is empty and the arithmetic comparison errors out. Add a [ -z "$size" ] && continue guard so the loop skips files whose size cannot be determined.
…check (#565) Phase 6: when SAVE_ONLY or first-run (no prior baseline), write a shortened report with "Verdict: BASELINE SAVED" instead of the full comparison report. Phases 1a-1d: replace ambiguous "If timeout / If non-zero" with explicit "If timeout / Else if non-zero" so the two conditions are clearly mutually exclusive.
Phase 4 (Resolution Accuracy) had all 6 sub-phases merged but status still said "In Progress". Phase 5 (TypeScript Migration) had 5.3-5.5 merged via PRs #553, #554, #555, #566 but was listed with stale counts. Updated both to reflect actual state: Phase 4 complete, Phase 5 at 5/7 with 76 of 283 modules migrated (~27%).
Previous commit incorrectly marked 5.3-5.5 as complete. In reality 76 of 283 src files are .ts (~27%) while 207 remain .js (~73%). PRs #553, #554, #555, #566 migrated a first wave but left substantial work in each step: 4 leaf files, 39 core files, 159 orchestration files. Updated each step with accurate migrated/remaining counts.
The /review skill allowed replying "acknowledged as follow-up" to reviewer comments without tracking them anywhere. These deferrals get lost — nobody revisits PR comment threads after merge. Now: if a fix is genuinely out of scope, the skill must create a GitHub issue with the follow-up label before replying. The reply must include the issue link. A matching rule in the Rules section reinforces the ban.
|
Claude finished @carlos-alm's task —— View job PR Review CompleteReviewing PR #568: fix(skill): ban untracked deferrals in /review skill Todo List
Root Cause AnalysisProblem Being Solved: The Why This Approach: Excellent approach. Instead of hiding the symptom, this PR prevents the root cause by:
Risk Assessment: Very low risk. This is pure documentation/process improvement with no code changes. Configuration Change Detection (Phase 2) ✅No configuration changes detected. This PR only modifies skill documentation. Test Integrity (Phase 3) ✅No test modifications. The changes are documentation-only, focused on process improvement. Code Quality (Phase 4) ✅Documentation Quality: High quality changes:
Specific improvements at
Backlog Compliance (Phase 5)Zero-dep: ✅ — No new runtime dependencies added Foundation-aligned: ✅ — Fully aligned with all principles:
Problem-fit: 4/5 — Indirectly helps AI agents by ensuring reviewer feedback gets properly tracked and addressed. When review comments are systematically captured as issues, future AI assistance can find and reference them instead of re-discovering the same problems. Breaking: No — Pure documentation addition, no API or behavior changes Tier: 1 (zero-dep ✓ + foundation-aligned ✓) Critical ConcernsNone. This is a well-executed process improvement that directly addresses a real tracking problem. Additional NotesPositive aspects:
Minor observation: The 4 new skill files (bench-check, deps-audit, housekeep, test-health) appear to be part of broader skill documentation efforts rather than core to this specific fix, but they're valuable additions for skill completeness. Final Recommendation
The only reason this isn't 5 stars is that it's primarily a process fix rather than a functional enhancement, but it's exactly the right solution for the problem being addressed. |
Greptile SummaryThis PR closes the "silent deferral" gap exposed on PR #558, where the Key changes:
One gap found: The new step 2i subagent result format has no Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
S0["Step 0\n/worktree (main session)"] --> S1["Step 1\nDiscover open PRs\ngh pr list"]
S1 --> S2["Step 2\nLaunch one subagent per PR\n(all in parallel, isolation: worktree)"]
S2 --> A1["Subagent A\nPR #N"]
S2 --> A2["Subagent B\nPR #M"]
S2 --> A3["Subagent …\nPR #…"]
A1 --> PA["2a. gh pr checkout\n2b. Resolve conflicts\n2c. Check CI\n2d. Gather comments"]
A2 --> PA
A3 --> PA
PA --> PE{"2e. Each comment"}
PE -->|"Fix possible"| FIX["Make the fix\n→ reply 'Fixed'"]
PE -->|"Genuinely out of scope"| DEFER["gh label create follow-up\ngh issue create --label follow-up\ncapture issue_url → issue_number\nreply 'tracked in #issue_number'"]
PE -->|"Disagree"| DISAGREE["Do NOT fix silently\n→ reply explaining why"]
FIX --> PF["2f. Commit & push fixes"]
DEFER --> PF
DISAGREE --> PF
PF --> PG["2g. Re-trigger reviewers\n(Greptile + Claude if applicable)"]
PG --> PH{"2h. New comments?\n(max 3 rounds)"}
PH -->|"Yes"| PE
PH -->|"No / limit reached"| PI["2i. Return structured result\nConflicts · CI · Comments Addressed\n⚠️ Issues Created not in schema"]
PI --> S3["Step 3 (main agent)\nCollect all results\nOutput summary table"]
Reviews (7): Last reviewed commit: "fix(skill): capture gh issue create outp..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
.claude/skills/review/SKILL.md
Outdated
| gh issue create \ | ||
| --title "follow-up: <concise description of what needs to be done>" \ | ||
| --body "$(cat <<'EOF' | ||
| Deferred from PR #<number> review. | ||
|
|
||
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | ||
|
|
||
| **Context:** <why this is out of scope for the current PR and what the fix entails> | ||
| EOF | ||
| )" \ | ||
| --label "follow-up" | ||
| ``` |
There was a problem hiding this comment.
--repo flag on gh issue create
gh issue create here has no --repo optave/codegraph flag. Elsewhere in this same skill, gh pr list uses --repo optave/codegraph (Step 1) and every gh api call is hardcoded to repos/optave/codegraph/.... Without the flag, gh infers the repo from the current git remote — which is correct in normal operation, but fragile if the skill is ever invoked outside a clean checkout or if the working directory is inside a nested worktree that has a different remote.
To be consistent with the rest of the skill and make this robust:
| gh issue create \ | |
| --title "follow-up: <concise description of what needs to be done>" \ | |
| --body "$(cat <<'EOF' | |
| Deferred from PR #<number> review. | |
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | |
| **Context:** <why this is out of scope for the current PR and what the fix entails> | |
| EOF | |
| )" \ | |
| --label "follow-up" | |
| ``` | |
| gh issue create \ | |
| --repo optave/codegraph \ | |
| --title "follow-up: <concise description of what needs to be done>" \ | |
| --body "$(cat <<'EOF' | |
| Deferred from PR #<number> review. | |
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | |
| **Context:** <why this is out of scope for the current PR and what the fix entails> | |
| EOF | |
| )" \ | |
| --label "follow-up" |
There was a problem hiding this comment.
Fixed — added --repo optave/codegraph to the gh issue create command.
.claude/skills/review/SKILL.md
Outdated
| Then reply to the reviewer comment referencing the issue: | ||
| ```bash | ||
| gh api repos/optave/codegraph/pulls/<number>/comments/<comment-id>/replies \ | ||
| -f body="Out of scope for this PR — tracked in #<issue-number>" | ||
| ``` |
There was a problem hiding this comment.
The template reply in step 5 hardcodes the pulls/<number>/comments/<comment-id>/replies endpoint, which is only valid for inline PR review comments (those returned by GET /pulls/{pull_number}/comments). Greptile and Claude also leave top-level review body comments (returned by GET /pulls/{pull_number}/reviews) and issue-style comments (returned by GET /issues/{number}/comments), both of which require a different endpoint.
If the out-of-scope comment is a top-level review body, calling pulls/<number>/comments/<comment-id>/replies will fail with a 404 or create a misplaced reply. Step 6 already distinguishes the three cases for the regular-fix reply — step 5 should do the same:
Then reply to the reviewer comment using the correct endpoint for the comment type:
- Inline PR review comment → gh api repos/optave/codegraph/pulls/<number>/comments/<comment-id>/replies -f body="Out of scope for this PR — tracked in #<issue-number>"
- Top-level review body → gh api repos/optave/codegraph/issues/<number>/comments -f body="Out of scope for this PR — tracked in #<issue-number>"
- Issue-style comment → gh api repos/optave/codegraph/issues/<number>/comments -f body="Out of scope for this PR — tracked in #<issue-number>"
There was a problem hiding this comment.
Fixed — step 5 now distinguishes all three reply endpoints (inline review comments, top-level review bodies, issue-style comments), matching the same pattern as step 6.
.claude/skills/review/SKILL.md
Outdated
| 5. **If the fix is genuinely out of scope** for this PR (e.g., it affects a different module not touched by this PR, or requires a design decision beyond the PR's purpose), you MUST create a GitHub issue to track it before replying. Never reply with "acknowledged as follow-up" or "noted for later" without a tracked issue — untracked deferrals get lost and nobody will ever revisit them. | ||
|
|
||
| ```bash | ||
| # Create a tracking issue for the deferred item | ||
| gh issue create \ | ||
| --title "follow-up: <concise description of what needs to be done>" \ | ||
| --body "$(cat <<'EOF' | ||
| Deferred from PR #<number> review. | ||
|
|
||
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | ||
|
|
||
| **Context:** <why this is out of scope for the current PR and what the fix entails> | ||
| EOF | ||
| )" \ | ||
| --label "follow-up" | ||
| ``` | ||
|
|
||
| Then reply to the reviewer comment referencing the issue: | ||
| ```bash | ||
| gh api repos/optave/codegraph/pulls/<number>/comments/<comment-id>/replies \ | ||
| -f body="Out of scope for this PR — tracked in #<issue-number>" | ||
| ``` |
There was a problem hiding this comment.
The new step 5 introduces a binary: either fix the comment (steps 3–4) or create an issue (step 5). The term "genuinely out of scope" is left undefined, which means a future run of the skill may be overly liberal in deferring (creating noise issues for minor comments) or overly conservative (trying to fix things it shouldn't).
Consider adding a short parenthetical or a note clarifying the bar, e.g.:
"Genuinely out of scope" means the fix touches a different module not in this PR's diff, requires an architectural decision outside this PR's mandate, or would introduce unrelated risk. Fixing a variable name, adding a missing null check, or updating a comment in a file that is already part of this PR's diff is not out of scope — address it directly."
This mirrors how the "Flag scope creep" rule in the Rules section gives concrete examples to anchor the judgment call.
There was a problem hiding this comment.
Fixed — added explicit guidance: out of scope means the fix touches a different module not in the PR's diff, requires an architectural decision beyond the PR's mandate, or would introduce unrelated risk. Fixing a variable name, null check, or string in a file already in the diff is NOT out of scope.
.claude/skills/review/SKILL.md
Outdated
| **Context:** <why this is out of scope for the current PR and what the fix entails> | ||
| EOF | ||
| )" \ | ||
| --label "follow-up" |
There was a problem hiding this comment.
--label will fail if "follow-up" label doesn't exist
gh issue create --label "follow-up" exits with a non-zero status if the label doesn't yet exist in the repository. If this runs on a freshly-cloned repo or one where the label was never created, the entire gh issue create command will fail — and the skill will have neither created the issue nor replied to the reviewer, silently defeating the very goal of this change.
Consider guarding with a label-creation step or using || true on the label lookup:
# Ensure the follow-up label exists (safe to re-run)
gh label create "follow-up" --color "0e8a16" --description "Deferred from PR review" --repo optave/codegraph 2>/dev/null || true
gh issue create \
--repo optave/codegraph \
--title "follow-up: <concise description of what needs to be done>" \
--body "$(cat <<'EOF'
Deferred from PR #<number> review.
**Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id>
**Context:** <why this is out of scope for the current PR and what the fix entails>
EOF
)" \
--label "follow-up"
There was a problem hiding this comment.
Fixed — added a gh label create guard step before gh issue create that ensures the "follow-up" label exists. The guard is safe to re-run (stderr suppressed, || true).
gh issue create --label "follow-up" fails if the label doesn't exist in the repo. Add a gh label create guard step that is safe to re-run.
.claude/skills/review/SKILL.md
Outdated
| --body "$(cat <<'EOF' | ||
| Deferred from PR #<number> review. | ||
|
|
||
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | ||
|
|
||
| **Context:** <why this is out of scope for the current PR and what the fix entails> | ||
| EOF | ||
| )" \ |
There was a problem hiding this comment.
EOF terminator is indented — will break literal execution
The EOF closing delimiter on line 139 ( EOF) is indented by 3 spaces inside the list-item code block. In bash, <<'EOF' requires the terminator to sit at column 0 — leading spaces are not stripped. If Claude (or a developer) copies and runs this block verbatim, the heredoc will never close and the command will hang or fail.
Use <<-'EOF' (note the -) with a tab-indented terminator, or move the EOF to column 0 within the fenced block:
| --body "$(cat <<'EOF' | |
| Deferred from PR #<number> review. | |
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | |
| **Context:** <why this is out of scope for the current PR and what the fix entails> | |
| EOF | |
| )" \ | |
| --body "$(cat <<'EOF' | |
| Deferred from PR #<number> review. | |
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | |
| **Context:** <why this is out of scope for the current PR and what the fix entails> | |
| EOF | |
| )" \ |
There was a problem hiding this comment.
Fixed — switched to <<-'EOF' with tab-indented terminator so the heredoc closes correctly when copied from the markdown list context.
| @@ -0,0 +1,271 @@ | |||
| --- | |||
There was a problem hiding this comment.
This PR is titled fix(skill): ban untracked deferrals in /review skill, but it bundles four entirely new skill files and a substantial roadmap update that are unrelated to that fix:
.claude/skills/bench-check/SKILL.md(271 new lines — new skill).claude/skills/deps-audit/SKILL.md(186 new lines — new skill).claude/skills/housekeep/SKILL.md(241 new lines — new skill).claude/skills/test-health/SKILL.md(261 new lines — new skill)docs/roadmap/ROADMAP.md(91 insertions / 55 deletions — phase status updates)
Of the 1,084 total insertions, roughly 959 (~88%) belong to these unrelated files. CLAUDE.md is explicit: "One PR = one concern. Each pull request should address a single feature, fix, or refactor. Do not pile unrelated changes into an existing PR — open a new branch and PR instead."
The four new skill files and the roadmap update should be split into their own PR(s) before this is merged.
Context Used: CLAUDE.md (source)
| | [**3**](#phase-3--architectural-refactoring) | Architectural Refactoring (Vertical Slice) | Unified AST analysis framework, command/query separation, repository pattern, queries.js decomposition, composable MCP, CLI commands, domain errors, builder pipeline, presentation layer, domain grouping, curated API, unified graph model, qualified names, CLI composability | **Complete** (v3.1.5) | | ||
| | [**4**](#phase-4--resolution-accuracy) | Resolution Accuracy | Dead role sub-categories, receiver type tracking, interface/trait implementation edges, resolution precision/recall benchmarks, `package.json` exports field, monorepo workspace resolution | **In Progress** (5 of 6 complete) | | ||
| | [**5**](#phase-5--typescript-migration) | TypeScript Migration | Project setup, core type definitions, leaf -> core -> orchestration module migration, test migration, supply-chain security, CI coverage gates | **In Progress** (2 of 7 complete) | | ||
| | [**4**](#phase-4--resolution-accuracy) | Resolution Accuracy | Dead role sub-categories, receiver type tracking, interface/trait implementation edges, resolution precision/recall benchmarks, `package.json` exports field, monorepo workspace resolution | **Complete** (v3.3.1) | |
There was a problem hiding this comment.
The Phase 5 row in the overview table previously listed supply-chain security and CI coverage gates as key deliverables. Both are removed here without explanation:
-| TypeScript Migration | ... test migration, supply-chain security, CI coverage gates | In Progress (2 of 7) |
+| TypeScript Migration | ... test migration | In Progress (76 of 283) |Neither item appears in the updated Phase 5 detailed section either. Were these sub-phases intentionally deprioritized or moved to a later phase? If so, it's worth a brief note in the Phase 5 section so future readers understand why they were dropped.
There was a problem hiding this comment.
Intentionally moved, not dropped. Supply-chain security and CI coverage gates were relocated to Phase 9 (see sections 9.1 and 9.2 in the updated ROADMAP.md) since they are independent of the TypeScript migration itself. Phase 5 now focuses purely on the .js-to-.ts file migration.
.claude/skills/review/SKILL.md
Outdated
| --body "$(cat <<-'EOF' | ||
| Deferred from PR #<number> review. | ||
|
|
||
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> |
There was a problem hiding this comment.
The issue body template always uses #discussion_r<comment-id>, which is the correct anchor for inline PR review comments. However, step 5 already distinguishes three comment types for the reply endpoint. The issue body's permalink should match the comment type:
- Inline PR review comment →
https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> - Top-level review body →
https://github.com/optave/codegraph/pull/<number>#pullrequestreview-<review-id> - Issue-style comment →
https://github.com/optave/codegraph/issues/<number>#issuecomment-<comment-id>
Using #discussion_r<comment-id> for a top-level review body or issue-style comment will still link to the PR page, but the anchor will not resolve to the specific comment — making it harder for the future issue assignee to find the original context.
Consider varying the URL template based on comment type, consistent with how step 5 and step 6 already branch on comment type for the reply endpoint.
There was a problem hiding this comment.
Fixed — the issue body URL template now varies by comment type: inline review comments use #discussion_r, top-level review bodies use #pullrequestreview-, and issue-style comments use #issuecomment-. This matches the same three-way branching already used in step 5 and step 6 for reply endpoints.
…#568) Kept PR's improvements (label guard, out-of-scope definition, heredoc fix, multi-endpoint replies). Also varied the issue body URL template to use the correct permalink format per comment type (inline, review body, issue-style).
.claude/skills/review/SKILL.md
Outdated
| # Create a tracking issue for the deferred item | ||
| gh issue create \ | ||
| --repo optave/codegraph \ | ||
| --title "follow-up: <concise description of what needs to be done>" \ | ||
| --body "$(cat <<'EOF' | ||
| Deferred from PR #<number> review. | ||
| --body "$(cat <<-'EOF' | ||
| Deferred from PR #<number> review. | ||
|
|
||
| **Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id> | ||
| **Original reviewer comment:** <use the correct permalink format for the comment type: inline review comment → `https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id>`, top-level review body → `https://github.com/optave/codegraph/pull/<number>#pullrequestreview-<review-id>`, issue-style comment → `https://github.com/optave/codegraph/issues/<number>#issuecomment-<comment-id>`> | ||
|
|
||
| **Context:** <why this is out of scope for the current PR and what the fix entails> | ||
| EOF | ||
| **Context:** <why this is out of scope for the current PR and what the fix entails> | ||
| EOF | ||
| )" \ | ||
| --label "follow-up" | ||
| ``` | ||
|
|
||
| Then reply to the reviewer comment referencing the issue: | ||
| Then reply to the reviewer comment referencing the issue. Use the same reply mechanism as step 6 below — inline PR review comments use `/pulls/<number>/comments/<comment-id>/replies`, top-level review bodies and issue-style comments use `/issues/<number>/comments`: | ||
| ```bash | ||
| # For inline PR review comments: | ||
| gh api repos/optave/codegraph/pulls/<number>/comments/<comment-id>/replies \ | ||
| -f body="Out of scope for this PR — tracked in #<issue-number>" | ||
| # For top-level review bodies or issue-style comments: | ||
| gh api repos/optave/codegraph/issues/<number>/comments \ | ||
| -f body="Out of scope for this PR — tracked in #<issue-number>" | ||
| ``` |
There was a problem hiding this comment.
gh issue create is run without capturing its output, yet #<issue-number> is referenced immediately afterwards in both reply templates. gh issue create prints the new issue's URL (e.g. https://github.com/optave/codegraph/issues/123) to stdout — Claude would need to parse that to get the number. Since the instructions don't make this explicit, a literal reading leaves a gap between the creation and the reply step.
Consider capturing the output and extracting the number explicitly:
# Create a tracking issue for the deferred item
issue_url=$(gh issue create \
--repo optave/codegraph \
--title "follow-up: <concise description of what needs to be done>" \
--body "$(cat <<-'EOF'
Deferred from PR #<number> review.
**Original reviewer comment:** https://github.com/optave/codegraph/pull/<number>#discussion_r<comment-id>
**Context:** <why this is out of scope for the current PR and what the fix entails>
EOF
)" \
--label "follow-up")
issue_number=$(echo "$issue_url" | grep -oE '[0-9]+$')Then use $issue_number in the reply body. This removes any ambiguity for the agent running these instructions.
There was a problem hiding this comment.
Fixed — \ output is now captured into , and the number is extracted with . Reply templates use \ instead of the ambiguous placeholder.
…umber gh issue create prints the new issue URL to stdout — capture it and extract the number so reply templates can reference it unambiguously.
| ### 2i. Return result | ||
|
|
||
| At the end of processing, the subagent MUST return a structured result with these fields so the main agent can build the summary table: | ||
|
|
||
| ``` | ||
| PR: #<number> | ||
| Branch: <branch-name> | ||
| Conflicts: resolved | none | ||
| CI: green | red | pending | ||
| Comments Addressed: <count> | ||
| Reviewers Re-triggered: <list> | ||
| Status: ready | needs-work | needs-human-review | skipped | ||
| Notes: <any issues encountered> | ||
| ``` |
There was a problem hiding this comment.
Step 2i's structured result format has no field for "Issues Created". When a subagent defers an out-of-scope comment via step 2e item 5, it creates a GitHub issue — but that issue number/URL is invisible to the main orchestrator and will never appear in the final summary table.
In practice this means a human reading the Step 3 table cannot tell which PRs had follow-up issues opened, or what those issues were. The same problem this PR was designed to fix (deferred items getting lost) could resurface at the summary level.
Consider adding an Issues Created field:
| ### 2i. Return result | |
| At the end of processing, the subagent MUST return a structured result with these fields so the main agent can build the summary table: | |
| ``` | |
| PR: #<number> | |
| Branch: <branch-name> | |
| Conflicts: resolved | none | |
| CI: green | red | pending | |
| Comments Addressed: <count> | |
| Reviewers Re-triggered: <list> | |
| Status: ready | needs-work | needs-human-review | skipped | |
| Notes: <any issues encountered> | |
| ``` | |
| ### 2i. Return result | |
| At the end of processing, the subagent MUST return a structured result with these fields so the main agent can build the summary table: | |
PR: #
Branch:
Conflicts: resolved | none
CI: green | red | pending
Comments Addressed:
Issues Created: <comma-separated list of # follow-up issues, or "none">
Reviewers Re-triggered:
Status: ready | needs-work | needs-human-review | skipped
Notes:
And update the Step 3 table header to include an Issues column so the main agent surfaces them in the final report.
1 participant
Summary
follow-uplabel) before replying — the reply must include the issue linkProblem: On PR #558, the
/reviewskill replied "Acknowledged — noted as follow-up" to 3 Greptile comments but never tracked them anywhere. They exist only as buried PR comment replies that nobody will revisit.Test plan
/reviewon a PR with an out-of-scope comment and verify it creates an issue before replying