Improve Router and Start agent skills - #7867
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughDocumentation and skill metadata were refreshed for newer Router and Start versions. Guidance now covers SSR-safe loaders, server-function boundaries, cache invalidation, runtime response validation, authentication boundaries, request-scoped sessions, generated route trees, and virtual route audits. ChangesSkill catalogs and metadata
React Start and Start Client workflows
Router guidance
Server sessions
Generated and virtual routes
Cross-framework metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
View your CI Pipeline Execution ↗ for commit 111f193
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 1 bumped as dependents. 🟩 Patch bumps
|
Contributor
Bundle Size BenchmarksThis pull request does not affect bundle size in any measured scenario. |
Merging this PR will degrade performance by 12.73%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server peak-large-page (react) |
953.9 KB | 2,384.3 KB | -59.99% |
| ❌ | Memory | mem server error-paths redirect (vue) |
397.5 KB | 876.7 KB | -54.66% |
| ❌ | Memory | mem server serialization-payload (solid) |
3.5 MB | 6 MB | -42.57% |
| ❌ | Simulation | client-mount loop (solid) |
58.5 ms | 60.7 ms | -3.61% |
| ⚡ | Memory | mem server error-paths unmatched (react) |
519.6 KB | 274.5 KB | +89.29% |
| ⚡ | Memory | mem server peak-large-page (solid) |
953 KB | 774.8 KB | +23% |
| ⚡ | Memory | mem server error-paths not-found (solid) |
491.8 KB | 422.4 KB | +16.45% |
| ⚡ | Memory | mem server server-fn-churn (solid) |
273.6 KB | 263 KB | +4.03% |
| ⚡ | Memory | mem server error-paths not-found (react) |
276.5 KB | 266.7 KB | +3.66% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing taren/improve-router-start-agent-skills (111f193) with main (fc83c03)
tannerlinsley
marked this pull request as ready for review
July 25, 2026 03:21
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react-start/skills/_artifacts/domain_map.yaml`:
- Around line 207-239: Regenerate the failure-mode artifacts from one canonical
source so counts and entries remain consistent: update
packages/react-start/skills/_artifacts/domain_map.yaml at lines 207-239,
390-397, and 431-454 by adding the missing server-functions, execution-model,
and server-routes entries, or consistently reduce the specification instead.
Then update packages/react-start/skills/_artifacts/skill_spec.md at lines 21-24,
40-51, 61-70, and 71-79 so inventory counts and all three domain tables exactly
match the canonical domain map.
In `@packages/router-core/skills/router-core/auth-and-guards/SKILL.md`:
- Around line 412-413: Align the direct-request attack and test guidance with
the method declared in the createServerFn example: use a direct anonymous GET
request, or change the example method to POST if POST is intended. Keep the
redirect-following and protected-data assertions unchanged.
In `@packages/router-core/skills/router-core/data-loading/SKILL.md`:
- Around line 218-223: Update the Route example’s beforeLoad callback to receive
fetchPosts from its context and return that binding, preserving the loader’s
existing context-based fetchPosts usage.
In `@packages/start-client-core/skills/start-core/server-functions/SKILL.md`:
- Around line 104-108: Replace the type-only validator in createIssue with a
real runtime validation using Zod or an explicit check before db.issues.create.
Apply the same runtime-validation fix at
packages/start-client-core/skills/start-core/server-functions/SKILL.md lines
104-108 and packages/start-server-core/skills/start-server-core/SKILL.md lines
183-184, including the credential-validation example, while preserving the
documented input and authentication behavior.
In `@packages/start-server-core/skills/start-server-core/SKILL.md`:
- Around line 185-188: Update the credential check around db.users.findByEmail
and verifyPassword so unknown users still invoke verifyPassword using the fixed
dummy hash required by the auth-server-primitives guidance. Preserve the real
user passwordHash for known users and continue throwing the same generic
“Invalid credentials” error for either failure case.
🪄 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: 7193e579-f878-4399-a226-0fcf0b30f5ee
📒 Files selected for processing (40)
_artifacts/skill_tree.yamlpackages/react-start/skills/_artifacts/domain_map.yamlpackages/react-start/skills/_artifacts/skill_spec.mdpackages/react-start/skills/_artifacts/skill_tree.yamlpackages/react-start/skills/lifecycle/migrate-from-nextjs/SKILL.mdpackages/react-start/skills/react-start/SKILL.mdpackages/react-start/skills/react-start/server-components/SKILL.mdpackages/router-core/skills/router-core/SKILL.mdpackages/router-core/skills/router-core/auth-and-guards/SKILL.mdpackages/router-core/skills/router-core/code-splitting/SKILL.mdpackages/router-core/skills/router-core/data-loading/SKILL.mdpackages/router-core/skills/router-core/navigation/SKILL.mdpackages/router-core/skills/router-core/not-found-and-errors/SKILL.mdpackages/router-core/skills/router-core/path-params/SKILL.mdpackages/router-core/skills/router-core/search-params/SKILL.mdpackages/router-core/skills/router-core/ssr/SKILL.mdpackages/router-core/skills/router-core/type-safety/SKILL.mdpackages/router-plugin/skills/_artifacts/domain_map.yamlpackages/router-plugin/skills/_artifacts/skill_spec.mdpackages/router-plugin/skills/_artifacts/skill_tree.yamlpackages/router-plugin/skills/router-plugin/SKILL.mdpackages/start-client-core/skills/_artifacts/domain_map.yamlpackages/start-client-core/skills/_artifacts/skill_spec.mdpackages/start-client-core/skills/_artifacts/skill_tree.yamlpackages/start-client-core/skills/start-core/SKILL.mdpackages/start-client-core/skills/start-core/auth-server-primitives/SKILL.mdpackages/start-client-core/skills/start-core/deployment/SKILL.mdpackages/start-client-core/skills/start-core/execution-model/SKILL.mdpackages/start-client-core/skills/start-core/middleware/SKILL.mdpackages/start-client-core/skills/start-core/server-functions/SKILL.mdpackages/start-client-core/skills/start-core/server-routes/SKILL.mdpackages/start-server-core/skills/_artifacts/domain_map.yamlpackages/start-server-core/skills/_artifacts/skill_spec.mdpackages/start-server-core/skills/_artifacts/skill_tree.yamlpackages/start-server-core/skills/start-server-core/SKILL.mdpackages/start-server-core/vite.config.tspackages/virtual-file-routes/skills/_artifacts/domain_map.yamlpackages/virtual-file-routes/skills/_artifacts/skill_spec.mdpackages/virtual-file-routes/skills/_artifacts/skill_tree.yamlpackages/virtual-file-routes/skills/virtual-file-routes/SKILL.md
💤 Files with no reviewable changes (1)
- _artifacts/skill_tree.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/router-core/skills/router-core/data-loading/SKILL.md (1)
231-233: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDefine the
dbdependency in the server-function example.The snippet references
db.posts.findMany()without declaring or importingdb, so it is not copyable as written. Add a representative import/definition or explicitly mark the database access as pseudocode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/router-core/skills/router-core/data-loading/SKILL.md` around lines 231 - 233, Update the createServerFn example around getPosts to declare or import a representative db dependency before calling db.posts.findMany(), making the snippet copyable; alternatively, explicitly label the database access as pseudocode.packages/start-server-core/skills/start-server-core/SKILL.md (1)
218-220: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCreate a fresh session before setting
userId.updateSessiononly mutates the current session data; it doesn’t rotate the session identifier, so reusing the pre-login cookie here leaves room for session fixation. Clear the old session and issue a new one at login.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-server-core/skills/start-server-core/SKILL.md` around lines 218 - 220, Update the login flow around updateSession so it clears the pre-login session and creates a fresh session identifier before assigning userId. Preserve the existing getSessionConfig() and SessionData usage while ensuring the authenticated session cannot reuse the original login cookie.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/start-server-core/skills/start-server-core/SKILL.md`:
- Around line 172-178: Update getDummyPasswordHash and its
startup/authentication flow so an unset DUMMY_PASSWORD_HASH is validated during
startup, or its failure is mapped to the same generic Invalid credentials
response used for unknown users. Ensure the unknown-user path never exposes the
distinct DUMMY_PASSWORD_HASH is required error.
---
Outside diff comments:
In `@packages/router-core/skills/router-core/data-loading/SKILL.md`:
- Around line 231-233: Update the createServerFn example around getPosts to
declare or import a representative db dependency before calling
db.posts.findMany(), making the snippet copyable; alternatively, explicitly
label the database access as pseudocode.
In `@packages/start-server-core/skills/start-server-core/SKILL.md`:
- Around line 218-220: Update the login flow around updateSession so it clears
the pre-login session and creates a fresh session identifier before assigning
userId. Preserve the existing getSessionConfig() and SessionData usage while
ensuring the authenticated session cannot reuse the original login cookie.
🪄 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: 639bbe71-d329-446b-b416-f03cae91bae6
📒 Files selected for processing (12)
packages/react-start/skills/_artifacts/domain_map.yamlpackages/react-start/skills/_artifacts/skill_spec.mdpackages/router-core/skills/router-core/auth-and-guards/SKILL.mdpackages/router-core/skills/router-core/data-loading/SKILL.mdpackages/router-core/skills/router-core/type-safety/SKILL.mdpackages/router-plugin/skills/_artifacts/skill_spec.mdpackages/start-client-core/skills/_artifacts/domain_map.yamlpackages/start-client-core/skills/_artifacts/skill_spec.mdpackages/start-client-core/skills/start-core/server-functions/SKILL.mdpackages/start-server-core/skills/_artifacts/skill_spec.mdpackages/start-server-core/skills/start-server-core/SKILL.mdpackages/virtual-file-routes/skills/_artifacts/skill_spec.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/router-core/skills/router-core/auth-and-guards/SKILL.md
- packages/start-client-core/skills/start-core/server-functions/SKILL.md
- packages/react-start/skills/_artifacts/skill_spec.md
- packages/router-plugin/skills/_artifacts/skill_spec.md
- packages/start-client-core/skills/_artifacts/skill_spec.md
Comment on lines
+172
to
+178
| function getDummyPasswordHash() { | ||
| // Precompute this with the same algorithm and cost as real password hashes. | ||
| const hash = process.env.DUMMY_PASSWORD_HASH | ||
| if (!hash) { | ||
| throw new Error('DUMMY_PASSWORD_HASH is required') | ||
| } | ||
| return hash |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file list ---\n'
git ls-files 'packages/start-server-core/skills/start-server-core/SKILL.md' || true
printf '\n--- search ---\n'
rg -n "DUMMY_PASSWORD_HASH|getDummyPasswordHash|Invalid credentials|updateSession|session" packages/start-server-core -S
printf '\n--- relevant excerpt ---\n'
sed -n '150,220p' packages/start-server-core/skills/start-server-core/SKILL.mdRepository: TanStack/router
Length of output: 244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file list ---'
git ls-files 'packages/start-server-core/skills/start-server-core/SKILL.md' || true
echo
echo '--- search ---'
rg -n "DUMMY_PASSWORD_HASH|getDummyPasswordHash|Invalid credentials|updateSession|session" packages/start-server-core -S || true
echo
echo '--- relevant excerpt ---'
sed -n '150,220p' packages/start-server-core/skills/start-server-core/SKILL.mdRepository: TanStack/router
Length of output: 9867
Fail closed without a distinct unknown-user error
If DUMMY_PASSWORD_HASH is unset, the unknown-user path throws DUMMY_PASSWORD_HASH is required before the generic Invalid credentials response. Validate this at startup, or map the failure to the same authentication error.
🧰 Tools
🪛 SkillSpector (2.3.11)
[error] 24: [YR1] YARA rule 'info_stealer': Information stealer patterns (credential harvesting, browser data theft) [malware]: YARA rule matched a known malware signature (reverse shell, backdoor, ransomware, C2 framework, or info stealer).
Remediation: Remove the malware payload or compromised file entirely. Investigate how it entered the skill and audit all other artifacts for additional indicators of compromise.
(YARA Match (YR1))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/start-server-core/skills/start-server-core/SKILL.md` around lines
172 - 178, Update getDummyPasswordHash and its startup/authentication flow so an
unset DUMMY_PASSWORD_HASH is validated during startup, or its failure is mapped
to the same generic Invalid credentials response used for unknown users. Ensure
the unknown-user path never exposes the distinct DUMMY_PASSWORD_HASH is required
error.
![nx-cloud[bot]](https://cdn.github.com/in/80458?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We fix the EADDRINUSE error by adding a module-level _allocatedPorts set in derivePort.ts that tracks every port assigned within the current process. When rm -rf port*.txt clears all cached port files before each test run, both getTestServerPort and getDummyServerPort called getRandomPort() in rapid succession and consistently received the same port (43099), because getRandomPort temporarily binds then immediately releases a port — leaving it free for the very next call. The do/while loop now retries until a port not already in the set is returned, guaranteeing unique ports across all derivePort invocations in the same process.
Tip
✅ We verified this fix by re-running tanstack-react-start-e2e-basic-react-query:test:e2e.
Suggested Fix changes
diff --git a/e2e/e2e-utils/src/derivePort.ts b/e2e/e2e-utils/src/derivePort.ts
index 3ecb86cb..2220203f 100644
--- a/e2e/e2e-utils/src/derivePort.ts
+++ b/e2e/e2e-utils/src/derivePort.ts
@@ -1,6 +1,8 @@
import fs from 'node:fs'
import { getRandomPort } from 'get-port-please'
+const _allocatedPorts = new Set<number>()
+
/**
* Check if a port has been allocated, if it hasn't generate a random port and save it.
* @param {string} input - port test allocation
@@ -10,10 +12,16 @@ export async function derivePort(input: string): Promise<number> {
const portFile = `port-${input}.txt`
if (!fs.existsSync(portFile)) {
- fs.writeFileSync(portFile, (await getRandomPort()).toString())
+ let port: number
+ do {
+ port = await getRandomPort()
+ } while (_allocatedPorts.has(port))
+ _allocatedPorts.add(port)
+ fs.writeFileSync(portFile, port.toString())
}
const portNumber = parseInt(await fs.promises.readFile(portFile, 'utf-8'))
+ _allocatedPorts.add(portNumber)
console.info(`Mapped "${input}" to port ${portNumber}`)
return portNumber
}
Or Apply changes locally with:
npx nx-cloud apply-locally sPYQ-BwLa
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/start-server-core/skills/start-server-core/SKILL.md (1)
173-179: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expose the dummy-hash configuration error to unknown users.
When
DUMMY_PASSWORD_HASHis unset, the unknown-user path throwsDUMMY_PASSWORD_HASH is requiredbefore returning the generic authentication failure. Validate this configuration before serving requests, or map the failure to the same genericInvalid credentialsresponse; this is the same unresolved issue from the previous review.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-server-core/skills/start-server-core/SKILL.md` around lines 173 - 179, Update getDummyPasswordHash so a missing DUMMY_PASSWORD_HASH cannot surface its configuration error to unknown users: validate the variable during server startup before requests are served, or catch and map the failure to the existing generic “Invalid credentials” response. Preserve identical authentication behavior for unknown and invalid users.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/start-server-core/skills/start-server-core/SKILL.md`:
- Around line 173-179: Update getDummyPasswordHash so a missing
DUMMY_PASSWORD_HASH cannot surface its configuration error to unknown users:
validate the variable during server startup before requests are served, or catch
and map the failure to the existing generic “Invalid credentials” response.
Preserve identical authentication behavior for unknown and invalid users.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30f787ea-8951-487a-a5d3-93e973ef1420
📒 Files selected for processing (30)
packages/react-router/skills/compositions/router-query/SKILL.mdpackages/react-router/skills/lifecycle/migrate-from-react-router/SKILL.mdpackages/react-router/skills/react-router/SKILL.mdpackages/react-start/skills/lifecycle/migrate-from-nextjs/SKILL.mdpackages/react-start/skills/react-start/SKILL.mdpackages/react-start/skills/react-start/server-components/SKILL.mdpackages/router-core/skills/router-core/SKILL.mdpackages/router-core/skills/router-core/auth-and-guards/SKILL.mdpackages/router-core/skills/router-core/code-splitting/SKILL.mdpackages/router-core/skills/router-core/data-loading/SKILL.mdpackages/router-core/skills/router-core/navigation/SKILL.mdpackages/router-core/skills/router-core/not-found-and-errors/SKILL.mdpackages/router-core/skills/router-core/path-params/SKILL.mdpackages/router-core/skills/router-core/search-params/SKILL.mdpackages/router-core/skills/router-core/ssr/SKILL.mdpackages/router-core/skills/router-core/type-safety/SKILL.mdpackages/router-plugin/skills/router-plugin/SKILL.mdpackages/solid-router/skills/solid-router/SKILL.mdpackages/solid-start/skills/solid-start/SKILL.mdpackages/start-client-core/skills/start-core/SKILL.mdpackages/start-client-core/skills/start-core/auth-server-primitives/SKILL.mdpackages/start-client-core/skills/start-core/deployment/SKILL.mdpackages/start-client-core/skills/start-core/execution-model/SKILL.mdpackages/start-client-core/skills/start-core/middleware/SKILL.mdpackages/start-client-core/skills/start-core/server-functions/SKILL.mdpackages/start-client-core/skills/start-core/server-routes/SKILL.mdpackages/start-server-core/skills/start-server-core/SKILL.mdpackages/virtual-file-routes/skills/virtual-file-routes/SKILL.mdpackages/vue-router/skills/vue-router/SKILL.mdpackages/vue-start/skills/vue-start/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/router-core/skills/router-core/ssr/SKILL.md
- packages/react-start/skills/react-start/server-components/SKILL.md
- packages/router-core/skills/router-core/not-found-and-errors/SKILL.md
- packages/router-core/skills/router-core/code-splitting/SKILL.md
- packages/start-client-core/skills/start-core/auth-server-primitives/SKILL.md
- packages/start-client-core/skills/start-core/server-functions/SKILL.md
- packages/router-core/skills/router-core/navigation/SKILL.md
- packages/virtual-file-routes/skills/virtual-file-routes/SKILL.md
- packages/router-core/skills/router-core/data-loading/SKILL.md
- packages/start-client-core/skills/start-core/server-routes/SKILL.md
- packages/router-core/skills/router-core/auth-and-guards/SKILL.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Protocol-v4 assisted Start runs used more context than baseline, repeatedly rediscovered skills, and still missed SSR, auth, cache, and runtime schema failures. These changes encode those failure modes at the package boundary that owns them.
Validation
@tanstack/intent@0.0.34 validate --github-summary— 31 skills passedgit diff --checkSummary by CodeRabbit