test: cover recoverable session-load errors and run-id branding#275
test: cover recoverable session-load errors and run-id branding#275
Conversation
Adds three integration cases to rdpath-find-integration.test.ts that exercise the new isRecoverableActiveStateLookupError branches in packages/claude-code-plugin/src/rdpath.ts: legacy ownedRunbooks format, legacy stacks format, and SessionDataSchema validation failure. Adds packages/core/__tests__/runbook/run-id.test.ts covering isRunId, assertRunId, RUN_ID_PATTERN, and RUN_ID_PREFIX. assertRunId is the canonical run-id validator that brand-helpers and a growing set of fixtures rely on; it had no direct tests in core.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🧰 Additional context used📓 Path-based instructions (6)**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,mjs,cjs}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/__tests__/**/*.ts⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (5)📚 Learning: 2026-03-13T07:37:13.038ZApplied to files:
📚 Learning: 2026-04-06T04:20:01.297ZApplied to files:
📚 Learning: 2026-04-24T01:48:48.712ZApplied to files:
📚 Learning: 2026-05-08T00:05:42.310ZApplied to files:
📚 Learning: 2026-05-03T01:45:03.732ZApplied to files:
🔇 Additional comments (4)
📝 WalkthroughWalkthroughAdds three integration tests checking rdpath soft-fail behavior with legacy/malformed .rundown/session.json when RD_WORK_PATH is set, and adds unit tests validating isRunId/assertRunId and exported RUN_ID constants/pattern. Changesrdpath Legacy Session Data Fallback Tests
Run ID Validation and Canonicalization Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying rundown with
|
| Latest commit: |
d9c469f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1c5cc1a2.rundown-7hl.pages.dev |
| Branch Preview URL: | https://worktree-rdpath-recoverable.rundown-7hl.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/__tests__/runbook/run-id.test.ts`:
- Around line 103-107: Add negative parity checks so RUN_ID_PATTERN and isRunId
remain consistent: update the test in run-id.test.ts (the case using
RUN_ID_PATTERN and isRunId) to include several rejected cases (e.g., missing
"rd_" prefix, uppercase hex, wrong length, non-hex chars) and assert both
RUN_ID_PATTERN.test(value) and isRunId(value) are false for those inputs;
alternatively factor allowed and disallowed examples into a shared fixture array
and iterate asserting true for allowed examples and false for disallowed ones to
ensure regex and isRunId stay in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: adfd1251-be58-4ea3-ba3f-3ee726960f99
📒 Files selected for processing (2)
packages/claude-code-plugin/__tests__/rdpath-find-integration.test.tspackages/core/__tests__/runbook/run-id.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: setup-build (24)
- GitHub Check: smoke-test-macos
- GitHub Check: smoke-test-linux
- GitHub Check: analyze
- GitHub Check: smoke-test-macos
- GitHub Check: smoke-test-linux
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use clear, descriptive variable and function names that convey intent
Add comments for complex logic and non-obvious code sections
Use async/await over callbacks and promise chains
Use const by default, let for variables that need reassignment, avoid var
Use template literals instead of string concatenation
Files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety; define explicit types instead of relying on inference
**/*.{ts,tsx}: Preferinterfacefor defining object shapes in TypeScript
UseisError(),isNodeError(), orgetErrorMessage()from@rundown-org/core(orpackages/claude-code-plugin/src/shared/errors.tsinside the plugin) — never callError.isError()directly
All exported TypeScript symbols must have TSDoc documentation including description,@paramfor all parameters,@returnsif non-void, and@throwsif exceptions are possible
Use discriminated unions and type narrowing to make invalid states unrepresentable. Guards should express domain conditions through typed return values, never raw action-type string checks.
Use XState's native event system and state graph structure instead of creating artificial state identifiers (like~channelprefixes) for synthetic IDs.
Files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests with clear arrange-act-assert structure
Files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
**/*.{ts,tsx,js,mjs,cjs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,mjs,cjs}: Use camelCase for variable names in TypeScript/JavaScript
Always use async/await for promises in TypeScript/JavaScript
Variable names must match the pattern/^[a-zA-Z_][a-zA-Z0-9_]*$/(alphanumeric and underscore, starting with letter or underscore)
Files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Mock injected core services structurally in non-core tests using object-shaped service doubles instead of constructing real core services from mocked modules
Files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
**/__tests__/**/*.ts
⚙️ CodeRabbit configuration file
**/__tests__/**/*.ts: Test files. Focus on:
- Use Error.isError() instead of instanceof Error except for same-realm custom error classes.
- Mock passthrough pattern: pass through real functions that do not need mocking.
- Test isolation, meaningful assertions, edge cases, and regression coverage for changed behavior.
- Prefer targeted verification commands that match the package touched by the PR.
Files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
packages/claude-code-plugin/**/*.ts
⚙️ CodeRabbit configuration file
packages/claude-code-plugin/**/*.ts: Claude Code plugin for runbook orchestration. Focus on:
- MCP integration correctness, runbook discovery, namespace resolution, state lifecycle, and hook behavior.
- Safe path handling for plugin root/data paths and generated files.
- Changes to Zod schemas must stay consistent with checked-in JSON schemas.
Files:
packages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
🧠 Learnings (6)
📚 Learning: 2026-03-13T07:37:13.038Z
Learnt from: tobyhede
Repo: tobyhede/rundown PR: 102
File: packages/cli/__tests__/commands/fail.test.ts:134-141
Timestamp: 2026-03-13T07:37:13.038Z
Learning: In the tobyhede/rundown repository, ensure that ErrorResponseSchema in packages/core/src/output/zod-schemas.ts uses command: z.string().optional() (renamed from action) to reflect NoActiveRunbookOutput.action → command across the codebase. Update and verify all references to this field, including related code paths such as OutputEmitter.noActiveRunbook() and json-renderer.ts. During reviews, check that tests (e.g., packages/cli/__tests__/commands/fail.test.ts) align with the new field name and that any expectations or mocks reference command rather than action.
Applied to files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
📚 Learning: 2026-04-06T04:20:01.297Z
Learnt from: tobyhede
Repo: tobyhede/rundown PR: 177
File: packages/claude-code-plugin/runbooks/planning/write-plan.runbook.md:24-24
Timestamp: 2026-04-06T04:20:01.297Z
Learning: In this repository (tobyhede/rundown), `CLAUDE_PLUGIN_ROOT` is part of a published contract and is guaranteed to already include a trailing `/` (enforced since PR `#174`). Therefore, when reviewing code, do not flag path concatenations that join the variable directly to the next path segment (e.g., `{{ CLAUDE_PLUGIN_ROOT }}schemas/plan.schema.json`) as missing a separator—this resolves correctly because the trailing slash is included in the variable value.
Applied to files:
packages/core/__tests__/runbook/run-id.test.tspackages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
📚 Learning: 2026-04-24T01:48:48.712Z
Learnt from: tobyhede
Repo: tobyhede/rundown PR: 227
File: packages/core/__tests__/runbook/compiler.test.ts:9985-10000
Timestamp: 2026-04-24T01:48:48.712Z
Learning: In `packages/core/__tests__/runbook/**/*.test.ts`, avoid using `as any` when preparing hydrated/persisted snapshots for `createActor(machine, { snapshot })`. Instead, construct the persisted snapshot in a typed way (e.g., start from `baseSnap` and build the final snapshot via object spread so TypeScript can infer `typeof baseSnap`). This keeps the persisted snapshot/context shape checked at compile time and prevents schema drift from bypassing the type system.
Applied to files:
packages/core/__tests__/runbook/run-id.test.ts
📚 Learning: 2026-05-03T01:45:03.732Z
Learnt from: tobyhede
Repo: tobyhede/rundown PR: 255
File: packages/cli/__tests__/commands/status.test.ts:239-249
Timestamp: 2026-05-03T01:45:03.732Z
Learning: In tobyhede/rundown’s status output logic (notably `buildStashedStatus` in `packages/cli`/`packages/core`), when a stashed runbook has `parentLinkage.kind === 'delegation'`, the generated status must omit the `vars` key for delegation-claimed children. Specifically, `rd status` (without `--claim-id`) must not expose inherited variable contents for these delegation-claimed descendants (e.g., ensure output does not contain sensitive values). Only callers using `rd status --claim-id <claimId>` (the capability-holder) are allowed to see the `vars` field for stashed delegated children. This guards against production data leaks; preserve/test this isolation behavior (as fixed in PR `#255`, commit dfba8464).
Applied to files:
packages/core/__tests__/runbook/run-id.test.ts
📚 Learning: 2026-03-12T12:08:17.780Z
Learnt from: tobyhede
Repo: tobyhede/rundown PR: 100
File: packages/claude-code-plugin/src/rdpath-core.ts:9-9
Timestamp: 2026-03-12T12:08:17.780Z
Learning: For imports from 'node:path' in the packages/claude-code-plugin package, prefer namespace imports: import * as path from 'node:path'. This is the dominant pattern across 8+ files (e.g., shared/logger.ts, shared/utils.ts, context.ts, gate-loader.ts, gates/plugin-path.ts, shared/config.ts). Do not suggest switching to named imports; preserve the namespace import convention in this package.
Applied to files:
packages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
📚 Learning: 2026-04-08T00:08:09.549Z
Learnt from: tobyhede
Repo: tobyhede/rundown PR: 187
File: packages/claude-code-plugin/__tests__/review-schema.test.ts:98-99
Timestamp: 2026-04-08T00:08:09.549Z
Learning: In tobyhede/rundown, enforce the CLAUDE.md `isZodError()` convention for Zod-related errors. Instead of `instanceof ZodError` in both test and production code (and instead of `toThrow(ZodError)` / `expect(...).toBeInstanceOf(ZodError)` in Jest tests), write Jest assertions as: use `expect(fn).toThrow()` (no `ZodError` class argument), then capture the thrown error (or use the matcher’s error value) and assert `expect(isZodError(err)).toBe(true)` using `isZodError` from `packages/claude-code-plugin/src/shared/errors.ts`. Do not accept the `ZodError` class checks even if they might work due to module loading nuances; treat them as violations. Existing occurrences may be considered acknowledged tech debt until cleaned up (e.g., in plan-schema*.test.ts files).
Applied to files:
packages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts
🔇 Additional comments (2)
packages/claude-code-plugin/__tests__/rdpath-find-integration.test.ts (1)
471-540: Nice regression coverage for recoverable session-load failures.These cases pin the intended CLI contract well: exit
0, no stderr, and no ctx segment whensession.jsonis legacy or schema-invalid.packages/core/__tests__/runbook/run-id.test.ts (1)
4-91: Good coverage of the canonical run-id validator contract.The accepted/rejected cases plus
assertRunIdidempotence make the branded run-id rules very explicit.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Existing parity test only verified the positive direction. Add an it.each block covering common invalid inputs (missing prefix, uppercase hex, wrong length, non-hex chars, UUID body) and assert both RUN_ID_PATTERN.test() and isRunId() return false, so the regex and the predicate stay in sync if either changes.
|
✅ Created PR with unit tests: #276 |
1 participant
Summary
packages/claude-code-plugin/__tests__/rdpath-find-integration.test.tsthat exercise theisRecoverableActiveStateLookupErrorbranches added topackages/claude-code-plugin/src/rdpath.tsin Artifact uri phase 3 cli runtime variable contract exec #267: legacyownedRunbookssession, legacystackssession, andSessionDataSchemavalidation failure.packages/core/__tests__/runbook/run-id.test.tscoveringisRunId,assertRunId,RUN_ID_PATTERN, andRUN_ID_PREFIX.assertRunIdis the canonical run-id validator thatbrand-helpers.tsand a growing set of fixtures rely on; it had no direct tests in core.Test plan
Summary by CodeRabbit