Skip to content

test(claude): pass the prompt registry the child-work drain now requires - #25063

Closed
brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/fix-claude-child-work-evidence-test
Closed

brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/fix-claude-child-work-evidence-test

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​6 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5
Prod 0 0 0 0

ELI5

Two changes that landed on main a day apart don't fit together, so the type check for the main (Node) part of the app fails on main and on every pull request tested against it. This fixes the one test that broke. The app itself is unchanged.

What Changed

The problem. #22634 (merged 2026-10-03) changed drainClaudeChildWork in src/main/claude/claude-child-work-evidence.ts so it also reads the session's prompt registry (prompts). #24707 (merged 2026-10-02) had added a test, src/main/claude/claude-child-work-evidence-retention.test.ts, that calls drainClaudeChildWork with only childWork and translator. Each PR passed CI on its own. Combined on main, the Node type check reports:

src/main/claude/claude-child-work-evidence-retention.test.ts(88,9): error TS2741: Property 'prompts' is missing in type '{ childWork: ClaudeChildWorkDecoder; translator: ClaudeJournalTranslator; }' but required in type 'Pick<ClaudeSession, "childWork" | "prompts" | "translator">'.

CI type-checks a PR merged with main, so every open PR's typecheck job fails on this line. CI's script stops at the first project that fails, so the web and CLI type checks never get to run either.

User-facing change: none. Only a test changes.

The mechanism. The test now passes a fresh ClaudePromptRegistry, built the same way production builds one when a session starts (claude-structured-session-acquisition.ts). The test is about how much child output is kept, not about prompts, so an empty registry is enough.

Why

Giving the test the argument the function now requires is the whole fix. Making prompts optional again would weaken the type #22634 added on purpose.

Linked Issue

None (maintainer fix for a type error on main).

Visual Proof

N/A: test-only change, nothing visible.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • vitest run --config config/vitest.config.ts src/main/claude/claude-child-work-evidence-retention.test.ts: 3/3 pass. They passed before the change too: vitest doesn't type-check, so only the type check can show this fix.

  • oxfmt and oxlint on the changed file: clean.

  • Not run locally: the type check (it runs through a shared gate on this machine). CI's typecheck job on this PR is the check that matters.

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Test-only change: no security, platform, SSH, mobile, compatibility, or performance impact.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

#22634 made drainClaudeChildWork read the session's prompt registry; the
retention test from #24707 landed without it, so main's node typecheck fails.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Superseded: main's claude-child-work-evidence-retention.test.ts now passes the prompt registry (landed on main separately), which is why this PR conflicts. No longer needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant