feat(skills): add maintainer loop skills for automated PR triage and salvage#1585
feat(skills): add maintainer loop skills for automated PR triage and salvage#1585
Conversation
…salvage A suite of 8 agent skills and 4 TypeScript scripts that automate the NemoClaw maintainer workflow: triage open PRs, check merge gates, salvage near-ready contributions, detect hotspots, and close test gaps. Skills: - nemoclaw-maintainer-loop: orchestrator for /loop integration - nemoclaw-maintainer-triage: deterministic queue builder (calls gh-pr-merge-now) - nemoclaw-maintainer-merge-gate: 4-gate approval checker - nemoclaw-maintainer-salvage-pr: near-ready PR rescue with fork push support - nemoclaw-maintainer-security-sweep: security-focused review routing - nemoclaw-maintainer-test-gaps: targeted test coverage for risky code - nemoclaw-maintainer-hotspots: churn/conflict pressure detection - nemoclaw-maintainer-sequence-work: large issue decomposition Scripts (node --experimental-strip-types): - triage.ts: calls gh-pr-merge-now, enriches with risky-area detection, scores - check-gates.ts: CI, conflicts, CodeRabbit, risky-code-tested gates - hotspots.ts: 30-day git churn + open PR file overlap analysis - state.ts: local state file CRUD (init, exclude, history, set-queue) Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a NemoClaw maintainer workflow: new documentation for maintainer day/evening/morning procedures, risk/gate/test guidance, and multiple TypeScript CLI scripts to compute version targets, triage/rank PRs, evaluate merge gates, generate hotspots/handoffs, bump labels, and persist maintainer state. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Maintainer as "Maintainer (CLI)"
participant Local as "Local Git / FS"
participant GH as "GitHub API (gh)"
participant State as "State file\n(.nemoclaw-maintainer/state.json)"
Maintainer->>Local: run version-target.ts
Local->>Local: fetch tags, compute targetVersion
Local->>GH: gh pr/issue list (find stragglers)
GH-->>Local: straggler list
Maintainer->>Local: run triage.ts
Local->>GH: gh api --paginate open PRs
GH-->>Local: PR list
Local->>GH: gh pr view (per-PR enrichment)
GH-->>Local: PR details (files, checks)
Local->>Local: score/rank PRs, detect hotClusters
Local->>State: state.ts set-queue (JSON stdin)
State-->>Local: persisted queue
Maintainer->>Local: optional run bump-stragglers.ts / hotspots.ts / handoff-summary.ts
Local->>GH: gh label/edit or gh pr view as needed
GH-->>Local: updated labels / data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Replace 9 separate maintainer skills with 3 phase-based skills matching the daily ship cycle: morning (triage + label), day (execute + loop), evening (handoff + tag). Inner workflows become reference docs under the day skill with progressive disclosure. Key changes: - Extract shared.ts to deduplicate RISKY_PATTERNS, run(), scoring weights - Fail-closed gate checker (was defaulting to pass on API error) - Named scoring constants with documented rationale - Add PR-REVIEW-PRIORITIES.md codifying the review priority order - Add version-label-based daily cadence (label items with target tag, bump stragglers at EOD, cut patch tag for overnight QA) - stderr logging in shell helpers instead of silent swallowing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…w issues New scripts for deterministic operations that were previously inline: - version-target.ts: compute next patch version + find stragglers - version-progress.ts: shipped vs open for a version label - bump-stragglers.ts: move open items from one version label to next - handoff-summary.ts: commits since last tag + risky areas + QA focus Review fixes: - Remove duplicated scoring table from morning (reference PR-REVIEW-PRIORITIES.md) - Security sweep uses triage-identified items instead of re-running find-review-pr - Evening uses state.ts script command for state updates - Morning uses version-target.ts for straggler discovery (no ambiguous placeholder) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both version-progress.ts and triage.ts now include ageDays as a computed field in their JSON output, so triage tables never depend on Claude doing date arithmetic from raw ISO timestamps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
At a daily ship cadence most PRs are <24h old, making ageDays show 0 for everything. ageHours gives useful resolution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove external gh-pr-merge-now dependency from triage script. The script now fetches open PRs directly via gh CLI, classifies them as merge-ready / near-miss / blocked, and scores them using the same weighted model. Key changes: - Fetch basic PR data first (lightweight), then enrich top candidates with statusCheckRollup individually to avoid GraphQL timeouts - Add single retry with 3s delay for transient GitHub API failures - Add labels array to QueueItem output for version-label awareness - Classification logic: merge-now requires approved + green CI + clean merge state; near-miss allows up to 2 fixable blockers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GitHub's GraphQL API returns 502 when requesting too many PRs with rich fields (statusCheckRollup, mergeStateStatus, reviewDecision). Limit to 50 most recent PRs which is sufficient for daily triage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three bugs in the triage classifier: - BLOCKED merge state was treated as a conflict (it means "missing required reviews", not a conflict — DIRTY is the actual conflict) - StatusContext checks (CodeRabbit) use state/context fields, not conclusion/status — handle both shapes from gh CLI - No bucket existed between "approved + merge-ready" and "needs salvage", so most OSS PRs fell to blocked Add review-ready bucket (green CI, not draft, no conflicts) scored at +35, between merge-now (+40) and near-miss (+30). Also simplify by fetching statusCheckRollup in the initial query (works at limit 50) instead of per-PR enrichment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace single gh pr list (capped at 50 by GraphQL) with gh api --paginate using the REST endpoint. Fetches all open PRs as lightweight NDJSON, then enriches only the top candidates with CI/review data via per-PR GraphQL calls. Previous: 50 PRs visible, missed older security PRs Now: all 243 open PRs visible, oldest items no longer invisible to triage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The salvage workflow was biased toward "ask the contributor and wait" which blocks the daily ship cadence. Now defaults to maintainer does the work (rebase, fix conflicts, add tests, push). Only defer to contributor when intent is ambiguous from the diff. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GitHub invalidates approvals when new commits are pushed. Approving a PR with conflicts is wasted effort since the rebase push resets it. Merge gate now explicitly: do not approve DIRTY PRs. Correct sequence is salvage (rebase) → CI green → approve → report ready for merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (7)
.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts (2)
76-76: Silent catch blocks hide debugging information.Swallowing JSON parse errors makes it difficult to diagnose malformed
ghoutput. Consider logging a warning to stderr before continuing.- } catch { /* ignore parse errors */ } + } catch (err) { + process.stderr.write(`[version-target] Failed to parse PR list: ${err}\n`); + }Also applies to: 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts at line 76, The empty catch in .agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts silently swallows JSON parse errors (seen at the try/catch around the JSON.parse call at lines ~76 and ~103); change the catch blocks to capture the error (e.g., catch (err)) and log a warning to stderr (console.error or process.stderr.write) including the error message and the raw gh output being parsed so malformed output is visible for debugging before continuing.
29-47: ExtractgetLatestTag()andbumpPatch()toshared.tsto eliminate duplication.Both functions appear identically in
handoff-summary.ts(lines 38–51). Sinceshared.tsis already the established utilities module imported by both files and both functions depend only on therun()helper already there, extracting them will reduce redundancy and improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts around lines 29 - 47, Extract the duplicated functions getLatestTag and bumpPatch from version-target.ts and handoff-summary.ts into shared.ts and have both files import them; specifically, move the implementations (which use the existing run() helper) into shared.ts as exported functions getLatestTag and bumpPatch, remove the local definitions from version-target.ts and handoff-summary.ts, and update their imports to import { getLatestTag, bumpPatch } from './shared' (or the module path already used for shared utilities)..agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts (1)
62-62: Silent catch blocks hide failures.Same concern as
version-target.ts—consider logging parse failures to stderr for debuggability.Also applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts at line 62, There are silent catch blocks (the empty "catch { /* ignore */ }" in bump-stragglers.ts and the similar one around line ~80) that swallow parse failures; replace each with a parameterized catch (e.g., "catch (err)") and log the error to stderr with context using console.error (include what was being parsed or the offending value and the error.message/stack) so failures are visible for debugging, keeping execution flow otherwise unchanged..agents/skills/nemoclaw-maintainer-day/scripts/handoff-summary.ts (1)
30-36: Code duplication of risky file patterns could lead to future drift.The
RISKY_PATTERNSinshared.tsandAREA_LABELSinhandoff-summary.tscurrently define identical patterns, but maintaining them in two separate places creates ongoing synchronization risk. While currently synchronized, they could diverge if one is updated without the other.Consider consolidating by either:
- Exporting
AREA_LABELSfromshared.tsalongsideisRiskyFile, or- Deriving
isRiskyFilefromAREA_LABELSto maintain a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/handoff-summary.ts around lines 30 - 36, AREA_LABELS in handoff-summary.ts duplicates the risky file patterns defined as RISKY_PATTERNS in shared.ts which risks future drift; consolidate to a single source by either exporting AREA_LABELS (or the pattern map) from shared.ts and importing it into handoff-summary.ts, or modify shared.ts to export a single RISKY_PATTERNS/AREA_LABELS constant and update isRiskyFile to derive its checks from that exported map so handoff-summary.ts reuses the shared export instead of defining its own AREA_LABELS..agents/skills/nemoclaw-maintainer-day/scripts/shared.ts (1)
34-38: TEST_PATTERNS may miss common test directory conventions.The pattern
^test\/only matches paths starting withtest/. Consider also matchingtests/,__tests__/, or*_test.go/test_*.pypatterns if the repo uses those conventions.Suggested expansion
export const TEST_PATTERNS: RegExp[] = [ /\.test\.[jt]sx?$/, /\.spec\.[jt]sx?$/, /^test\//, + /^tests\//, + /^__tests__\//, + /_test\.go$/, + /test_.*\.py$/, ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/shared.ts around lines 34 - 38, The TEST_PATTERNS array currently only matches /\.test\.[jt]sx?$/, /\.spec\.[jt]sx?$/, and paths starting with /^test\//; expand it to cover other common test conventions by adding patterns for /^tests?\// (both test/ and tests/), /__tests__\//, filenames ending with /_test\.[a-z]+$/ (e.g., *_test.go), and filenames starting with /^test_.*\.[a-z]+$/ (e.g., test_*.py); update the TEST_PATTERNS RegExp[] to include these new regexes so test file discovery handles these conventions..agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts (1)
219-243: Consider whether "any test file" is sufficient for the risky-code gate.The gate passes if risky files are changed and any test file exists in the PR. This doesn't verify the tests actually cover the risky code. This may be intentional (keeping the gate simple), but worth noting that a PR touching
install.shwould pass if it also adds an unrelated unit test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts around lines 219 - 243, The current checkRiskyCodeTested function treats any test file in the PR as sufficient for risky-file coverage, which allows unrelated tests to pass the gate; update checkRiskyCodeTested to require tests that are likely to exercise the risky files by matching test filenames or paths to the riskyFiles (e.g., test file path contains the risky file basename, same directory, or follows a naming convention like foo.test.ts for foo.ts) or otherwise map riskyFiles to corresponding test files using isTestFile and filename matching; ensure you reference and use the riskyFiles array and the existing isTestFile/isRiskyFile helpers to compute a more precise hasTests value and adjust the returned details and hasTests boolean accordingly..agents/skills/nemoclaw-maintainer-day/scripts/triage.ts (1)
398-404: Enrichment only covers firstlimit * 3candidates, leaving others with empty CI data.Non-draft PRs beyond
enrichCountwon't havestatusCheckRolluppopulated, causingclassifyPrto mark them aschecksGreen = false(line 231). This is documented at line 406 as intentional, but means the "blocked" count at line 412 may be inflated by un-enriched PRs rather than truly blocked ones.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts around lines 398 - 404, The enrichment only runs for enrichCount = Math.min(candidates.length, limit * 3), leaving later non-draft PRs without statusCheckRollup and causing classifyPr to treat them as failing; change the logic so all non-draft PRs are enriched before classification—either set enrichCount = candidates.length or call enrichPr for every entry in candidates (and await/Promise.all if enrichPr is async), ensuring statusCheckRollup is populated for use in classifyPr and referenced checks (e.g., checksGreen).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/nemoclaw-maintainer-day/HOTSPOTS.md:
- Around line 1-46: Do not edit HOTSPOTS.md directly; instead update the
original Hotspots source document that feeds the markdown generator, then run
the docs-to-skills.py generator to regenerate the skill artifact (ensure the
change appears in the generated HOTSPOTS.md), and commit both the source doc and
the regenerated output so CI remains consistent.
In @.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md:
- Around line 1-47: This file is autogenerated and must not be edited directly;
revert any changes to .agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md and
instead make your content edits in the canonical source docs, then run the
generator script scripts/docs-to-skills.py to regenerate the skill file;
reference the autogenerated path
(.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md) and the generator
(scripts/docs-to-skills.py) when updating so future regenerations don't
overwrite manual edits.
In @.agents/skills/nemoclaw-maintainer-day/SALVAGE-PR.md:
- Around line 1-84: The SALVAGE-PR.md content was edited manually but must be
generated; revert manual edits to the "Salvage PR Workflow" markdown and
regenerate it from the canonical source using the docs-to-skills generator (run
the docs-to-skills.py tool to rebuild the skill artifact), then commit the
autogenerated output instead of hand-editing; ensure the generator is run and
its output replaces the current SALVAGE-PR.md so the file matches the canonical
source.
In @.agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts:
- Around line 36-37: The current extraction of the repo value from args uses
repoIdx without ensuring an argument exists after the flag (repoIdx >= 0 ?
args[repoIdx + 1] : ...), which can read undefined or throw; update the check in
bump-stragglers.ts so you only access args[repoIdx + 1] when repoIdx >= 0 &&
repoIdx + 1 < args.length, otherwise fall back to the default "NVIDIA/NemoClaw";
reference the variables repoIdx, args and repo when making the change.
In @.agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts:
- Around line 112-116: Validate the parsed CLI args before use: check that
daysIdx >= 0 and the token at args[daysIdx + 1] exists and parseInt(args[daysIdx
+ 1], 10) yields a finite number (not NaN) before assigning to days, otherwise
throw a clear error or fallback to the default 30; similarly ensure repoIdx >= 0
and args[repoIdx + 1] is a non-empty string before assigning to repo, otherwise
throw a clear error or fallback to "NVIDIA/NemoClaw". Update the code around
daysIdx/days and repoIdx/repo and add explicit validation checks prior to
calling gitChurn() and openPrFileOverlap() so downstream git/gh invocations
never receive NaN or undefined values and instead fail fast with a helpful
message.
In @.agents/skills/nemoclaw-maintainer-day/scripts/state.ts:
- Around line 179-192: The cmdSetQueue function currently calls JSON.parse on
stdin without error handling; wrap the readFileSync + JSON.parse logic in a
try-catch inside cmdSetQueue (around the input/triageOutput parse) to catch
SyntaxError and other JSON errors, log a clear user-friendly error via
console.error including the raw input or the parse error message, and exit
non-zero (process.exit(1)) so callers see failure; reference cmdSetQueue,
readFileSync(0, "utf-8"), JSON.parse(input), loadState, saveState and ensure
normal state update only proceeds when parsing succeeds.
- Around line 194-205: The cmdSetHotspots function lacks error handling around
reading and parsing stdin; wrap the readFileSync/JSON.parse of input in a
try-catch inside cmdSetHotspots, catch and log a descriptive error via
console.error (including the caught error.message or error object) and exit with
a non-zero status (e.g., process.exit(1)) on failure; also validate that
hotspotOutput.hotspots is an array before assigning to state.hotspots.files and
log a clear error if the structure is invalid so callers get a helpful message.
- Around line 146-154: cmdExclude currently always writes exclusions to
state.excluded.prs, but the function is documented as "Add PR/issue to permanent
exclusion list" so it must distinguish PRs vs issues; update cmdExclude to
accept a type flag or detect item kind and write to either state.excluded.prs or
state.excluded.issues accordingly (e.g., add a boolean parameter like isIssue or
check a CLI --issue flag from the caller), ensure the CLI handler is updated to
pass that flag, and persist to state.excluded.issues with the same shape
(reason, excludedAt) when the item is an issue, leaving existing PR behavior
unchanged.
In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts:
- Around line 165-167: The --approved-only filter in fetchOpenPrs is ineffective
because fetchOpenPrs filters on pr.reviewDecision === "APPROVED" before enrichPr
populates reviewDecision; move the approved-only logic out of fetchOpenPrs and
into main() after calling enrichPr (or remove the flag entirely). Specifically,
stop filtering by reviewDecision inside fetchOpenPrs, call enrichPr for each PR
(using enrichPr), then if approvedOnly is true filter the enriched PR array by
pr.reviewDecision === "APPROVED" before further processing.
- Around line 302-342: The scoreItem function has a type/logic mismatch: the
bucket variable and the function return type should include "merge-now" and
"review-ready" (not the currently declared "ready-now"), and the security-path
conditional is dead/wrong (it checks bucket === "ready-now"); update the bucket
type union in scoreItem and the function return type to "merge-now" |
"review-ready" | "salvage-now" | "blocked", and change the security-files branch
to test whether bucket is one of the merge-capable buckets (e.g., bucket ===
"merge-now" || bucket === "review-ready") so nextAction becomes "security-sweep
→ merge-gate" for those buckets and "security-sweep → salvage-pr" otherwise;
keep references to bucket, nextAction, scoreItem, and riskyFiles.
In @.agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts:
- Around line 63-65: The repo lookup currently reads args[repoIdx + 1] into repo
without validating it, so when `--repo` is passed with no value you end up
passing undefined/"undefined" to gh; change the logic around repoIdx/repo to
validate that args[repoIdx + 1] exists and is a non-flag string (e.g., not
starting with "--" and not the literal "undefined"), and if the value is
missing/invalid throw an Error or call process.exit(1) with a clear message;
update the code that defines repo (the repoIdx and repo variables in
version-progress.ts) to perform this check before using repo in the gh query.
In @.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts:
- Around line 110-112: The repo extraction logic using args, repoIdx and repo is
missing a bounds check for the value at args[repoIdx + 1]; update the code that
computes repo to verify repoIdx >= 0 and repoIdx + 1 < args.length and that
args[repoIdx + 1] is a non-empty string, otherwise fall back to the default
"NVIDIA/NemoClaw"; modify the expression that sets repo (or add a short
conditional) so passing "--repo" as the last argument won't set repo to
undefined.
In @.agents/skills/nemoclaw-maintainer-day/SKILL.md:
- Around line 1-79: The SKILL.md for the skill named "nemoclaw-maintainer-day"
is autogenerated and must not be hand-edited; revert your manual edits to
SKILL.md and instead apply your changes to the upstream documentation source
used by docs-to-skills.py, then run the docs-to-skills.py regeneration step to
produce an updated "nemoclaw-maintainer-day" SKILL.md (verify helper scripts
like version-target.ts, version-progress.ts and state.ts still work), and commit
the regenerated skill output rather than committing direct edits to the
generated SKILL.md.
In @.agents/skills/nemoclaw-maintainer-day/TEST-GAPS.md:
- Around line 1-65: The TEST-GAPS.md file is autogenerated and must not be
edited directly; update the original source documentation that feeds the
generator and then run the generator script to regenerate this skill file:
modify the upstream docs (the source that maps to TEST-GAPS content), then run
scripts/docs-to-skills.py to produce the updated
.agents/skills/nemoclaw-maintainer-day/TEST-GAPS.md, committing the source
changes and the regenerated output rather than editing TEST-GAPS.md by hand.
In @.agents/skills/nemoclaw-maintainer-evening/SKILL.md:
- Around line 22-28: bump-stragglers.ts can silently continue when run() returns
an empty string, so update the script to detect empty/invalid run() results and
fail fast: after each call to run() in bump-stragglers.ts validate the return
(and any parsed JSON) and if missing or malformed (e.g. no "bumped" array) write
a clear error to stderr and exit non-zero; also ensure any label-creation or
PR/issue-editing failures from run() are surfaced (log the error output and
include it in the process exit) and add a final check that the emitted JSON
contains the expected "bumped" items before reporting success.
In @.agents/skills/nemoclaw-maintainer-morning/SKILL.md:
- Around line 1-65: The SKILL.md file
(.agents/skills/nemoclaw-maintainer-morning/SKILL.md) is autogenerated and must
not be edited directly; revert any manual changes here and update the
authoritative source content or the generator instead (update the original doc
used by scripts/docs-to-skills.py or modify scripts/docs-to-skills.py to produce
the desired change), then re-run the docs-to-skills.py generation to regenerate
SKILL.md so the change persists.
- Around line 29-32: Update the SKILL.md paragraph to reflect actual triage
behavior: replace the claim that the script "calls `gh-pr-merge-now --json`"
with a statement that triage.ts uses the GitHub CLI REST API (`gh api
--paginate`) to list PRs and enriches top candidates with `gh pr view --json`
(review status, CI status), performs risky-file detection, applies the scoring
model from PR-REVIEW-PRIORITIES.md, and also uses find-review-pr to add PRs
labeled `security` + `priority: high` into the candidate pool.
---
Nitpick comments:
In @.agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts:
- Line 62: There are silent catch blocks (the empty "catch { /* ignore */ }" in
bump-stragglers.ts and the similar one around line ~80) that swallow parse
failures; replace each with a parameterized catch (e.g., "catch (err)") and log
the error to stderr with context using console.error (include what was being
parsed or the offending value and the error.message/stack) so failures are
visible for debugging, keeping execution flow otherwise unchanged.
In @.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts:
- Around line 219-243: The current checkRiskyCodeTested function treats any test
file in the PR as sufficient for risky-file coverage, which allows unrelated
tests to pass the gate; update checkRiskyCodeTested to require tests that are
likely to exercise the risky files by matching test filenames or paths to the
riskyFiles (e.g., test file path contains the risky file basename, same
directory, or follows a naming convention like foo.test.ts for foo.ts) or
otherwise map riskyFiles to corresponding test files using isTestFile and
filename matching; ensure you reference and use the riskyFiles array and the
existing isTestFile/isRiskyFile helpers to compute a more precise hasTests value
and adjust the returned details and hasTests boolean accordingly.
In @.agents/skills/nemoclaw-maintainer-day/scripts/handoff-summary.ts:
- Around line 30-36: AREA_LABELS in handoff-summary.ts duplicates the risky file
patterns defined as RISKY_PATTERNS in shared.ts which risks future drift;
consolidate to a single source by either exporting AREA_LABELS (or the pattern
map) from shared.ts and importing it into handoff-summary.ts, or modify
shared.ts to export a single RISKY_PATTERNS/AREA_LABELS constant and update
isRiskyFile to derive its checks from that exported map so handoff-summary.ts
reuses the shared export instead of defining its own AREA_LABELS.
In @.agents/skills/nemoclaw-maintainer-day/scripts/shared.ts:
- Around line 34-38: The TEST_PATTERNS array currently only matches
/\.test\.[jt]sx?$/, /\.spec\.[jt]sx?$/, and paths starting with /^test\//;
expand it to cover other common test conventions by adding patterns for
/^tests?\// (both test/ and tests/), /__tests__\//, filenames ending with
/_test\.[a-z]+$/ (e.g., *_test.go), and filenames starting with
/^test_.*\.[a-z]+$/ (e.g., test_*.py); update the TEST_PATTERNS RegExp[] to
include these new regexes so test file discovery handles these conventions.
In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts:
- Around line 398-404: The enrichment only runs for enrichCount =
Math.min(candidates.length, limit * 3), leaving later non-draft PRs without
statusCheckRollup and causing classifyPr to treat them as failing; change the
logic so all non-draft PRs are enriched before classification—either set
enrichCount = candidates.length or call enrichPr for every entry in candidates
(and await/Promise.all if enrichPr is async), ensuring statusCheckRollup is
populated for use in classifyPr and referenced checks (e.g., checksGreen).
In @.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts:
- Line 76: The empty catch in
.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts silently
swallows JSON parse errors (seen at the try/catch around the JSON.parse call at
lines ~76 and ~103); change the catch blocks to capture the error (e.g., catch
(err)) and log a warning to stderr (console.error or process.stderr.write)
including the error message and the raw gh output being parsed so malformed
output is visible for debugging before continuing.
- Around line 29-47: Extract the duplicated functions getLatestTag and bumpPatch
from version-target.ts and handoff-summary.ts into shared.ts and have both files
import them; specifically, move the implementations (which use the existing
run() helper) into shared.ts as exported functions getLatestTag and bumpPatch,
remove the local definitions from version-target.ts and handoff-summary.ts, and
update their imports to import { getLatestTag, bumpPatch } from './shared' (or
the module path already used for shared utilities).
🪄 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: CHILL
Plan: Pro
Run ID: e167920d-f3a7-45a6-a368-839751d62115
📒 Files selected for processing (21)
.agents/skills/nemoclaw-maintainer-day/HOTSPOTS.md.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md.agents/skills/nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md.agents/skills/nemoclaw-maintainer-day/RISKY-AREAS.md.agents/skills/nemoclaw-maintainer-day/SALVAGE-PR.md.agents/skills/nemoclaw-maintainer-day/SECURITY-SWEEP.md.agents/skills/nemoclaw-maintainer-day/SEQUENCE-WORK.md.agents/skills/nemoclaw-maintainer-day/SKILL.md.agents/skills/nemoclaw-maintainer-day/STATE-SCHEMA.md.agents/skills/nemoclaw-maintainer-day/TEST-GAPS.md.agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts.agents/skills/nemoclaw-maintainer-day/scripts/handoff-summary.ts.agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts.agents/skills/nemoclaw-maintainer-day/scripts/shared.ts.agents/skills/nemoclaw-maintainer-day/scripts/state.ts.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts.agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts.agents/skills/nemoclaw-maintainer-evening/SKILL.md.agents/skills/nemoclaw-maintainer-morning/SKILL.md
| # Hotspots Workflow | ||
|
|
||
| Find files hurting throughput and reduce their future blast radius. | ||
|
|
||
| ## Step 1: Run the Hotspot Script | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts | ||
| ``` | ||
|
|
||
| This combines 30-day git churn on `main` with open PR file overlap, flags risky areas, and outputs a ranked JSON list. | ||
|
|
||
| Pipe into state: | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts | node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/state.ts set-hotspots | ||
| ``` | ||
|
|
||
| ## Step 2: Prioritize | ||
|
|
||
| Review the ranked output. Most urgent: high `combinedScore` + `isRisky: true` + weak tests. | ||
|
|
||
| ## Step 3: Choose Cooling Strategy | ||
|
|
||
| Smallest change to reduce future collisions: | ||
|
|
||
| - extract stable logic from giant file into tested helper | ||
| - split parsing from execution | ||
| - add regression tests around repeated breakage | ||
| - deduplicate workflow logic | ||
| - narrow interfaces with typed helpers | ||
|
|
||
| Prefer changes that also improve testability. | ||
|
|
||
| ## Step 4: Keep Small | ||
|
|
||
| One file cluster per pass. Stop if next step is large redesign → follow [SEQUENCE-WORK.md](SEQUENCE-WORK.md). | ||
|
|
||
| ## Step 5: Validate | ||
|
|
||
| Run relevant tests. If risky code, also follow [TEST-GAPS.md](TEST-GAPS.md). | ||
|
|
||
| ## Notes | ||
|
|
||
| - Goal is lower future merge pain, not aesthetic cleanup. | ||
| - No giant refactors inside contributor PRs. |
There was a problem hiding this comment.
Avoid direct edits to generated skill markdown.
Please make the change in the source docs and regenerate this artifact.
As per coding guidelines, "Files in .agents/skills/nemoclaw-/.md are autogenerated by scripts/docs-to-skills.py and must never be edited directly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/HOTSPOTS.md around lines 1 - 46, Do
not edit HOTSPOTS.md directly; instead update the original Hotspots source
document that feeds the markdown generator, then run the docs-to-skills.py
generator to regenerate the skill artifact (ensure the change appears in the
generated HOTSPOTS.md), and commit both the source doc and the regenerated
output so CI remains consistent.
| # Merge Gate Workflow | ||
|
|
||
| Run the last maintainer check before approval. Never merge automatically. | ||
|
|
||
| ## Gates | ||
|
|
||
| For the full priority list see [PR-REVIEW-PRIORITIES.md](PR-REVIEW-PRIORITIES.md). A PR is approval-ready only when **all** hard gates pass: | ||
|
|
||
| 1. **CI green** — all required checks in `statusCheckRollup`. | ||
| 2. **No conflicts** — `mergeStateStatus` clean. | ||
| 3. **No major CodeRabbit** — ignore style nits; block on correctness/security bugs. | ||
| 4. **Risky code tested** — see [RISKY-AREAS.md](RISKY-AREAS.md). Confirm tests exist (added or pre-existing). | ||
|
|
||
| ## Step 1: Run the Gate Checker | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts <pr-number> | ||
| ``` | ||
|
|
||
| This checks all 4 gates programmatically and returns structured JSON with `allPass` and per-gate `pass`/`details`. | ||
|
|
||
| ## Step 2: Interpret Results | ||
|
|
||
| The script handles the deterministic checks. You handle judgment calls: | ||
|
|
||
| - **Conflicts (DIRTY):** Do NOT approve — GitHub invalidates approvals when new commits are pushed. Salvage first (rebase), wait for CI, then re-run the gate checker. Follow [SALVAGE-PR.md](SALVAGE-PR.md). | ||
| - **CI failing but narrow:** Follow the salvage workflow in [SALVAGE-PR.md](SALVAGE-PR.md). | ||
| - **CI pending:** Wait and re-check. Do not approve while checks are still running. | ||
| - **CodeRabbit:** Script flags unresolved major/critical threads. Review the `snippet` to confirm it's a real issue vs style nit. If doubt, leave unapproved. | ||
| - **Tests:** If `riskyCodeTested.pass` is false, follow [TEST-GAPS.md](TEST-GAPS.md). | ||
|
|
||
| ## Step 3: Approve or Report | ||
|
|
||
| **Approve only when:** `allPass` is true AND `mergeStateStatus` is not DIRTY. Approving a PR with conflicts is wasted effort — the rebase will invalidate the approval. | ||
|
|
||
| The correct sequence for a conflicted PR: **salvage (rebase) → CI green → approve → report ready for merge.** | ||
|
|
||
| **All pass + no conflicts:** Approve and summarize why. | ||
|
|
||
| **Any fail:** | ||
|
|
||
| | Gate | Status | What is needed | | ||
| |------|--------|----------------| | ||
| | CI | Failing | Fix flaky timeout test | | ||
| | Conflicts | DIRTY | Rebase onto main first — approval would be invalidated | | ||
|
|
||
| Use full GitHub links. |
There was a problem hiding this comment.
Generated skill file should not be edited directly.
This path is designated as autogenerated. Please apply content changes in the source docs and regenerate via scripts/docs-to-skills.py to avoid drift/overwrite on the next regeneration.
As per coding guidelines, "Files in .agents/skills/nemoclaw-/.md are autogenerated by scripts/docs-to-skills.py and must never be edited directly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md around lines 1 - 47,
This file is autogenerated and must not be edited directly; revert any changes
to .agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md and instead make your
content edits in the canonical source docs, then run the generator script
scripts/docs-to-skills.py to regenerate the skill file; reference the
autogenerated path (.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md) and
the generator (scripts/docs-to-skills.py) when updating so future regenerations
don't overwrite manual edits.
| # Salvage PR Workflow | ||
|
|
||
| Take one near-mergeable PR and make the smallest safe change to unblock it. | ||
|
|
||
| **Default to maintainer salvage.** When a maintainer picks an item from the queue, the assumption is they're doing the work now — rebase, fix conflicts, add missing tests, push. Do not default to "ask the contributor and wait" because that blocks the daily cadence. Only defer to the contributor when the fix requires understanding intent that isn't clear from the diff. | ||
|
|
||
| ## Step 1: Gather Context | ||
|
|
||
| ```bash | ||
| gh pr view <number> --repo NVIDIA/NemoClaw \ | ||
| --json number,title,url,body,baseRefName,headRefName,author,files,commits,comments,reviews,statusCheckRollup,mergeStateStatus,reviewDecision | ||
|
|
||
| gh pr diff <number> --repo NVIDIA/NemoClaw | ||
| ``` | ||
|
|
||
| Also read: maintainer and CodeRabbit comments, linked issues, recent `main` changes in touched files. Understand the PR's purpose before coding. | ||
|
|
||
| ## Step 2: Assess Fit | ||
|
|
||
| **Maintainer does it now:** rebase and resolve conflicts, add missing tests for risky code, fix one or two failing checks, apply small correctness fixes from review, narrow gate cleanup. | ||
|
|
||
| **Defer to contributor only when:** the fix requires a design change the maintainer can't judge from the diff, contributor intent is ambiguous and the wrong guess would change semantics, or the PR spans multiple subsystems the maintainer isn't familiar with. | ||
|
|
||
| ## Step 3: Check Out and Reproduce | ||
|
|
||
| ```bash | ||
| gh pr checkout <number> | ||
| git fetch origin --prune | ||
| ``` | ||
|
|
||
| Reproduce locally. Run narrowest relevant commands first. | ||
|
|
||
| ## Step 4: Review PR Scope Before Fixing | ||
|
|
||
| Before fixing, review **all** changed files in the PR — not just the ones causing failures. Flag any files that expand the PR's scope unnecessarily (config changes, unrelated refactors, tool settings). Revert those to `main` if they aren't needed for the feature to work. | ||
|
|
||
| ## Step 5: Fix Narrowly | ||
|
|
||
| Smallest change that clears the blocker. No opportunistic reformatting. | ||
|
|
||
| If risky code is touched (see [RISKY-AREAS.md](RISKY-AREAS.md)), treat missing tests as part of the fix — follow [TEST-GAPS.md](TEST-GAPS.md) when needed. | ||
|
|
||
| ## Step 6: Conflicts | ||
|
|
||
| Resolve only mechanical conflicts (import ordering, adjacent additions, branch drift). Stop and summarize if the conflict changes behavior. | ||
|
|
||
| ## Step 7: Validate | ||
|
|
||
| ```bash | ||
| npm test # root integration tests | ||
| cd nemoclaw && npm test # plugin tests | ||
| npm run typecheck:cli # CLI type check | ||
| make check # all linters | ||
| ``` | ||
|
|
||
| Use only commands matching the changed area. | ||
|
|
||
| ## Step 8: Push | ||
|
|
||
| Push when: fix is small, improves mergeability, validation passed, you have push permission. Never force-push. If you cannot push, prepare a comment describing the fix. | ||
|
|
||
| **Fork PRs:** Most PRs come from contributor forks. Check where to push: | ||
|
|
||
| ```bash | ||
| gh pr view <number> --repo NVIDIA/NemoClaw --json headRepositoryOwner,headRepository,headRefName,maintainerCanModify | ||
| ``` | ||
|
|
||
| If `maintainerCanModify` is true, push directly to the fork: | ||
|
|
||
| ```bash | ||
| git push git@github.com:<owner>/<repo>.git <local-branch>:<headRefName> | ||
| ``` | ||
|
|
||
| Do **not** push to `origin` — that creates a separate branch on NVIDIA/NemoClaw that won't appear in the PR. | ||
|
|
||
| ## Step 9: Route to Merge Gate | ||
|
|
||
| If PR looks ready, follow [MERGE-GATE.md](MERGE-GATE.md). | ||
|
|
||
| ## Notes | ||
|
|
||
| - Goal is safe backlog reduction, not finishing the PR at any cost. | ||
| - Never hide unresolved reviewer concerns. | ||
| - Use full GitHub links. |
There was a problem hiding this comment.
This skill markdown should be generated, not manually edited.
Please update source documentation and regenerate this file to keep skill artifacts canonical.
As per coding guidelines, "Files in .agents/skills/nemoclaw-/.md are autogenerated by scripts/docs-to-skills.py and must never be edited directly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/SALVAGE-PR.md around lines 1 - 84,
The SALVAGE-PR.md content was edited manually but must be generated; revert
manual edits to the "Salvage PR Workflow" markdown and regenerate it from the
canonical source using the docs-to-skills generator (run the docs-to-skills.py
tool to rebuild the skill artifact), then commit the autogenerated output
instead of hand-editing; ensure the generator is run and its output replaces the
current SALVAGE-PR.md so the file matches the canonical source.
.agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts
Outdated
Show resolved
Hide resolved
| --- | ||
| name: nemoclaw-maintainer-day | ||
| description: Runs the daytime maintainer loop for NemoClaw, prioritizing items labeled with the current version target. Picks the highest-value item, executes the right workflow (merge gate, salvage, security sweep, test gaps, hotspot cooling, or sequencing), and reports progress. Use during the workday to land PRs and close issues. Designed for /loop (e.g. /loop 10m /nemoclaw-maintainer-day). Trigger keywords - maintainer day, work on PRs, land PRs, make progress, what's next, keep going, maintainer loop. | ||
| user_invocable: true | ||
| --- | ||
|
|
||
| # NemoClaw Maintainer Day | ||
|
|
||
| Execute one pass of the maintainer loop, prioritizing version-targeted work. | ||
|
|
||
| **Autonomy:** push small fixes and approve when gates pass. Never merge. Stop and ask for merge decisions, architecture decisions, and unclear contributor intent. | ||
|
|
||
| ## References | ||
|
|
||
| - PR review priorities: [PR-REVIEW-PRIORITIES.md](PR-REVIEW-PRIORITIES.md) | ||
| - Risky code areas: [RISKY-AREAS.md](RISKY-AREAS.md) | ||
| - State schema: [STATE-SCHEMA.md](STATE-SCHEMA.md) | ||
|
|
||
| ## Step 1: Check Version Progress | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts <version> | ||
| ``` | ||
|
|
||
| The first script determines the target version. The second shows shipped vs open. | ||
|
|
||
| ## Step 2: Pick One Action | ||
|
|
||
| From the open version-targeted items, pick the highest-value one: | ||
|
|
||
| 1. **Ready-now PR** — green CI, no conflicts, no major CodeRabbit, has tests → follow [MERGE-GATE.md](MERGE-GATE.md) | ||
| 2. **Salvage-now PR** — close to ready, needs small fix → follow [SALVAGE-PR.md](SALVAGE-PR.md) | ||
| 3. **Security item** — touches risky areas → follow [SECURITY-SWEEP.md](SECURITY-SWEEP.md) | ||
| 4. **Test-gap item** — risky code with weak tests → follow [TEST-GAPS.md](TEST-GAPS.md) | ||
| 5. **Hotspot cooling** — repeated conflicts → follow [HOTSPOTS.md](HOTSPOTS.md) | ||
| 6. **Sequencing needed** — too large for one pass → follow [SEQUENCE-WORK.md](SEQUENCE-WORK.md) | ||
|
|
||
| If all version-targeted items are blocked, fall back to the general backlog. Productive work on non-labeled items is better than waiting. | ||
|
|
||
| Prefer finishing one almost-ready contribution over starting a new refactor. | ||
|
|
||
| ## Step 3: Execute | ||
|
|
||
| Follow the chosen workflow document. A good pass ends with one of: | ||
|
|
||
| - a PR approved, a fix pushed, a test gap closed, a hotspot mitigated, or a blocker surfaced. | ||
|
|
||
| ## Step 4: Report Progress | ||
|
|
||
| Re-run the progress script and show the update: | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts <version> | ||
| ``` | ||
|
|
||
| If all version-targeted items are done, suggest running `/nemoclaw-maintainer-evening` early. | ||
|
|
||
| Update `.nemoclaw-maintainer/state.json` via the state script: | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/state.ts history <action> <item> "<note>" | ||
| ``` | ||
|
|
||
| ## Commit Hygiene | ||
|
|
||
| The prek "Regenerate agent skills from docs" hook auto-stages `.agents/skills/` files. Before every `git add` and `git commit` on a PR branch, run `git reset HEAD .agents/skills/nemoclaw-maintainer-*` to unstage them. Only commit skill files in dedicated skill PRs. | ||
|
|
||
| ## Stop and Ask When | ||
|
|
||
| - Broad refactor or architecture decision needed | ||
| - Contributor intent unclear and diff would change semantics | ||
| - Multiple subsystems must change for CI | ||
| - Sensitive security boundaries with unclear risk | ||
| - Next step is opening a new PR or merging | ||
|
|
||
| ## /loop Integration | ||
|
|
||
| Designed for `/loop 10m /nemoclaw-maintainer-day`. Each pass should produce compact output: what was done, what changed, what needs the user. Check `state.json` history to avoid re-explaining prior context on repeat runs. |
There was a problem hiding this comment.
Do not hand-edit this autogenerated skill artifact.
Please move edits to the upstream docs and regenerate the skill output instead of modifying this file directly.
As per coding guidelines, "Files in .agents/skills/nemoclaw-/.md are autogenerated by scripts/docs-to-skills.py and must never be edited directly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/SKILL.md around lines 1 - 79, The
SKILL.md for the skill named "nemoclaw-maintainer-day" is autogenerated and must
not be hand-edited; revert your manual edits to SKILL.md and instead apply your
changes to the upstream documentation source used by docs-to-skills.py, then run
the docs-to-skills.py regeneration step to produce an updated
"nemoclaw-maintainer-day" SKILL.md (verify helper scripts like
version-target.ts, version-progress.ts and state.ts still work), and commit the
regenerated skill output rather than committing direct edits to the generated
SKILL.md.
| # Test Gaps Workflow | ||
|
|
||
| Close the highest-value test gaps without turning the task into a rewrite. | ||
|
|
||
| For risky code areas, see [RISKY-AREAS.md](RISKY-AREAS.md). | ||
|
|
||
| ## Step 1: Collect File Set | ||
|
|
||
| Sources: files changed in target PR, recent failing CI, `main` churn in hotspot area, hotspot list in state file. If unknown, derive from highest-ranked actionable PRs. | ||
|
|
||
| ## Step 2: Map to Existing Tests | ||
|
|
||
| Repo conventions: | ||
|
|
||
| - Root integration tests (`test/`): ESM imports | ||
| - Plugin tests: co-located as `*.test.ts` | ||
| - Shell logic: may need extraction into testable helper first | ||
|
|
||
| For each risky file: is there a test covering the changed behavior? Is it too indirect or flaky? Can a small extraction improve testability? | ||
|
|
||
| ## Step 3: Choose Highest-Value Tests | ||
|
|
||
| Prefer tests that catch regressions the maintainer loop actually sees: | ||
|
|
||
| - invalid/boundary env values, shell quoting, retry/timeout behavior | ||
| - missing/malformed config, denied network/policy paths | ||
| - duplicate workflow/hook behavior, version/tag/DCO edge cases | ||
| - unauthorized or unsafe inputs | ||
|
|
||
| Include at least one negative-path test for risky code. | ||
|
|
||
| ## Step 4: Extract Narrow Seams If Needed | ||
|
|
||
| Smallest extraction that improves testability: move parsing into a pure helper, separate construction from execution, factor hook logic into a reusable function, replace bags of primitives with typed helpers. | ||
|
|
||
| Do not broad-refactor under the label of "adding tests." | ||
|
|
||
| ## Step 5: Add Tests | ||
|
|
||
| - CLI tests → `test/` | ||
| - Plugin tests → `nemoclaw/src/` | ||
| - TypeScript helpers → TypeScript tests | ||
| - Mock external systems; no real API calls in unit tests | ||
| - Security paths: prove the unsafe action is denied, not just that happy path works | ||
|
|
||
| ## Step 6: Validate | ||
|
|
||
| ```bash | ||
| npm test # root tests | ||
| cd nemoclaw && npm test # plugin tests | ||
| npm run typecheck:cli | ||
| make check | ||
| ``` | ||
|
|
||
| Narrowest command set that gives confidence. | ||
|
|
||
| ## Step 7: Report Gaps Honestly | ||
|
|
||
| If untested risk remains, say so: no clean seam without larger refactor, too much hidden shell state, missing fixture strategy, flaky infra dependency. | ||
|
|
||
| ## Notes | ||
|
|
||
| - Tests for risky code are merge readiness, not polish. | ||
| - One precise regression test beats many vague integration checks. | ||
| - If credible fix needs bigger redesign, follow [SEQUENCE-WORK.md](SEQUENCE-WORK.md). |
There was a problem hiding this comment.
This generated skill markdown should be regenerated, not edited directly.
Please update the source docs and re-run scripts/docs-to-skills.py so this file is produced deterministically.
As per coding guidelines, "Files in .agents/skills/nemoclaw-/.md are autogenerated by scripts/docs-to-skills.py and must never be edited directly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/TEST-GAPS.md around lines 1 - 65, The
TEST-GAPS.md file is autogenerated and must not be edited directly; update the
original source documentation that feeds the generator and then run the
generator script to regenerate this skill file: modify the upstream docs (the
source that maps to TEST-GAPS content), then run scripts/docs-to-skills.py to
produce the updated .agents/skills/nemoclaw-maintainer-day/TEST-GAPS.md,
committing the source changes and the regenerated output rather than editing
TEST-GAPS.md by hand.
| ## Step 2: Bump Stragglers | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts <version> <next-version> | ||
| ``` | ||
|
|
||
| This creates the next version label if needed, then moves all open items from the current version to the next. Tell the user what got bumped. |
There was a problem hiding this comment.
Silent failure risk: bump-stragglers.ts does not surface errors.
Based on the relevant code snippets, bump-stragglers.ts uses run() which returns an empty string on failure. If label creation fails or PR/issue edits fail, the script continues silently and may leave items in a partially-bumped state. Consider adding guidance to verify the JSON output shows expected bumped items, or check stderr for failure messages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-evening/SKILL.md around lines 22 - 28,
bump-stragglers.ts can silently continue when run() returns an empty string, so
update the script to detect empty/invalid run() results and fail fast: after
each call to run() in bump-stragglers.ts validate the return (and any parsed
JSON) and if missing or malformed (e.g. no "bumped" array) write a clear error
to stderr and exit non-zero; also ensure any label-creation or PR/issue-editing
failures from run() are surfaced (log the error output and include it in the
process exit) and add a final check that the emitted JSON contains the expected
"bumped" items before reporting success.
| --- | ||
| name: nemoclaw-maintainer-morning | ||
| description: Runs the morning maintainer standup for NemoClaw. Triages the backlog, determines the day's target version, labels selected items, surfaces stragglers from previous versions, and outputs the daily plan. Use at the start of the workday. Trigger keywords - morning, standup, start of day, daily plan, what are we shipping today. | ||
| user_invocable: true | ||
| --- | ||
|
|
||
| # NemoClaw Maintainer Morning | ||
|
|
||
| Start the day: triage, pick a version target, label items, share the plan. | ||
|
|
||
| See [PR-REVIEW-PRIORITIES.md](../nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md) for the daily cadence and review priorities. | ||
|
|
||
| ## Step 1: Determine Target Version and Stragglers | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts | ||
| ``` | ||
|
|
||
| This fetches tags, computes the next patch version, and finds open items still carrying older version labels. Surface stragglers first — the team needs to decide: relabel to today's target, or defer further. | ||
|
|
||
| ## Step 2: Triage | ||
|
|
||
| Run the triage script to rank the full backlog: | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts --approved-only | ||
| ``` | ||
|
|
||
| If too few results, run without `--approved-only`. The script calls `gh-pr-merge-now --json`, enriches candidates with risky-area detection, and applies the scoring model documented in [PR-REVIEW-PRIORITIES.md](../nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md). | ||
|
|
||
| Also use `find-review-pr` to surface PRs with `security` + `priority: high` labels. Merge these into the candidate pool. | ||
|
|
||
| ## Step 3: Label Version Targets | ||
|
|
||
| Present the ranked queue to the user. After they confirm which items to target, label them: | ||
|
|
||
| ```bash | ||
| gh label create "<version>" --repo NVIDIA/NemoClaw --description "Release target" --color "1d76db" 2>/dev/null || true | ||
| gh pr edit <number> --repo NVIDIA/NemoClaw --add-label "<version>" | ||
| gh issue edit <number> --repo NVIDIA/NemoClaw --add-label "<version>" | ||
| ``` | ||
|
|
||
| ## Step 4: Save State and Output the Plan | ||
|
|
||
| Pipe triage output into state: | ||
|
|
||
| ```bash | ||
| node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts \ | ||
| | node --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-day/scripts/state.ts set-queue | ||
| ``` | ||
|
|
||
| Output the daily plan: | ||
|
|
||
| | Target | Item | Type | Owner | Next action | | ||
| |--------|------|------|-------|-------------| | ||
| | v0.0.8 | [#1234](https://github.com/NVIDIA/NemoClaw/pull/1234) | PR | @author | Run merge gate | | ||
| | v0.0.8 | [#1235](https://github.com/NVIDIA/NemoClaw/issues/1235) | Issue | unassigned | Needs PR | | ||
|
|
||
| Include: total items targeted, how many are PRs vs issues, how many are already merge-ready. | ||
|
|
||
| ## Notes | ||
|
|
||
| - This skill runs once at the start of the day. Use `/nemoclaw-maintainer-day` during the day to execute. | ||
| - The target version label is the source of truth for "what we're shipping today." | ||
| - Stragglers from previous versions should be addressed first — they already slipped once. |
There was a problem hiding this comment.
Autogenerated skill file is being edited directly.
This file should be generated from source docs, not manually maintained in-place.
As per coding guidelines, "Files in .agents/skills/nemoclaw-/.md are autogenerated by scripts/docs-to-skills.py and must never be edited directly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-morning/SKILL.md around lines 1 - 65, The
SKILL.md file (.agents/skills/nemoclaw-maintainer-morning/SKILL.md) is
autogenerated and must not be edited directly; revert any manual changes here
and update the authoritative source content or the generator instead (update the
original doc used by scripts/docs-to-skills.py or modify
scripts/docs-to-skills.py to produce the desired change), then re-run the
docs-to-skills.py generation to regenerate SKILL.md so the change persists.
| If too few results, run without `--approved-only`. The script calls `gh-pr-merge-now --json`, enriches candidates with risky-area detection, and applies the scoring model documented in [PR-REVIEW-PRIORITIES.md](../nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md). | ||
|
|
||
| Also use `find-review-pr` to surface PRs with `security` + `priority: high` labels. Merge these into the candidate pool. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether triage logic still references gh-pr-merge-now
rg -n -C2 "gh-pr-merge-now|gh pr list|--approved-only" \
.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts \
.agents/skills/nemoclaw-maintainer-morning/SKILL.mdRepository: NVIDIA/NemoClaw
Length of output: 2039
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if SKILL.md is truly autogenerated and find the source
echo "=== Checking docs-to-skills.py script ==="
cat -n scripts/docs-to-skills.py | head -50
echo -e "\n=== Looking for source markdown/docs files ==="
fd -e md . .agents/skills/nemoclaw-maintainer-day/ | grep -v SKILL.md | head -20
echo -e "\n=== Checking triage.ts to understand actual implementation ==="
sed -n '1,50p' .agents/skills/nemoclaw-maintainer-day/scripts/triage.tsRepository: NVIDIA/NemoClaw
Length of output: 5069
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get more context from triage.ts about what it actually does
echo "=== Full triage.ts to understand the implementation ==="
wc -l .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts
# Search for enrichment and scoring logic
rg -n "enrich|risky|score|candidate|approval" .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts | head -30Repository: NVIDIA/NemoClaw
Length of output: 1932
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the source markdown files in docs/ directory
echo "=== Looking for source files in docs/ ==="
find docs/ -type f -name "*.md" 2>/dev/null | head -20
# Search for references to gh-pr-merge-now or maintainer-morning
echo -e "\n=== Searching for gh-pr-merge-now in docs/ ==="
rg -n "gh-pr-merge-now" docs/ 2>/dev/null || echo "Not found in docs/"
# Search for triage-related content
echo -e "\n=== Searching for triage in docs/ ==="
rg -n "triage" docs/ -i 2>/dev/null | head -20
# Look for maintainer-morning references
echo -e "\n=== Searching for maintainer-morning context ==="
rg -n "approved-only|enriches candidates|risky-area" docs/ 2>/dev/null || echo "Not found"Repository: NVIDIA/NemoClaw
Length of output: 879
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let's check the structure of docs/ directory more thoroughly
echo "=== Full directory structure of docs/ ==="
fd . docs/ -t f 2>/dev/null || ls -la docs/ 2>/dev/null || echo "docs/ may not exist or is empty"
# Check if there's a specific directory structure
echo -e "\n=== Check if docs directory exists and list contents ==="
if [ -d docs/ ]; then
ls -la docs/
else
echo "docs/ directory does not exist"
fiRepository: NVIDIA/NemoClaw
Length of output: 3376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's a separate directory for these maintainer skills
echo "=== Check if there are other doc/skill sources ==="
find . -name "*.md" -path "*maintainer*" -type f 2>/dev/null | grep -v node_modules | head -20
# Look at the SKILL.md file itself for metadata/comments
echo -e "\n=== Check SKILL.md for metadata about its source ==="
head -50 .agents/skills/nemoclaw-maintainer-morning/SKILL.md
# Check if there's a README or documentation about how these are generated
echo -e "\n=== Check for generation instructions ==="
find . -name "README*" -o -name "GENERATION*" -o -name "*generation*" 2>/dev/null | grep -v node_modules | head -10Repository: NVIDIA/NemoClaw
Length of output: 3333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any script that might generate or write to nemoclaw-maintainer files
echo "=== Search for scripts that touch nemoclaw-maintainer files ==="
rg -n "nemoclaw-maintainer" scripts/ --type py --type sh --type ts 2>/dev/null | head -30
# Check what's in .agents/skills directory
echo -e "\n=== List all skill directories ==="
ls -d .agents/skills/*/ | head -20Repository: NVIDIA/NemoClaw
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if docs-to-skills.py would actually generate nemoclaw-maintainer files
echo "=== Checking docs-to-skills.py for how it names skills ==="
rg -n "nemoclaw-maintainer|maintainer-" scripts/docs-to-skills.py
# Look for any hardcoded skill generation
echo -e "\n=== Check if script has hardcoded nemoclaw-maintainer generation ==="
grep -n "nemoclaw-maintainer\|maintainer-morning\|maintainer-day" scripts/docs-to-skills.py || echo "Not found"
# Check git history to see if these files were ever auto-generated
echo -e "\n=== Check git log for these files ==="
git log --oneline -20 -- .agents/skills/nemoclaw-maintainer-morning/SKILL.md 2>/dev/null | head -5
# Check if there's any marker in the file indicating it's auto-generated
echo -e "\n=== Check for auto-generation markers ==="
head -10 .agents/skills/nemoclaw-maintainer-morning/SKILL.md | grep -i "autogen\|generated\|do not edit"Repository: NVIDIA/NemoClaw
Length of output: 117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify: does triage.ts actually call gh-pr-merge-now anywhere?
echo "=== Final verification: grep for gh-pr-merge-now in triage.ts ==="
grep -n "gh-pr-merge-now\|gh pr list\|merge.now" .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts || echo "NOT FOUND - confirmed"
# Show what triage actually does with gh cli
echo -e "\n=== What triage.ts actually calls with gh ==="
rg -n "run\(|execSync|execa|shell" .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts | grep -i "gh\|command" | head -15Repository: NVIDIA/NemoClaw
Length of output: 740
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# See the actual gh API calls in triage.ts
echo "=== Examine the run() function and gh API calls ==="
sed -n '119,145p' .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts
echo "---"
sed -n '175,200p' .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts
# Check shared.ts to see what the run() function does
echo -e "\n=== Check shared.ts for run() implementation ==="
grep -A 10 "^function run\|^const run\|^export.*run" .agents/skills/nemoclaw-maintainer-day/scripts/shared.ts | head -20Repository: NVIDIA/NemoClaw
Length of output: 2303
Fix stale documentation: SKILL.md incorrectly describes triage script behavior.
Line 29 states the script "calls gh-pr-merge-now --json", but triage.ts actually uses direct gh CLI commands: gh api --paginate to list PRs and gh pr view --json to enrich candidate data. Update the description to accurately reflect that triage.ts fetches PR data via gh REST API, enriches top candidates with review and CI status, detects risky files, and applies the scoring model documented in PR-REVIEW-PRIORITIES.md.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-morning/SKILL.md around lines 29 - 32,
Update the SKILL.md paragraph to reflect actual triage behavior: replace the
claim that the script "calls `gh-pr-merge-now --json`" with a statement that
triage.ts uses the GitHub CLI REST API (`gh api --paginate`) to list PRs and
enriches top candidates with `gh pr view --json` (review status, CI status),
performs risky-file detection, applies the scoring model from
PR-REVIEW-PRIORITIES.md, and also uses find-review-pr to add PRs labeled
`security` + `priority: high` into the candidate pool.
PRs with GitHub labels now score higher in triage: - "security" label: +15 - "priority: high" label: +10 This produces clear score tiers: security+high+risky (80) > high+risky (65) > unlabeled+risky (55) > unlabeled clean (35). Previously all review-ready PRs with risky files scored identical 55. Also fix stale return type in scoreItem and nextAction text. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts (2)
326-329:⚠️ Potential issue | 🟠 MajorRisky
salvage-nowitems are routed into review instead of salvage.When
bucket === "salvage-now", Line 328 rewritesnextActiontosecurity-sweep → review. That drops the salvage workflow for fixable-but-blocked PRs and sends the maintainer loop down the wrong path.Proposed fix
if (riskyFiles.length > 0 && bucket !== "blocked") { score += SCORE_SECURITY_ACTIONABLE; - nextAction = bucket === "merge-now" ? "security-sweep → merge-gate" : "security-sweep → review"; + nextAction = + bucket === "salvage-now" + ? "security-sweep → salvage-pr" + : bucket === "merge-now" + ? "security-sweep → merge-gate" + : "security-sweep → review"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts around lines 326 - 329, The current block treating riskyFiles sets nextAction to "security-sweep → review" for any non-"blocked" bucket, which incorrectly routes bucket === "salvage-now" into review; update the conditional in triage.ts so that when riskyFiles.length > 0 and bucket !== "blocked" you still add SCORE_SECURITY_ACTIONABLE to score but set nextAction based on bucket values: preserve the existing "security-sweep → merge-gate" for bucket === "merge-now", set "security-sweep → salvage" (or the project's salvage label) for bucket === "salvage-now", and otherwise set "security-sweep → review"; adjust the nextAction assignment near the riskyFiles check (reference variables: riskyFiles, bucket, nextAction, SCORE_SECURITY_ACTIONABLE).
167-169:⚠️ Potential issue | 🟠 Major
--approved-onlystill does not work.The pre-enrichment filter at Lines 167-169 can never match, and Line 398 now hardcodes
false, so the flag parsed on Line 390 is ignored entirely.triage.ts --approved-onlywill still emit unapproved PRs. If this option stays, it needs to filter after enriching all non-draft PRs for that mode.Also applies to: 390-398
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts around lines 167 - 169, The --approved-only flag is ignored because you filter by pr.reviewDecision before enrichment (pr.reviewDecision is only set during enrichment) and you also hardcoded false where the parsed flag is used; move the approved-only filtering to run after the enrichment step that populates reviewDecision (i.e., after the enrich/enrichPullRequests/enrichPRs call that processes non-draft PRs) and remove the hardcoded false so the code uses the parsed approvedOnly variable from argument parsing (the flag parsed around Line 390). Ensure the post-enrichment filter uses prs.filter(pr => pr.reviewDecision === "APPROVED") so --approved-only works correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts:
- Around line 406-414: The code currently only enriches the first enrichCount
(limit * 3) candidates and later only scores the first limit * 2 actionable
items, which biases results by fetch order; change the flow so all non-draft
candidates (the full candidates array) are enriched before
classification/scoring and ensure file-based scoring runs over the full set of
actionable PRs (not just a sliced subset). Specifically, remove or adjust the
enrichCount slicing and any subsequent slicing of actionable items so that
enrichPr is invoked for every item in candidates, classifyPr runs on the full
prs array as before, and the file-based scoring step processes all entries
produced by classifyPr (refer to candidates, enrichPr, enrichCount, classifyPr,
classified and the actionable/file-based scoring variables around the existing
limit-based slices).
- Around line 281-287: fetchPrFiles currently fetches only the first 100 PR
files and drops the rest; update fetchPrFiles to paginate like fetchOpenPrs by
calling ghApi with the --paginate option (or the same pagination helper used by
fetchOpenPrs), accumulate filenames across all pages, keep the return type
Array<{ filename: string }> | null handling, and return the concatenated list of
filename strings (preserve the existing Array.isArray check and empty-array
fallback).
---
Duplicate comments:
In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts:
- Around line 326-329: The current block treating riskyFiles sets nextAction to
"security-sweep → review" for any non-"blocked" bucket, which incorrectly routes
bucket === "salvage-now" into review; update the conditional in triage.ts so
that when riskyFiles.length > 0 and bucket !== "blocked" you still add
SCORE_SECURITY_ACTIONABLE to score but set nextAction based on bucket values:
preserve the existing "security-sweep → merge-gate" for bucket === "merge-now",
set "security-sweep → salvage" (or the project's salvage label) for bucket ===
"salvage-now", and otherwise set "security-sweep → review"; adjust the
nextAction assignment near the riskyFiles check (reference variables:
riskyFiles, bucket, nextAction, SCORE_SECURITY_ACTIONABLE).
- Around line 167-169: The --approved-only flag is ignored because you filter by
pr.reviewDecision before enrichment (pr.reviewDecision is only set during
enrichment) and you also hardcoded false where the parsed flag is used; move the
approved-only filtering to run after the enrichment step that populates
reviewDecision (i.e., after the enrich/enrichPullRequests/enrichPRs call that
processes non-draft PRs) and remove the hardcoded false so the code uses the
parsed approvedOnly variable from argument parsing (the flag parsed around Line
390). Ensure the post-enrichment filter uses prs.filter(pr => pr.reviewDecision
=== "APPROVED") so --approved-only works correctly.
🪄 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: CHILL
Plan: Pro
Run ID: f0bbe067-f9d4-419c-9d70-a7f360a3de03
📒 Files selected for processing (2)
.agents/skills/nemoclaw-maintainer-day/scripts/shared.ts.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/skills/nemoclaw-maintainer-day/scripts/shared.ts
| function fetchPrFiles(repo: string, number: number): string[] { | ||
| const data = ghApi(`repos/${repo}/pulls/${number}/files?per_page=100`) as | ||
| | Array<{ filename: string }> | ||
| | null; | ||
| if (!Array.isArray(data)) return []; | ||
| return data.map((f) => f.filename); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="${1:-NVIDIA/NemoClaw}"
gh api --paginate "repos/$repo/pulls?state=open&per_page=100" --jq '.[].number' |
while read -r pr; do
changed="$(gh pr view "$pr" --repo "$repo" --json changedFiles --jq '.changedFiles')"
if [ "$changed" -gt 100 ]; then
fetched="$(gh api "repos/$repo/pulls/$pr/files?per_page=100" | jq 'length')"
printf 'PR #%s changedFiles=%s single_page_files=%s\n' "$pr" "$changed" "$fetched"
fi
doneRepository: NVIDIA/NemoClaw
Length of output: 188
🏁 Script executed:
cat -n .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts | head -300 | tail -50Repository: NVIDIA/NemoClaw
Length of output: 1998
🏁 Script executed:
rg "function fetchOpenPrs" .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts -A 10Repository: NVIDIA/NemoClaw
Length of output: 574
🏁 Script executed:
rg "fetchPrFiles" .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts -B 3 -A 3Repository: NVIDIA/NemoClaw
Length of output: 771
🏁 Script executed:
rg "hotClusters|risky" .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts | head -20Repository: NVIDIA/NemoClaw
Length of output: 487
🏁 Script executed:
rg "per_page|paginate" .agents/skills/nemoclaw-maintainer-day/scripts/triage.tsRepository: NVIDIA/NemoClaw
Length of output: 377
🏁 Script executed:
grep -n "const data = ghApi" .agents/skills/nemoclaw-maintainer-day/scripts/triage.tsRepository: NVIDIA/NemoClaw
Length of output: 138
Add pagination to fetchPrFiles() to prevent file truncation.
fetchPrFiles() only requests ?per_page=100 without pagination. PRs with more than 100 changed files will silently lose later filenames, skewing risky-file detection and hotClusters scoring. The function should use --paginate like fetchOpenPrs() does.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts around lines 281 -
287, fetchPrFiles currently fetches only the first 100 PR files and drops the
rest; update fetchPrFiles to paginate like fetchOpenPrs by calling ghApi with
the --paginate option (or the same pagination helper used by fetchOpenPrs),
accumulate filenames across all pages, keep the return type Array<{ filename:
string }> | null handling, and return the concatenated list of filename strings
(preserve the existing Array.isArray check and empty-array fallback).
| const candidates = prs.filter((pr) => !pr.isDraft); | ||
| const enrichCount = Math.min(candidates.length, limit * 3); | ||
| process.stderr.write(`Enriching ${enrichCount} of ${candidates.length} candidates...\n`); | ||
| for (let i = 0; i < enrichCount; i++) { | ||
| enrichPr(repo, candidates[i]); | ||
| } | ||
|
|
||
| // 3. Classify all PRs (un-enriched ones will be blocked due to empty checks) | ||
| const classified = prs.map(classifyPr); |
There was a problem hiding this comment.
Actionable PR selection is truncated before scoring.
Only the first limit * 3 non-draft PRs are enriched, and then only the first limit * 2 actionable items are sent through file-based scoring. Ready PRs later in the REST listing never compete for the queue, so the output is driven by fetch order rather than actual readiness/score.
Also applies to: 432-434
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts around lines 406 -
414, The code currently only enriches the first enrichCount (limit * 3)
candidates and later only scores the first limit * 2 actionable items, which
biases results by fetch order; change the flow so all non-draft candidates (the
full candidates array) are enriched before classification/scoring and ensure
file-based scoring runs over the full set of actionable PRs (not just a sliced
subset). Specifically, remove or adjust the enrichCount slicing and any
subsequent slicing of actionable items so that enrichPr is invoked for every
item in candidates, classifyPr runs on the full prs array as before, and the
file-based scoring step processes all entries produced by classifyPr (refer to
candidates, enrichPr, enrichCount, classifyPr, classified and the
actionable/file-based scoring variables around the existing limit-based slices).
The CLAUDE.md guideline said all .agents/skills/nemoclaw-*/*.md is autogenerated and should never be edited directly. This is true for docs-derived skills but not for the hand-authored maintainer skills (morning/day/evening). CodeRabbit was flagging valid edits based on this overly broad rule. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the overly broad "nemoclaw-*/*.md is autogenerated" rule with an explicit list of the skills that docs-to-skills.py actually produces. All other skills (maintainer-*, cut-release-tag, etc.) are clearly documented as hand-authored. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Main replaced CLAUDE.md with a symlink to AGENTS.md and reorganized skills into audience buckets (nemoclaw-user-*, nemoclaw-maintainer-*, nemoclaw-contributor-*). The narrower autogenerated rule (nemoclaw-user-* only) is already in main's AGENTS.md, making our explicit-list change redundant. Accept main's version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add parseStringArg/parseIntArg helpers to shared.ts for safe CLI arg parsing with validation and fallback defaults. Update all 6 scripts. - Wrap JSON.parse in state.ts cmdSetQueue/cmdSetHotspots with try-catch for clear error on malformed piped input. - Fix cmdExclude docstring to match behavior (PR-only, not PR+issue). - Document enrichment cap (limit*3) as intentional in triage.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts:
- Around line 58-76: The empty catch around JSON.parse of prOut (in the block
that populates stragglers by iterating prs and labels) swallows parse errors;
change it to surface failures by capturing the parse error and rethrowing or
returning a rejected Error so the maintainer loop fails fast (e.g., throw new
Error with context including prOut and the original error). Apply the same
change to the corresponding issueOut parsing block (the catch that currently
ignores parse errors when building issue-based stragglers) so both parse
failures no longer produce silent empty/partial straggler lists.
- Around line 65-66: The code currently treats any semver label not equal to
targetVersion as a "straggler" (using label.name !== targetVersion), which
incorrectly includes future release labels; change the logic in
version-target.ts to parse label.name and targetVersion into semver tuples
(major, minor, patch) and compare them numerically so only labels strictly older
than targetVersion are pushed into stragglers; update both occurrences (the
block that pushes into stragglers and the similar check at lines referenced
92-93) to use a helper compareSemver or inline tuple comparison rather than
string inequality, ensuring future-version labels are excluded and only
older-version labels are collected for bump-stragglers.ts to consume.
- Around line 111-116: The script reads the --repo value via parseStringArg but
still resolves tags from the local git checkout using run("git", ["fetch",
"origin", "--tags", "--prune"]) and getLatestTag(); update the logic so the repo
flag actually controls version resolution: if a non-default --repo is provided,
query that remote repository for tags/releases (e.g., use the GitHub CLI gh
release list or gh api to list tags) and derive latestTag and targetVersion
(bumpPatch) from that output; otherwise keep the existing local run("git",
["fetch", ..."]) and getLatestTag() path. Refer to parseStringArg, run,
getLatestTag, and bumpPatch to locate and branch the code, or alternatively
remove the --repo flag and its uses if you decide to restrict the script to the
local checkout.
🪄 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: CHILL
Plan: Pro
Run ID: c4836786-827a-470e-81a1-1715ad2336dd
📒 Files selected for processing (8)
.agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts.agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts.agents/skills/nemoclaw-maintainer-day/scripts/shared.ts.agents/skills/nemoclaw-maintainer-day/scripts/state.ts.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts.agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts
✅ Files skipped from review due to trivial changes (1)
- .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts
- .agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts
- .agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts
- .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts
- .agents/skills/nemoclaw-maintainer-day/scripts/state.ts
- .agents/skills/nemoclaw-maintainer-day/scripts/shared.ts
| try { | ||
| const prs = JSON.parse(prOut) as Array<{ | ||
| number: number; title: string; url: string; | ||
| labels: Array<{ name: string }>; | ||
| }>; | ||
| for (const pr of prs) { | ||
| for (const label of pr.labels) { | ||
| if (/^v\d+\.\d+\.\d+$/.test(label.name) && label.name !== targetVersion) { | ||
| stragglers.push({ | ||
| number: pr.number, | ||
| title: pr.title, | ||
| url: pr.url, | ||
| type: "pr", | ||
| versionLabel: label.name, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } catch { /* ignore parse errors */ } |
There was a problem hiding this comment.
Don't turn gh parse failures into a clean scan.
Both empty catch {} blocks silently drop malformed gh output and continue, which can emit an empty or partial stragglers list even though the scan failed. This should fail fast so the maintainer loop can retry or alert.
Suggested change
- } catch { /* ignore parse errors */ }
+ } catch (error) {
+ throw new Error(`Failed to parse gh pr list output: ${(error as Error).message}`);
+ }Apply the same change to the issueOut block.
Also applies to: 85-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts around
lines 58 - 76, The empty catch around JSON.parse of prOut (in the block that
populates stragglers by iterating prs and labels) swallows parse errors; change
it to surface failures by capturing the parse error and rethrowing or returning
a rejected Error so the maintainer loop fails fast (e.g., throw new Error with
context including prOut and the original error). Apply the same change to the
corresponding issueOut parsing block (the catch that currently ignores parse
errors when building issue-based stragglers) so both parse failures no longer
produce silent empty/partial straggler lists.
.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts
Outdated
Show resolved
Hide resolved
… errors Three CodeRabbit findings on version-target.ts: - Future version labels (e.g. v0.0.11) were incorrectly flagged as stragglers. Now uses semver comparison (compareSemver < 0) to only catch labels older than the target. - Empty catch blocks silently hid gh parse failures. Now logs to stderr. - Documented that --repo controls gh queries while git tags always come from the local checkout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
/loop 10m /nemoclaw-maintainer-loopcontinuous operationgh-pr-merge-nowas data engine)Test plan
/nemoclaw-maintainer-loopand verify triage outputnode --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-loop/scripts/triage.ts --approved-onlynode --experimental-strip-types --no-warnings .agents/skills/nemoclaw-maintainer-loop/scripts/check-gates.ts <pr-number>prekvalidation (tested in pre-commit)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation