Readability: sync/async cleanup, DRY, type safety, config fixes

[WolfieLeader]

Summary

Makes scanForIntents and scanLibrary synchronous — both functions only use readFileSync/existsSync/readdirSync internally, so the async/Promise wrapper was misleading. Also cleans up test DRY and type safety.

Approach

Sync/async correction

• Remove async keyword and Promise<> return type from scanForIntents() and scanLibrary()
• Update all call sites: drop await, switch test assertions from .rejects.toThrow() to .toThrow()
• scanIntentsOrFail in cli.ts stays async (it uses dynamic import()) but drops the redundant await on the now-sync call

DRY test helpers (staleness.test.ts)

• Extract mockFetchVersion(), mockFetchNotOk(), requireFirstSkill() — each replaces a 3-4 line repeated pattern
• Import findSkillFiles from src/utils.ts instead of reimplementing in skills.test.ts

Type safety (skills.test.ts)

• Add explicit guards (!match[1] || match[2] === undefined) before accessing regex capture groups, respecting noUncheckedIndexedAccess: true

Config (tsconfig.json)

• Remove outDir/declaration/noEmit: false overrides — tsdown handles the build, these were dead config

Key invariants

• scanForIntents and scanLibrary must remain purely synchronous (no await, no .then(), no dynamic import() inside them)
• All test assertions that check thrown errors must use synchronous .toThrow(), not .rejects.toThrow()

Non-goals

• ESLint config changes were removed from this PR (scope creep — they affect the entire monorepo)
• No error handling changes — all existing try/catch patterns are preserved

Verification

pnpm run build
pnpm vitest run  # 205 tests pass

Files changed

File	Change
src/scanner.ts	async → sync for scanForIntents
src/library-scanner.ts	async → sync for scanLibrary
src/intent-library.ts	Drop await from cmdList and its call site
src/cli.ts	Drop redundant await in scanIntentsOrFail
tests/scanner.test.ts	Remove async/await from all 26 tests
tests/library-scanner.test.ts	Remove async/await, use scriptPath helper
tests/staleness.test.ts	Extract DRY helpers, consistent usage
tests/skills.test.ts	Import findSkillFiles, tighten type guards
tsconfig.json	Remove unused build overrides
.changeset/neat-pots-melt.md	Patch changeset

---

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Simplified internal operations by converting asynchronous implementations to synchronous execution where operations are purely synchronous
  • Removed unnecessary async/await patterns throughout the codebase

• Tests

  • Converted test suite from async to synchronous patterns for improved clarity
  • Introduced shared test helper functions to reduce duplication and enhance maintainability

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@tanstack/intent@55

commit: 3c1dae7

[changeset-bot]

🦋 Changeset detected

Latest commit: 3c1dae7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@tanstack/intent	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coderabbitai]

📝 Walkthrough

Walkthrough

Converts scanForIntents and scanLibrary functions from asynchronous to synchronous execution by removing async/await keywords and updating all call sites. Introduces test helper functions for common operations and adjusts TypeScript configuration. Updates test suites to work with synchronous functions.

Changes

Cohort / File(s)	Summary
Function Signature Updates packages/intent/src/scanner.ts, packages/intent/src/library-scanner.ts, packages/intent/src/intent-library.ts	Removed async keyword and Promise wrappers from scanForIntents and scanLibrary. Changed cmdList from async to sync with corresponding call site updates to remove await.
Error Handling Impact packages/intent/src/cli.ts	Changed from await scanForIntents() to returning the promise directly, altering error handling semantics—promise rejections now bypass the surrounding try/catch block.
Test Synchronization packages/intent/tests/scanner.test.ts, packages/intent/tests/library-scanner.test.ts	Converted all test cases from async/await to synchronous execution. Removed await keywords from scanForIntents invocations and adjusted assertions accordingly.
Test Utilities & Refactoring packages/intent/tests/staleness.test.ts, packages/intent/tests/skills.test.ts	Added test helpers (requireFirstSkill, mockFetchVersion, mockFetchNotOk) for DRY principles. Moved findSkillFiles implementation to external module import; tightened null checks in frontmatter extraction.
Configuration packages/intent/tsconfig.json	Enabled noEmit, removed outDir and declaration flags, expanded include to add vitest.config.ts. Compilation checks now include test configuration file.
Changelog .changeset/neat-pots-melt.md	Documentation of async-to-sync conversion and test refactoring changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Async hops now skip away,
Sync functions save the day!
Helpers jump to aid the test,
Cleaner code—now at its best!
One small catch: check error's flight,
Promises handled safe and tight. 🎯

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: converting async functions to sync, removing unnecessary async/await, improving DRY in tests, and addressing type safety and config issues.
Description check	✅ Passed	The PR description comprehensively covers all major changes with clear sections on sync/async correction, DRY test helpers, type safety improvements, and config fixes, and includes verification steps and file changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/intent/tests/staleness.test.ts (1)

49-54: Minor: Redundant assertion before throw.

The expect(skill).toBeDefined() on line 51 will already fail the test if skill is undefined. The subsequent if (!skill) throw on line 52 is only needed for TypeScript narrowing. Consider simplifying:

♻️ Suggested simplification

 function requireFirstSkill(report: Awaited<ReturnType<typeof checkStaleness>>) {
   const skill = report.skills[0]
-  expect(skill).toBeDefined()
   if (!skill) throw new Error('Expected at least one skill in staleness report')
   return skill
 }

The throw provides both the type narrowing and a clear error message, making the expect redundant.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/intent/tests/staleness.test.ts` around lines 49 - 54, In function
requireFirstSkill, remove the redundant test assertion
expect(skill).toBeDefined() and keep the existing runtime null-check and throw
(if (!skill) throw new Error(...)) to provide TypeScript narrowing and the clear
error message; this simplifies the function while preserving type-safety and
test clarity.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/intent/tests/staleness.test.ts`:
- Around line 49-54: In function requireFirstSkill, remove the redundant test
assertion expect(skill).toBeDefined() and keep the existing runtime null-check
and throw (if (!skill) throw new Error(...)) to provide TypeScript narrowing and
the clear error message; this simplifies the function while preserving
type-safety and test clarity.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b4dd4deb-8abf-42a8-95bd-d6f2d61d91dd

📥 Commits

Reviewing files that changed from the base of the PR and between b808071 and 3c1dae7.

📒 Files selected for processing (10)

• .changeset/neat-pots-melt.md
• packages/intent/src/cli.ts
• packages/intent/src/intent-library.ts
• packages/intent/src/library-scanner.ts
• packages/intent/src/scanner.ts
• packages/intent/tests/library-scanner.test.ts
• packages/intent/tests/scanner.test.ts
• packages/intent/tests/skills.test.ts
• packages/intent/tests/staleness.test.ts
• packages/intent/tsconfig.json

[WolfieLeader]

@KyleAMathews

Thank you very much!
I appreciate that🙏🏽

[WolfieLeader]

OMG Tanner himself approved it
@tannerlinsley Thank you so much!