Skip to content

test: add cross-package export smoke test to catch missing imports#875

Merged
tamirdresher merged 4 commits intoinsiderfrom
fix/cross-package-export-smoke-test
Apr 8, 2026
Merged

test: add cross-package export smoke test to catch missing imports#875
tamirdresher merged 4 commits intoinsiderfrom
fix/cross-package-export-smoke-test

Conversation

@tamirdresher
Copy link
Copy Markdown
Collaborator

What

Adds a vitest smoke test (\ est/cross-package-exports.test.ts) that validates every value import \squad-cli\ uses from \squad-sdk\ resolves to a defined export at runtime.

Why

v0.9.3-insider.1 shipped with \FSStorageProvider\ missing from the SDK barrel — broke users at runtime while all TypeScript-level tests passed (TS resolves from source, not compiled output). This test would have caught that.

What it covers

20 test cases across 15 SDK subpaths and 50+ named exports:

SDK subpath Key exports tested
@bradygaster/squad-sdk\ (root) \FSStorageProvider, \SquadState, \TIMEOUTS, \StreamingPipeline, \RuntimeEventBus, \
esolveSquad, \
esolveGlobalSquadPath, \initSquadTelemetry, \
ecordAgent*, \safeTimestamp, \getMeter, \listRoles, \searchRoles, \getCategories, \getRoleById, \generateCharterFromRole, \�ddAgentToConfig, \initSquad, \cleanupOrphanInitPrompt, \�nsurePersonalSquadDir, \
esolvePersonalSquadDir, \
esolveExternalStateDir, \deriveProjectKey, \setupConsultMode, \isConsultMode, \PersonalSquadNotFoundError, \detectLicense, \loadStagedLearnings, \logConsultation, \mergeToPersonalSquad, \getPersonalSquadRoot, \discoverSquads, \ ormatDiscoveryTable, \ indSquadByName, \�uildDelegationArgs, \loadSubSquadsConfig, \
esolveSubSquad, \RemoteBridge\
/config\ \initSquad, \MigrationRegistry, \writeEconomyMode, \
eadEconomyMode\
/config/agent-source\ \LocalAgentSource\
/resolution\ \
esolveSquad, \
esolveSquadPaths, \
esolveGlobalSquadPath, \
esolvePersonalSquadDir, \�nsurePersonalSquadDir\
/client\ \SquadClient\
/adapter/errors\ \RateLimitError\
/agents/personal\ \
esolvePersonalAgents, \mergeSessionCast\
/casting\ \CastingEngine\
/platform\ \createPlatformAdapter\
/ralph\ \RalphMonitor\
/ralph/triage\ \parseRoster, \parseRoutingRules, \parseModuleOwnership, \ riageIssue\
/ralph/rate-limiting\ \PredictiveCircuitBreaker, \getTrafficLight\
/runtime/event-bus\ \EventBus\
Exports-map file check All entries in SDK \package.json\ exports map point to real files

How to maintain

When adding a new import from @bradygaster/squad-sdk\ to \squad-cli, add a corresponding assertion in this test.

Refs #836

diberry and others added 4 commits April 5, 2026 12:47
…tles to PR readiness (#813)

- Add file list table with per-file +additions/-deletions stats
- Add PR scope classification (Product/Infrastructure/Mixed)
- Rename Architectural Review and Security Review checks with descriptive subtitles

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add CI check that fails when repo-health PRs include product source
code changes under packages/*/src/. Prevents scope creep where
infrastructure PRs accidentally touch product code.

- Add squad-scope-check.yml workflow
- Document PR scope rules in copilot-instructions.md
- Fail loudly on git diff errors instead of silently passing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validates every value import squad-cli uses from squad-sdk resolves to
a defined export at runtime.  Covers 15 SDK subpaths and 50+ named
exports including FSStorageProvider, SquadClient, CastingEngine,
RalphMonitor, and all resolution/config/platform helpers.

Also verifies that every entry in the SDK package.json exports map
points to a file that actually exists on disk.

Motivation: v0.9.3-insider.1 shipped with FSStorageProvider missing
from the SDK barrel — broke users at runtime while all TS-level tests
passed (TypeScript resolves from source, not compiled output).

Refs: #836
Copilot AI review requested due to automatic review settings April 6, 2026 05:17
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2026

🟢 Impact Analysis — PR #875

Risk tier: 🟢 LOW

📊 Summary

Metric Count
Files changed 1
Files added 1
Files modified 0
Files deleted 0
Modules touched 1

🎯 Risk Factors

  • 1 files changed (≤5 → LOW)
  • 1 module(s) touched (≤1 → LOW)

📦 Modules Affected

tests (1 file)
  • test/cross-package-exports.test.ts

This report is generated automatically for every PR. See #733 for details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2026

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit e3a9c75

PR Scope: 🔧 Infrastructure

⚠️ 6 item(s) to address before review

Status Check Details
Single commit 4 commits — consider squashing before review
Not in draft Ready for review
Branch up to date insider is 1 commit(s) ahead — rebase recommended
Copilot review No Copilot review yet — it may still be processing
Changeset present Changeset file found
Scope clean No .squad/ or docs/proposals/ files
No merge conflicts No merge conflicts
Copilot threads resolved 2 unresolved Copilot thread(s) — fix and resolve before merging
CI passing 10 check(s) still running
Issue linked No issue reference — add Closes #N to PR body or commit message
Protected files No protected bootstrap files changed

Files Changed (11 files, +880 −7)

File +/−
.changeset/readiness-file-list.md +7 −0
.changeset/scope-boundary-check.md +8 −0
.changeset/smart-pr-nudge.md +8 −0
.github/copilot-instructions.md +4 −0
.github/workflows/squad-pr-nudge.yml +184 −0
.github/workflows/squad-repo-health.yml +2 −2
.github/workflows/squad-scope-check.yml +31 −0
CONTRIBUTING.md +2 −0
scripts/pr-readiness.mjs +99 −4
test/cross-package-exports.test.ts +287 −0
test/pr-readiness.test.ts +248 −1

Total: +880 −7


This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Vitest smoke test to prevent runtime breakages caused by missing/incorrect @bradygaster/squad-sdk re-exports (e.g., SDK barrel/export-map drift that TypeScript may not catch during development).

Changes:

  • Introduces test/cross-package-exports.test.ts to dynamically import the SDK root barrel + key subpaths and assert required named exports are defined at runtime.
  • Adds an additional check that every packages/squad-sdk/package.json exports entry points to an existing file on disk.

Comment on lines +264 to +279
const pkg = JSON.parse(
fs.readFileSync(resolve(sdkRoot!, 'package.json'), 'utf8'),
);
const exportsMap = pkg.exports as Record<string, Record<string, string>>;
const missing: string[] = [];

for (const [subpath, targets] of Object.entries(exportsMap)) {
if (typeof targets === 'string') {
if (!existsSync(resolve(sdkRoot!, targets))) {
missing.push(`${subpath} → ${targets}`);
}
continue;
}
for (const [condition, file] of Object.entries(targets)) {
if (!existsSync(resolve(sdkRoot!, file))) {
missing.push(`${subpath}[${condition}] → ${file}`);
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkg.exports is being cast to Record<string, Record<string, string>>, but Node’s package.json exports field can be either a string or a nested condition object. This cast makes the typeof targets === 'string' branch type-inconsistent (and would fail if tests are ever type-checked) and also masks the possibility of nested/non-string conditions. Prefer typing exportsMap as unknown (or a union of string | Record<string, unknown>) and narrowing/recursing on leaf string targets before calling resolve()/existsSync().

Copilot uses AI. Check for mistakes.
Comment on lines +15 to +16
* add a corresponding assertion here. The grep one-liner in the test
* description shows how to audit.
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The header comment mentions “The grep one-liner in the test description shows how to audit”, but this file doesn’t currently include that one-liner. Either add the referenced command here (so the maintenance instructions are self-contained) or remove that sentence to avoid stale documentation.

Suggested change
* add a corresponding assertion here. The grep one-liner in the test
* description shows how to audit.
* add a corresponding assertion here. Audit current usage with:
* grep -RhoE "from '@bradygaster/squad-sdk[^']*'" packages/squad-cli/src

Copilot uses AI. Check for mistakes.
@tamirdresher
Copy link
Copy Markdown
Collaborator Author

✅ Verified: Test catches the exact FSStorageProvider bug

We manually reproduced the insider.1 failure and confirmed this test catches it:

Reproduction steps

  1. Commented out export * from './storage/index.js' in packages/squad-sdk/src/index.ts (simulating the missing FSStorageProvider export)
  2. Ran npx vitest run test/cross-package-exports.test.ts

Result

`
❌ FAIL exports FSStorageProvider and core runtime symbols
→ Error: Test timed out in 5000ms (import failed — export doesn't exist)

✅ 19 passed | 1 failed (20 total)
`

The exact test case that checks FSStorageProvider failed when the storage export was removed — which is precisely the bug users hit with v0.9.3-insider.1.

What this prevents going forward

Coverage

  • 20 test cases across 15 SDK subpaths
  • 50+ named exports validated
  • Separate test for exports-map file existence

@tamirdresher tamirdresher changed the base branch from dev to insider April 8, 2026 05:33
@tamirdresher tamirdresher merged commit 9206b89 into insider Apr 8, 2026
34 of 35 checks passed
@diberry
Copy link
Copy Markdown
Collaborator

diberry commented Apr 8, 2026

🧪 Post-Merge Review — Simon (Tester)

Verdict: 🟡 Conditional Approve — Core test is valuable, follow-ups needed.

What's Good

The cross-package export smoke test directly targets the v0.9.3-insider.1 FSStorageProvider incident. The approach — dynamic import() against 15 SDK subpaths plus an exports-map file existence check across all 52 subpaths — is sound. It tests compiled output, not source, catching the exact class of bug that broke users.

Concerns

# Severity Finding
1 🔴 Scope creep — Only 287 of 880 lines are the stated test. Three unrelated features (PR nudge workflow, scope check workflow, readiness file list) were bundled in, each with its own changeset.
2 🟡 Resolution fragility — The test claims to test compiled output, but doesn't verify it resolves to dist/. If vitest resolution ever changes, this test silently becomes useless.
3 🟡 Two unresolved Copilot review comments merged — The exportsMap type cast is unsafe, and the referenced grep audit one-liner was never added.
4 🟡 Manual maintenance burden — New CLI→SDK imports require hand-editing this test. No automated drift detection. This will go stale.
5 🟠 Build order dependency is implicit — Nothing enforces that SDK is built before this test runs.

Recommended Follow-Ups

  1. Add compiled-output resolution guard (expect(import.meta.resolve(...)).toContain('/dist/'))
  2. Add the grep audit command to the test header
  3. Fix the unsafe type cast on pkg.exports
  4. Consider automated import-coverage drift detection in CI
  5. Enforce PR scope discipline — one concern per PR

Review by Squad AI team (Simon — Tester) · requested by Dina Berry

@tamirdresher
Copy link
Copy Markdown
Collaborator Author

Thanks @diberry — Simon's review is valuable. Filed #926 for the resolution guard and type safety fixes. The drift detection idea is worth exploring in CI.

tamirdresher pushed a commit to tamirdresher/squad that referenced this pull request Apr 8, 2026
…#900, bradygaster#875)

- Add YAML value escaping helper for skill metadata
- Replace catch(err: any) with catch(err: unknown) + narrowing
- Add type guards to replace unsafe type assertions
- Standardize deprecation messages on `gh copilot`
- Fix unsafe exports type cast in cross-package test

Closes bradygaster#924, bradygaster#925, bradygaster#926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bradygaster pushed a commit that referenced this pull request Apr 12, 2026
- Add YAML value escaping helper for skill metadata
- Replace catch(err: any) with catch(err: unknown) + narrowing
- Add type guards to replace unsafe type assertions
- Standardize deprecation messages on `gh copilot`
- Fix unsafe exports type cast in cross-package test

Closes #924, #925, #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bradygaster added a commit that referenced this pull request Apr 12, 2026
* fix: address post-merge review findings (#876, #900, #875)

- Add YAML value escaping helper for skill metadata
- Replace catch(err: any) with catch(err: unknown) + narrowing
- Add type guards to replace unsafe type assertions
- Standardize deprecation messages on `gh copilot`
- Fix unsafe exports type cast in cross-package test

Closes #924, #925, #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: add changeset for review findings fix

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: trigger CI

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore(.squad): EECOM history — PR #942 rebase learnings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: update changeset to accurately reflect PR changes (drop YAML escaping reference)

Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/54f41407-61bf-4977-85b7-572341c47b62

Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
tamirdresher added a commit that referenced this pull request Apr 21, 2026
)

* feat(ci): add file list with line stats, scope badge, and check subtitles to PR readiness (#813)

- Add file list table with per-file +additions/-deletions stats
- Add PR scope classification (Product/Infrastructure/Mixed)
- Rename Architectural Review and Security Review checks with descriptive subtitles

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci: scope boundary enforcement for repo-health PRs (#826)

Add CI check that fails when repo-health PRs include product source
code changes under packages/*/src/. Prevents scope creep where
infrastructure PRs accidentally touch product code.

- Add squad-scope-check.yml workflow
- Document PR scope rules in copilot-instructions.md
- Fail loudly on git diff errors instead of silently passing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci: smart PR nudge for stale PRs (#827)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test: add cross-package export smoke test to catch missing imports

Validates every value import squad-cli uses from squad-sdk resolves to
a defined export at runtime.  Covers 15 SDK subpaths and 50+ named
exports including FSStorageProvider, SquadClient, CastingEngine,
RalphMonitor, and all resolution/config/platform helpers.

Also verifies that every entry in the SDK package.json exports map
points to a file that actually exists on disk.

Motivation: v0.9.3-insider.1 shipped with FSStorageProvider missing
from the SDK barrel — broke users at runtime while all TS-level tests
passed (TypeScript resolves from source, not compiled output).

Refs: #836

---------

Co-authored-by: Dina Berry (MSFT) <diberry@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
tamirdresher pushed a commit that referenced this pull request Apr 21, 2026
* fix: address post-merge review findings (#876, #900, #875)

- Add YAML value escaping helper for skill metadata
- Replace catch(err: any) with catch(err: unknown) + narrowing
- Add type guards to replace unsafe type assertions
- Standardize deprecation messages on `gh copilot`
- Fix unsafe exports type cast in cross-package test

Closes #924, #925, #926

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: add changeset for review findings fix

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: trigger CI

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore(.squad): EECOM history — PR #942 rebase learnings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: update changeset to accurately reflect PR changes (drop YAML escaping reference)

Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/54f41407-61bf-4977-85b7-572341c47b62

Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
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.

4 participants