fix(intent): review stale skills against artifact coverage#120
fix(intent): review stale skills against artifact coverage#120LadyBluenotes merged 16 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughIntroduces artifact-based coverage tracking and signals generation to the Intent system. Adds a new Changes
Sequence DiagramsequenceDiagram
actor GitHub as GitHub Actions
participant Workflow as check-skills.yml
participant CLI as intent stale
participant Artifacts as _artifacts/
participant Review as workflow-review
participant API as GitHub API
GitHub->>Workflow: Trigger (on schedule/push)
Workflow->>CLI: Run intent stale --json
CLI->>Artifacts: readIntentArtifacts()
Artifacts-->>CLI: Parse YAML artifacts
CLI->>CLI: buildWorkspaceCoverageSignals()
CLI-->>Workflow: JSON report + signals
Workflow->>Review: collectStaleReviewItems()
Review->>Review: Merge skills + signals
Review-->>Workflow: Structured items array
Workflow->>Review: buildStaleReviewBody()
Review-->>Workflow: Markdown PR body
Workflow->>API: Check for existing PR
alt PR exists on branch
API-->>Workflow: Found PR
Workflow->>API: Update PR body
else No PR found
Workflow->>API: Create new PR
end
API-->>GitHub: PR created/updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit f655d04
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 74951e4
☁️ Nx Cloud last updated this comment at |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
packages/intent/tests/cli.test.ts (1)
1527-1568: Defensive: mockfetchhere too.Unlike the other four new tests in this block, this one omits the
fetchSpymock. It currently works because no package has skills and the staleness pipeline skips the npm probe, but that is an implementation detail. If a future change starts readinglibrary.name/library.versionfrom_artifactsto resolve the current version, this test will hit the real npm registry and become slow/flaky in CI.🧪 Suggested defensive mock
writeFileSync( join(root, '_artifacts', 'skill_tree.yaml'), [ 'library:', " name: '@tanstack/router'", " version: '1.0.0'", 'skills: []', ].join('\n'), ) + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({ version: '1.0.0' }), + } as Response) + process.chdir(root) @@ expect(reports[0]?.signals).toEqual([ expect.objectContaining({ type: 'missing-package-coverage', packageName: '@tanstack/react-start-rsc', }), ]) + + fetchSpy.mockRestore() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/tests/cli.test.ts` around lines 1527 - 1568, This test is missing the defensive npm network mock used by the other staleness tests; add the same fetch mock setup/teardown (e.g., create a fetchSpy that stubs global.fetch and returns a resolved Response) before calling main(['stale', '--json']) and restore it afterward so the test doesn't hit the real npm registry if the code starts resolving library.name/library.version; locate the pattern used in the sibling tests in packages/intent/tests/cli.test.ts and apply the same mocking around this specific it(...) block.packages/intent/tests/stale-command.test.ts (1)
101-113: Import the workflow-version constant instead of hardcoding version numbers.The test assertions use hardcoded values
1(old) and2(current), which are tied to the exportedINTENT_CHECK_SKILLS_WORKFLOW_VERSIONconstant. When that constant is bumped, the "current version" test will silently fail (since it checks for an exact value) while the "old version" test can pass for the wrong reason (any value below current still reads as old). Importing the constant makes the tests resilient to future bumps and documents the intent explicitly.🔧 Suggested change
-import { getCheckSkillsWorkflowAdvisories } from '../src/cli-support.js' +import { + INTENT_CHECK_SKILLS_WORKFLOW_VERSION, + getCheckSkillsWorkflowAdvisories, +} from '../src/cli-support.js'it('advises when the workflow has an old intent version stamp', () => { - const root = writeWorkflow('# intent-workflow-version: 1\n') + const root = writeWorkflow( + `# intent-workflow-version: ${INTENT_CHECK_SKILLS_WORKFLOW_VERSION - 1}\n`, + ) @@ it('does not advise when the workflow has the current version stamp', () => { - const root = writeWorkflow('# intent-workflow-version: 2\n') + const root = writeWorkflow( + `# intent-workflow-version: ${INTENT_CHECK_SKILLS_WORKFLOW_VERSION}\n`, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/tests/stale-command.test.ts` around lines 101 - 113, Replace the hardcoded version numbers in the tests with the exported INTENT_CHECK_SKILLS_WORKFLOW_VERSION constant: import INTENT_CHECK_SKILLS_WORKFLOW_VERSION at the top of the test file and use INTENT_CHECK_SKILLS_WORKFLOW_VERSION for the "current version" case and INTENT_CHECK_SKILLS_WORKFLOW_VERSION - 1 (or Math.max(0, ... - 1) if needed) for the "old version" case when calling writeWorkflow; keep the existing assertions that call getCheckSkillsWorkflowAdvisories so behavior remains the same but resilient to future bumps of the constant.packages/intent/meta/templates/workflows/check-skills.yml (1)
55-107: Inline Node script duplicates and has already drifted fromworkflow-review.ts.This template re-implements
collectStaleReviewItems/createFailedStaleReviewItem, but with subtle divergences from the exported helpers inpackages/intent/src/workflow-review.ts:
- Line 87-92 omits
signal?.subjectfrom the subject fallback chain (present atworkflow-review.ts:40). Any signal that populates onlysubject(e.g. theartifact-parse-warningsignal whosesubjectiswarning.artifactPath) will fall through toreport.libraryhere even though the CLI and the helper would show the artifact path.- Line 93 references
signal?.message, butStalenessSignal(seepackages/intent/src/types.ts:89-99) has nomessagefield — this fallback is dead and misleading.- Lines 55-64 hardcode the same shape that
createFailedStaleReviewItemalready returns.Since
@tanstack/intentis installed globally on line 40, the workflow can call the exported helpers directly and remove the drift risk entirely:♻️ Sketch of the refactor
if [ "$STATUS" -ne 0 ]; then echo "has_review=true" >> "$GITHUB_OUTPUT" echo "check_failed=true" >> "$GITHUB_OUTPUT" - cat > review-items.json <<'JSON' - [ - { - "type": "stale-check-failed", - "library": "{{PACKAGE_LABEL}}", - "subject": "intent stale --json", - "reasons": ["The stale check command failed. Review the workflow logs before updating skills."] - } - ] - JSON + node <<'NODE' + const fs = require('fs') + const { createFailedStaleReviewItem } = require('@tanstack/intent') + fs.writeFileSync( + 'review-items.json', + JSON.stringify([createFailedStaleReviewItem('{{PACKAGE_LABEL}}')], null, 2) + '\n', + ) + NODE else node <<'NODE' const fs = require('fs') - const reports = JSON.parse(fs.readFileSync('stale.json', 'utf8')) - const items = [] - - for (const report of reports) { - for (const skill of report.skills ?? []) { - /* …inline duplication… */ - } - for (const signal of report.signals ?? []) { - /* …inline duplication with drift… */ - } - } + const { collectStaleReviewItems } = require('@tanstack/intent') + const reports = JSON.parse(fs.readFileSync('stale.json', 'utf8')) + const items = collectStaleReviewItems(reports) fs.writeFileSync('review-items.json', JSON.stringify(items, null, 2) + '\n')The "Build review PR body" step (lines 128-218) can likewise defer to
buildStaleReviewBody(items)instead of hand-rollingsignalRows/itemRows/prompt.If keeping the logic inlined is intentional (e.g. to decouple the workflow from the CLI's JS API surface), at minimum please restore the missing
signal?.subjectfallback on lines 87-92 and drop the deadsignal?.messagereference on line 93 so the inline version matches the helper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/meta/templates/workflows/check-skills.yml` around lines 55 - 107, The inline Node script duplicates and has drifted from the exported helpers; either replace the inline logic with calls to the package helpers (import and call collectStaleReviewItems and createFailedStaleReviewItem from packages/intent/src/workflow-review.ts and use buildStaleReviewBody for the PR body) so the workflow defers to the canonical implementations, or if you must keep the inline script, restore the exact fallbacks from workflow-review.ts by adding signal?.subject into the subject fallback chain and remove the dead signal?.message fallback so the reasons use signal?.reasons only; reference the functions collectStaleReviewItems, createFailedStaleReviewItem and buildStaleReviewBody to locate the authoritative logic.packages/intent/src/commands/stale.ts (1)
49-58: Consider centralizing the signal-subject fallback.The fallback here is
subject → packageName → packageRoot → skill → artifactPath → type, butcollectStaleReviewItemsinpackages/intent/src/workflow-review.tsusespackageName → packageRoot → skill → artifactPath → subject → library, and the inline Node script inpackages/intent/meta/templates/workflows/check-skills.ymluses yet another order (and dropssubjectentirely). Three different orderings for the same derivation will inevitably drift. Consider extracting a sharedresolveSignalSubject(signal, fallback)helper and reusing it from bothstale.tsandworkflow-review.ts(and having the workflow template call those helpers, see the separate comment oncheck-skills.yml).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/src/commands/stale.ts` around lines 49 - 58, Multiple places derive a signal "subject" with different fallback orders (the loop in stale.ts and collectStaleReviewItems in workflow-review.ts plus the inline Node script), causing inconsistent results; extract a single helper (e.g., resolveSignalSubject(signal, fallbackOrder?)) that implements the canonical fallback order and replace the inline fallback logic in the for loop in stale.ts and in collectStaleReviewItems to call resolveSignalSubject, and update the workflow template to call the same helper (or mirror its order) so all consumers use the same ordering (reference resolveSignalSubject, the for-loop over signals in stale.ts, and collectStaleReviewItems).packages/intent/src/cli-support.ts (2)
95-99: Redundant sub-condition on Line 98.With
!isWorkspaceRootTargetnow gating entry,resolvedRoot !== context.workspaceRootis tautological: eitherworkspaceRootisnull(soresolvedRoot !== nulltrivially holds) or it's non-null and!isWorkspaceRootTargetalready implies inequality. The(targetSkillsDir !== null || …)disjunction therefore always evaluates true. The outer condition collapses tocontext.packageRoot && !isWorkspaceRootTarget.♻️ Proposed simplification
- if ( - context.packageRoot && - !isWorkspaceRootTarget && - (context.targetSkillsDir !== null || resolvedRoot !== context.workspaceRoot) - ) { + if (context.packageRoot && !isWorkspaceRootTarget) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/src/cli-support.ts` around lines 95 - 99, The if condition in the block using context.packageRoot, isWorkspaceRootTarget, context.targetSkillsDir, resolvedRoot and context.workspaceRoot contains a redundant disjunction; simplify the guard to just check context.packageRoot && !isWorkspaceRootTarget by removing the "(context.targetSkillsDir !== null || resolvedRoot !== context.workspaceRoot)" part so the conditional logic is clearer and equivalent under the current semantics (update the if expression where these symbols are used).
132-146: Attaching workspace-level coverage signals toreports[0]conflates contexts.When
packageDirsWithSkillsis non-empty,missing-package-coveragesignals (describing uncovered packages B, C, …) are appended to the first package's report (library A). Consumers grouping signals by the enclosing report'slibrarywill misattribute them; callers that iteratereport.signalsbut usereport.libraryfor display/headers will mislabel these entries.Prefer always emitting a dedicated workspace-level synthetic report for coverage signals (or dispatch each signal to a report keyed by
signal.library) so per-package reports only carry signals intrinsic to that package.♻️ Proposed fix — always emit a dedicated workspace report
- if (coverageSignals.length > 0) { - const workspaceReport = reports[0] - if (workspaceReport) { - workspaceReport.signals.push(...coverageSignals) - } else { - reports.push({ - library: relative(process.cwd(), workspaceRoot) || 'workspace', - currentVersion: null, - skillVersion: null, - versionDrift: null, - skills: [], - signals: coverageSignals, - }) - } - } + if (coverageSignals.length > 0) { + reports.push({ + library: relative(process.cwd(), workspaceRoot) || 'workspace', + currentVersion: null, + skillVersion: null, + versionDrift: null, + skills: [], + signals: coverageSignals, + }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/src/cli-support.ts` around lines 132 - 146, The current logic appends workspace-level coverageSignals to reports[0] (via workspaceReport.signals.push(...coverageSignals)) which misattributes workspace "missing-package-coverage" signals to the first package; instead always create and push a dedicated synthetic workspace-level report when coverageSignals.length > 0 (use the same shape used in the else branch) rather than mutating an existing per-package report, or alternatively distribute each signal to the correct per-package report keyed by signal.library; update the code paths in cli-support.ts around coverageSignals, reports and workspaceReport so workspace-level signals are represented by their own report and not mixed into package reports (also consider packageDirsWithSkills usage).packages/intent/src/staleness.ts (1)
178-192: Nit: redundant check on Line 185.
relPackageDiris always a string (relative(...).split(sep).join('/')), so!relPackageDiralready covers the empty-string case — the|| relPackageDir === ''branch is dead. Minor cleanup only.♻️ Proposed tidy-up
- const relPackageDir = relative(artifactRoot, packageDir).split(sep).join('/') - if (!relPackageDir || relPackageDir === '') return true + const relPackageDir = relative(artifactRoot, packageDir).split(sep).join('/') + if (!relPackageDir) return true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/intent/src/staleness.ts` around lines 178 - 192, In artifactPackageMatches, remove the redundant relPackageDir === '' check because relPackageDir is always a string and the existing !relPackageDir branch already covers the empty-string case; update the conditional that currently reads if (!relPackageDir || relPackageDir === '') return true to just if (!relPackageDir) return true so behavior is unchanged but the dead branch is removed (references: function artifactPackageMatches, variable relPackageDir).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/intent/meta/templates/workflows/check-skills.yml`:
- Around line 241-249: PR_URL is getting set to the literal string "null" when
no PR exists because the jq expression returns null, so the [ -n "$PR_URL" ]
check passes and gh pr edit is called; update the PR_URL assignment that uses gh
pr list/--jq so that it yields an empty string instead of "null" when no PR
exists (e.g., use jq's null-coalescing to emit empty), and keep the existing
conditional that tests PR_URL before calling gh pr edit/create (symbols to
change: PR_URL variable assignment and the gh pr list --head ... --json url --jq
expression).
In `@packages/intent/src/staleness.ts`:
- Around line 283-302: The computed subject (currently set via subject =
artifact.slug ?? artifact.name ?? artifact.path) can be undefined because
IntentArtifactSkill.slug/name/path are optional; change the fallback to a
guaranteed identifier (e.g., artifact.artifactPath) so subject is always a
string before you call findMatchingSkill and before emitting signals in
signals.push; update the subject declaration and any places that emit signals
(reference: subject, artifact.slug, artifact.name, artifact.path,
artifact.artifactPath, findMatchingSkill, and the signals.push block) to use the
new fallback and ensure the emitted signal.subject is never undefined.
In `@scripts/create-github-release.mjs`:
- Around line 207-224: The branch in createReleaseTag that returns a v<version>
tag when versions.length === 1 is dead/untested; either remove that shortcut so
createReleaseTag always produces the time-based tag (using changedPackages and
now-based slices) for consistent release names, or explicitly gate the
v<version> behavior behind a named option (e.g., perPackageRelease flag) and add
unit tests for both paths; update references to changedPackages/versions and any
callers to pass the new option if you choose the gated approach.
---
Nitpick comments:
In `@packages/intent/meta/templates/workflows/check-skills.yml`:
- Around line 55-107: The inline Node script duplicates and has drifted from the
exported helpers; either replace the inline logic with calls to the package
helpers (import and call collectStaleReviewItems and createFailedStaleReviewItem
from packages/intent/src/workflow-review.ts and use buildStaleReviewBody for the
PR body) so the workflow defers to the canonical implementations, or if you must
keep the inline script, restore the exact fallbacks from workflow-review.ts by
adding signal?.subject into the subject fallback chain and remove the dead
signal?.message fallback so the reasons use signal?.reasons only; reference the
functions collectStaleReviewItems, createFailedStaleReviewItem and
buildStaleReviewBody to locate the authoritative logic.
In `@packages/intent/src/cli-support.ts`:
- Around line 95-99: The if condition in the block using context.packageRoot,
isWorkspaceRootTarget, context.targetSkillsDir, resolvedRoot and
context.workspaceRoot contains a redundant disjunction; simplify the guard to
just check context.packageRoot && !isWorkspaceRootTarget by removing the
"(context.targetSkillsDir !== null || resolvedRoot !== context.workspaceRoot)"
part so the conditional logic is clearer and equivalent under the current
semantics (update the if expression where these symbols are used).
- Around line 132-146: The current logic appends workspace-level coverageSignals
to reports[0] (via workspaceReport.signals.push(...coverageSignals)) which
misattributes workspace "missing-package-coverage" signals to the first package;
instead always create and push a dedicated synthetic workspace-level report when
coverageSignals.length > 0 (use the same shape used in the else branch) rather
than mutating an existing per-package report, or alternatively distribute each
signal to the correct per-package report keyed by signal.library; update the
code paths in cli-support.ts around coverageSignals, reports and workspaceReport
so workspace-level signals are represented by their own report and not mixed
into package reports (also consider packageDirsWithSkills usage).
In `@packages/intent/src/commands/stale.ts`:
- Around line 49-58: Multiple places derive a signal "subject" with different
fallback orders (the loop in stale.ts and collectStaleReviewItems in
workflow-review.ts plus the inline Node script), causing inconsistent results;
extract a single helper (e.g., resolveSignalSubject(signal, fallbackOrder?))
that implements the canonical fallback order and replace the inline fallback
logic in the for loop in stale.ts and in collectStaleReviewItems to call
resolveSignalSubject, and update the workflow template to call the same helper
(or mirror its order) so all consumers use the same ordering (reference
resolveSignalSubject, the for-loop over signals in stale.ts, and
collectStaleReviewItems).
In `@packages/intent/src/staleness.ts`:
- Around line 178-192: In artifactPackageMatches, remove the redundant
relPackageDir === '' check because relPackageDir is always a string and the
existing !relPackageDir branch already covers the empty-string case; update the
conditional that currently reads if (!relPackageDir || relPackageDir === '')
return true to just if (!relPackageDir) return true so behavior is unchanged but
the dead branch is removed (references: function artifactPackageMatches,
variable relPackageDir).
In `@packages/intent/tests/cli.test.ts`:
- Around line 1527-1568: This test is missing the defensive npm network mock
used by the other staleness tests; add the same fetch mock setup/teardown (e.g.,
create a fetchSpy that stubs global.fetch and returns a resolved Response)
before calling main(['stale', '--json']) and restore it afterward so the test
doesn't hit the real npm registry if the code starts resolving
library.name/library.version; locate the pattern used in the sibling tests in
packages/intent/tests/cli.test.ts and apply the same mocking around this
specific it(...) block.
In `@packages/intent/tests/stale-command.test.ts`:
- Around line 101-113: Replace the hardcoded version numbers in the tests with
the exported INTENT_CHECK_SKILLS_WORKFLOW_VERSION constant: import
INTENT_CHECK_SKILLS_WORKFLOW_VERSION at the top of the test file and use
INTENT_CHECK_SKILLS_WORKFLOW_VERSION for the "current version" case and
INTENT_CHECK_SKILLS_WORKFLOW_VERSION - 1 (or Math.max(0, ... - 1) if needed) for
the "old version" case when calling writeWorkflow; keep the existing assertions
that call getCheckSkillsWorkflowAdvisories so behavior remains the same but
resilient to future bumps of the constant.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb32aed2-5d34-492e-b2db-0fb738b10210
📒 Files selected for processing (20)
.changeset/ten-shrimps-bow.md.codexpackages/intent/meta/templates/workflows/check-skills.ymlpackages/intent/meta/templates/workflows/notify-intent.ymlpackages/intent/src/artifact-coverage.tspackages/intent/src/cli-support.tspackages/intent/src/commands/stale.tspackages/intent/src/index.tspackages/intent/src/staleness.tspackages/intent/src/types.tspackages/intent/src/workflow-review.tspackages/intent/src/workspace-patterns.tspackages/intent/tests/artifact-coverage.test.tspackages/intent/tests/cli.test.tspackages/intent/tests/setup.test.tspackages/intent/tests/stale-command.test.tspackages/intent/tests/staleness.test.tspackages/intent/tests/workflow-review.test.tspackages/intent/tests/workspace-patterns.test.tsscripts/create-github-release.mjs
💤 Files with no reviewable changes (1)
- packages/intent/meta/templates/workflows/notify-intent.yml
| PR_URL="$(gh pr list --head "$BRANCH" --json url --jq '.[0].url')" | ||
| if [ -n "$PR_URL" ]; then | ||
| gh pr edit "$PR_URL" --body-file pr-body.md | ||
| else | ||
| gh pr create \ | ||
| --title "Review intent skills (${VERSION})" \ | ||
| --body-file pr-body.md \ | ||
| --head "$BRANCH" \ | ||
| --base "$BASE_BRANCH" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
gh CLI --jq empty array null output behavior .[0].url
💡 Result:
When using gh CLI with --jq '.[0].url' on a command that returns an empty JSON array (e.g., gh pr list --json url --jq '.[0].url' when no PRs match), the output is null. This follows standard jq behavior: accessing .[0] on an empty array [] returns null, as there is no element at index 0. gh CLI uses an embedded jq implementation (gojq), which matches standard jq for this case. Official documentation confirms --jq processes JSON output using jq syntax (https://cli.github.com/manual/gh_help_formatting). Past bugs with empty arrays producing null unexpectedly (e.g., [] | unique) were fixed by updating gojq, but direct indexing like .[0] correctly yields null. To safely get the first URL or empty output, use: --jq '.[0].url // empty' or check length first: --jq 'if length > 0 then .[0].url else empty end'.
Citations:
- 1: https://cli.github.com/manual/gh_help_formatting
- 2: jq
uniqueproduces null from empty arrays cli/cli#7001 - 3:
gh cache list --jsonshould output[]when no caches exist cli/cli#9882 - 4: Bug: Embedded jq parser does not handle empty arrays as expected Kong/kongctl#456
PR_URL becomes the literal string "null" when no PR exists, causing gh pr edit null to fail.
When gh pr list --head "$BRANCH" --json url --jq '.[0].url' finds no matching PRs, it returns an empty JSON array. The jq expression .[0].url on an empty array evaluates to jq's null value, which the gh CLI (via its embedded jq implementation) outputs as the literal string null. This causes PR_URL="null" (a non-empty string), making the condition [ -n "$PR_URL" ] true. The workflow then attempts gh pr edit null --body-file pr-body.md, which fails instead of falling through to create a new PR.
Use --jq '.[0].url // empty' to output an empty string when no PR exists:
- PR_URL="$(gh pr list --head "$BRANCH" --json url --jq '.[0].url')"
+ PR_URL="$(gh pr list --head "$BRANCH" --json url --jq '.[0].url // empty')"
if [ -n "$PR_URL" ]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PR_URL="$(gh pr list --head "$BRANCH" --json url --jq '.[0].url')" | |
| if [ -n "$PR_URL" ]; then | |
| gh pr edit "$PR_URL" --body-file pr-body.md | |
| else | |
| gh pr create \ | |
| --title "Review intent skills (${VERSION})" \ | |
| --body-file pr-body.md \ | |
| --head "$BRANCH" \ | |
| --base "$BASE_BRANCH" | |
| PR_URL="$(gh pr list --head "$BRANCH" --json url --jq '.[0].url // empty')" | |
| if [ -n "$PR_URL" ]; then | |
| gh pr edit "$PR_URL" --body-file pr-body.md | |
| else | |
| gh pr create \ | |
| --title "Review intent skills (${VERSION})" \ | |
| --body-file pr-body.md \ | |
| --head "$BRANCH" \ | |
| --base "$BASE_BRANCH" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/intent/meta/templates/workflows/check-skills.yml` around lines 241 -
249, PR_URL is getting set to the literal string "null" when no PR exists
because the jq expression returns null, so the [ -n "$PR_URL" ] check passes and
gh pr edit is called; update the PR_URL assignment that uses gh pr list/--jq so
that it yields an empty string instead of "null" when no PR exists (e.g., use
jq's null-coalescing to emit empty), and keep the existing conditional that
tests PR_URL before calling gh pr edit/create (symbols to change: PR_URL
variable assignment and the gh pr list --head ... --json url --jq expression).
| const subject = artifact.slug ?? artifact.name ?? artifact.path | ||
| const matchingSkill = findMatchingSkill( | ||
| artifact, | ||
| skillMetas, | ||
| packageDir, | ||
| artifactRoot, | ||
| ) | ||
|
|
||
| if (artifact.path && !matchingSkill) { | ||
| signals.push({ | ||
| type: 'artifact-skill-missing', | ||
| library, | ||
| subject, | ||
| reasons: [ | ||
| `artifact skill path does not resolve to a generated SKILL.md (${artifact.path})`, | ||
| ], | ||
| needsReview: true, | ||
| artifactPath: artifact.artifactPath, | ||
| skill: artifact.slug ?? artifact.name, | ||
| }) |
There was a problem hiding this comment.
subject can be undefined when artifact lacks slug, name, and path.
IntentArtifactSkill.slug/name/path are all optional (per types.ts), so artifact.slug ?? artifact.name ?? artifact.path can resolve to undefined and be embedded in emitted signals. Downstream renderers/groupers that assume a string subject may produce "undefined" output or mis-key aggregates. Add a safe fallback (e.g. artifactPath) so every signal has an identifiable subject.
🛡️ Proposed fallback
- const subject = artifact.slug ?? artifact.name ?? artifact.path
+ const subject =
+ artifact.slug ?? artifact.name ?? artifact.path ?? artifact.artifactPath🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/intent/src/staleness.ts` around lines 283 - 302, The computed
subject (currently set via subject = artifact.slug ?? artifact.name ??
artifact.path) can be undefined because IntentArtifactSkill.slug/name/path are
optional; change the fallback to a guaranteed identifier (e.g.,
artifact.artifactPath) so subject is always a string before you call
findMatchingSkill and before emitting signals in signals.push; update the
subject declaration and any places that emit signals (reference: subject,
artifact.slug, artifact.name, artifact.path, artifact.artifactPath,
findMatchingSkill, and the signals.push block) to use the new fallback and
ensure the emitted signal.subject is never undefined.
| function createReleaseTag(changedPackages) { | ||
| const versions = [...new Set(changedPackages.map((pkg) => pkg.version))] | ||
| if (versions.length === 1) { | ||
| const version = versions[0] | ||
| return { | ||
| tag: `v${version}`, | ||
| title: `v${version}`, | ||
| } | ||
| } | ||
|
|
||
| const now = new Date().toISOString() | ||
| const tag = `release-${now.slice(0, 10)}-${now.slice(11, 13)}${now.slice(14, 16)}` | ||
| const title = `Release ${now.slice(0, 10)} ${now.slice(11, 16)}` | ||
| const title = `Release ${changedPackages | ||
| .map((pkg) => `${pkg.name}@${pkg.version}`) | ||
| .join(', ')}` | ||
|
|
||
| return { tag, title } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack release tag convention v<version> vs per-package
💡 Result:
TanStack uses a monorepo structure with multiple packages (e.g., @tanstack/react-query, @tanstack/vue-query). They use Changesets for version management, where contributors add changeset files describing changes and bump types (major/minor/patch). A "Version Packages" PR is automatically created/updates package.json versions and changelogs based on changesets. When merged, packages are published to npm automatically. GitHub tags are created per-package in the format @tanstack/package-name@Version (e.g., @tanstack/vue-query@5.96.1, @tanstack/solid-query@5.96.1), not a single v tag for the monorepo. GitHub Releases use descriptive names like release-2026-04-11-1610 listing all updated packages and their versions (e.g., @tanstack/react-query@5.98.0). This per-package tagging supports independent-yet-coordinated versioning via Changesets fixed groups/internal deps (e.g., adapters bump with core), avoiding a single monorepo vX.Y.Z tag which wouldn't reflect per-package semver.
Citations:
- 1: https://github.com/TanStack/query/releases/tag/release-2026-04-11-1610
- 2: https://deepwiki.com/TanStack/query/8.5-release-process
- 3: https://github.com/TanStack/query/tags
- 4: https://github.com/tanstack/query/releases
- 5: ci: introduce changesets query#9502
🏁 Script executed:
cat -n scripts/create-github-release.mjs | sed -n '200,230p'Repository: TanStack/intent
Length of output: 1044
🏁 Script executed:
# Also check how the tag is used downstream
rg -A 10 -B 5 "createReleaseTag" scripts/create-github-release.mjsRepository: TanStack/intent
Length of output: 747
🏁 Script executed:
# Check for any other tag creation or Git tag operations
rg "tag.*v\d|@tanstack.*@" scripts/create-github-release.mjsRepository: TanStack/intent
Length of output: 41
🏁 Script executed:
# Find the createGitHubRelease function and see how it uses the tag
rg -A 30 "function createGitHubRelease|const createGitHubRelease" scripts/create-github-release.mjsRepository: TanStack/intent
Length of output: 654
🏁 Script executed:
# Look for pushTag function
rg -A 15 "function pushTag|const pushTag" scripts/create-github-release.mjsRepository: TanStack/intent
Length of output: 384
🏁 Script executed:
# Search for where the tag parameter is used in the file
rg "pushTag\(|gh release create" scripts/create-github-release.mjsRepository: TanStack/intent
Length of output: 112
🏁 Script executed:
# Look at the main entry point and how createGitHubRelease is called
rg -B 10 -A 5 "createdTag = pushTag|createGitHubRelease\(" scripts/create-github-release.mjs | head -50Repository: TanStack/intent
Length of output: 669
🏁 Script executed:
# Get broader context around the main release creation logic
rg -B 5 -A 20 "createdTag\s*=" scripts/create-github-release.mjsRepository: TanStack/intent
Length of output: 322
🏁 Script executed:
# Check if this is actually TanStack/intent or TanStack/query
git config --get remote.origin.url 2>/dev/null || git remote -v | head -1Repository: TanStack/intent
Length of output: 97
🏁 Script executed:
# Check the package.json to see what project this is
head -20 package.jsonRepository: TanStack/intent
Length of output: 956
🏁 Script executed:
# Look for any changelog or docs that explain the versioning scheme
find . -name "CHANGELOG*" -o -name "release*.md" -o -name ".github/workflows/*release*" | head -5Repository: TanStack/intent
Length of output: 437
🏁 Script executed:
# Check the actual CHANGELOG to see what tags are used
head -100 packages/intent/CHANGELOG.mdRepository: TanStack/intent
Length of output: 8233
🏁 Script executed:
# Check git tags to see actual tag format
git tag | head -20Repository: TanStack/intent
Length of output: 41
🏁 Script executed:
# Check actual GitHub releases for this repo
rg "release|tag|@" .github/ -t md 2>/dev/null | head -20Repository: TanStack/intent
Length of output: 143
🏁 Script executed:
# Check git history of the create-github-release.mjs file
git log --oneline scripts/create-github-release.mjs 2>/dev/null | head -10Repository: TanStack/intent
Length of output: 92
🏁 Script executed:
# Check if there are any actual releases yet (check .git/refs/tags or git ls-remote)
git ls-remote --tags origin 2>/dev/null | head -20Repository: TanStack/intent
Length of output: 364
🏁 Script executed:
# Check the .github/workflows to see current release process
find .github/workflows -name "*release*" -o -name "*publish*" | xargs cat 2>/dev/null | head -100Repository: TanStack/intent
Length of output: 3309
The v<version> tag branch appears to be dead code or untested; actual releases use time-based tags.
All releases in this repository use release-YYYY-MM-DD-HHMM tags (e.g., release-2026-03-16-1953), not v<version> tags. The code branch returning v<version> (lines 209–214) when versions.length === 1 is either unreachable or never exercised in practice. Since @tanstack/intent is a single-package publish, multiple distinct versions across packages are unlikely, meaning releases consistently hit the time-based branch instead.
If this branch is intended for future use (e.g., supporting per-package releases), clarify the intent and add safeguards. Otherwise, simplify by removing the single-version shortcut and always using the time-based scheme for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/create-github-release.mjs` around lines 207 - 224, The branch in
createReleaseTag that returns a v<version> tag when versions.length === 1 is
dead/untested; either remove that shortcut so createReleaseTag always produces
the time-based tag (using changedPackages and now-based slices) for consistent
release names, or explicitly gate the v<version> behavior behind a named option
(e.g., perPackageRelease flag) and add unit tests for both paths; update
references to changedPackages/versions and any callers to pass the new option if
you choose the gated approach.
1 participant
🎯 Changes
notify-intentworkflow path and keep stale review in the same repo.check-skills.ymlso release/manual runs open or update one review PR for stale skills, artifact drift, and workspace coverage gaps.signals[]for artifact parse warnings, unresolved artifact skill paths, source drift, library version drift, and missing workspace package coverage._artifacts/*domain_map.yamland_artifacts/*skill_tree.yamlas maintainer-reviewed coverage._artifacts, while respectingcoverage.ignored_packages.skills/does not shadow package-local generated skills.npx @tanstack/intent@latest setup.Summary by CodeRabbit
intent stalecommand for monorepos with repository artifact coverage evaluation and warnings for uncovered public packages; private workspaces are now skipped