Skip to content

fix: restore missing src/types/index.ts (measured: SDK bump does NOT fix the 145 drift errors) - #49

Merged
yakimoto merged 1 commit into
mainfrom
fix/sdk-pin-and-types
Sep 4, 2026
Merged

fix: restore missing src/types/index.ts (measured: SDK bump does NOT fix the 145 drift errors)#49
yakimoto merged 1 commit into
mainfrom
fix/sdk-pin-and-types

Conversation

@yakimoto

@yakimoto yakimoto commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

The two-experiment numbers (measured, not guessed)

Baseline on origin/main (5899f5b), npm ci --include=dev && npm run type-check:
148 errors, split into ~145 SDK-API-surface-drift errors across src/commands/**/index.ts
and 3 TS2307: Cannot find module '../../types/index.js' errors.

Experiment: npm install --no-save @wave-av/sdk@2.1.3 --@wave-av:registry=https://registry.npmjs.org/
(the repo's scoped registry config points @wave-av at GitHub Packages, which 404s on this
package — had to force the public npm registry for the one-off install) then npm run type-check:
152 errors. A line-by-line diff of the two error lists shows the bump fixed zero of the
145 original SDK-drift errors and added 4 new ones (Cannot find module 'vitest' — the
reinstall perturbed vitest's ambient type resolution as a side effect). Net: strictly worse.

Decision (per the brief's own rule): the count did not drop, so the pin is NOT the cause. Did
not bump it.
package.json and package-lock.json are untouched by this PR — no collision
with PR #47 which also touches package.json. The 145 SDK-drift errors need call-site fixes
against the currently-pinned 2.0.14 client surface, which is a much larger, separate pass (see
"What remains red" below) — not attempted here to keep this PR reviewable and avoid guessing at
35 files of unfamiliar SDK surface under time pressure.

Task 2 — the 3 missing-module errors (fixed)

src/types/index.ts does not exist on origin/main at all, but an unmerged commit
(382adee, branch origin/fix/remediation-af8a172e-5f3f12, was part of PR #19 which was closed
without merging) contains exactly this file. I read all three importing files
(src/lib/auth/device-flow.ts, src/lib/config/manager.ts, src/lib/output/index.ts) and
verified every field that commit's version of the file exports is actually used, and used
consistently, at every call site:

  • DeviceAuthResponse / TokenResponse — every field device-flow.ts reads
    (verification_uri, user_code, verification_uri_complete, device_code, interval,
    expires_in, access_token, etc.) is present and correctly typed.
  • WaveConfig — matches waveConfigSchema in src/lib/config/schema.ts field-for-field
    (version, currentProject, projects, defaults, telemetry).
  • OutputFormat — matches the "table" | "json" | "yaml" union used throughout output/.

Recreated src/types/index.ts with this verified content rather than guessing or stubbing any.

Restoring WaveConfig as a real nominal type (instead of an implicit any from the unresolved
module) surfaced one genuine new error: src/commands/config/index.ts's getNestedValue /
setNestedValue helpers were typed as Record<string, unknown>, and a nominal WaveConfig
interface (no index signature) is not assignable to that. These two functions are intentionally
generic reflection utilities that walk arbitrary dot-separated config paths — they already
narrow with typeof/in checks internally — so I retyped their parameters as object and cast
only inside the function body where the reflective indexing actually happens. No any, no
@ts-ignore, no @ts-expect-error, no tsconfig.json changes.

Results (measured)

  • npm run type-check: 148 → 142 errors. All 3 TS2307 errors gone. The 2 extra beyond
    "3 fixed" are cascading TS18046 'X' is of type 'unknown' errors that only existed because
    TS treated the import as any when the module couldn't resolve; fixing the module resolution
    fixed those for free. One new, genuine error appeared (the WaveConfig/Record assignability
    above) and was fixed in the same commit — net trace shows zero unaccounted regressions.
  • npm run build (tsup): passes, exit 0, dist/index.js emitted (158.82 KB). This is the
    same "which is exactly why version 1.0.8 shipped printing 1.0.0" mechanism named in the brief:
    tsup transpiles without typechecking, so a source tree that fails tsc --noEmit by 142 errors
    still builds and can still be published — the red type-check is invisible to the release
    pipeline unless something gates on it explicitly.
  • npm run test (vitest): passes, 4 test files / 11 tests, 8.12s.

What remains red, and why

142 errors remain, essentially all of the original ~145 SDK-API-surface-drift group, spread
across 35 files under src/commands/**/index.ts (zoom, podcast, drm, audience, signage, stream,
vault, studio, phone, notify, edge, search, distribution, creator, collab, voice, usb, slides,
sentiment, scene, prism, org, mesh, ghost, editor, desktop, connect, clips, chapters, captions,
analytics, ai, transcribe, qr, marketplace, fleet). Breakdown by error code: 119×TS2339
(property does not exist), 9×TS2551 (property does not exist, "did you mean X"), 8×TS2353
(unknown object-literal property), 3×TS2554 (arg count mismatch), 2×TS2345, 1×TS2561.

These are not left red out of avoidance — I checked whether they're a quick win. The
@wave-av/sdk@2.0.14 .d.ts surface is 8,628 lines across the package; TS's "did you mean X"
suggestions for the TS2551 cases (e.g. DistributionAPI.destinationslistDestinations,
PhoneAPI.conferencesgetConference) are not confirmed 1:1 renames — several look like a
list-vs-single-item shape change (conferences plural vs getConference singular), which means
blind-applying the suggestion risks silently changing behavior rather than fixing a typo. Fixing
this class correctly requires reading each command's business logic against the actual SDK
method signatures file-by-file, which is a legitimately separate, larger pass — not something to
rush through here per the "never silence, only real fixes" instruction. Confirmed via
git status --porcelain that this PR touches exactly src/types/index.ts (new) and
src/commands/config/index.ts (2-line type fix) — nothing else.

Relation to the live 1.0.8-prints-1.0.0 defect

Confirms the brief's hypothesis directly: npm run build succeeds with 148 (now 142) red
type-check errors because tsup never runs tsc. Anything that silently drifted from the SDK's
real shape — including whatever produced the version-string defect — ships anyway. This PR does
not add a type-check gate to CI (out of scope / owned by PR #46 VER-001, which is the actual
fix for the version-surface defect and derives every version string from package.json); it
narrows the type-check red count so that gate, whenever it lands, has less legacy debt to clear.

Peer-PR collision check (done before editing)

Checked gh pr view --json files for all 4 open PRs before touching anything: #48 (release.yml
lint-step removal), #47 (package.json/package-lock.json/license files — LEGAL-001), #46
(src/cli.ts, src/cli.test.ts, src/commands/api/index.ts, src/lib/api-client.ts,
src/lib/version.ts — VER-001), #45 (release.yml — trusted publishing). None of those files
were in this PR's baseline error list (src/commands/api/index.ts has zero type-check errors)
and none overlap the two files this PR touches. package.json/package-lock.json are
untouched here since the measurement showed bumping the SDK pin is not the right move — no
collision with #47 either.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Note

Low Risk
Type-only restoration and narrow signature tweaks to generic config helpers; no runtime behavior or auth/data-path changes.

Overview
Restores the missing src/types/index.ts module so imports in auth, config, and output resolve again. The file defines DeviceAuthResponse, TokenResponse, WaveConfig, and OutputFormat to match existing call sites and the config schema.

In src/commands/config/index.ts, getNestedValue / setNestedValue now take object instead of Record<string, unknown>, with reflective indexing cast inside the body, so a nominal WaveConfig can be passed without assignability errors.

Reviewed by Cursor Bugbot for commit 0406bfe. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by Sourcery

Restore the shared type definitions and align configuration helpers with the typed configuration model without attempting the separate SDK API migration.

Bug Fixes:

  • Restore the missing shared type definitions for device authentication, tokens, configuration, and output formats.
  • Resolve configuration helper typing so the restored nominal configuration type passes type checking.

Enhancements:

  • Reduce strict type-check failures by eliminating unresolved type imports without changing the pinned SDK version.

Tests:

  • Verify the project build and test suite continue to pass after restoring the types.

Review in cubic

…fig typing

src/types/index.ts does not exist on origin/main, causing 3 TS2307 errors
(device-flow.ts, config/manager.ts, output/index.ts) plus cascading
TS18046 'unknown' errors — 5 errors total stemmed from this one gap.

Recovered the exact type shapes from an unmerged historical commit
(382adee, branch fix/remediation-af8a172e-5f3f12, PR #19 closed unmerged)
and verified each field against current call-site usage:
- DeviceAuthResponse / TokenResponse match every field device-flow.ts reads
  (verification_uri, user_code, verification_uri_complete, etc.)
- WaveConfig matches waveConfigSchema in config/schema.ts exactly
- OutputFormat matches the "table" | "json" | "yaml" union used everywhere

Restoring WaveConfig as a real interface (not implicit any) surfaced one
genuine new error: src/commands/config/index.ts's getNestedValue/
setNestedValue helpers were typed as Record<string, unknown>, which a
nominal WaveConfig is not assignable to. Retyped both helpers' parameter
as `object` (they already narrow via typeof/instanceof internally) since
they are intentionally generic reflection utilities over arbitrary nested
config paths — no `any`, no `@ts-ignore`, no loosened tsconfig.

Verified: SDK pin bump (2.0.14 -> 2.1.3) was tested separately and does
NOT reduce the ~145 API-surface-drift errors (148 -> 152, i.e. it makes
things worse by breaking vitest's ambient types) — that group is left
untouched pending a dedicated pass against the real 2.0.14 client surface.

type-check: 148 -> 142 errors (all 3 TS2307 gone, zero new regressions
introduced beyond the one WaveConfig assignability fixed above)
build: passes (tsup transpiles without typechecking)
test: 4 files / 11 tests passing
@codeant-ai

codeant-ai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your workspace is out of credits. Ask your workspace admin to add credits to resume reviews. Manage billing

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @yakimoto, this account has used its review budget of 2,500,000 diff characters for the last 7 days.

You can request another review in 2 hours and 1 minute by commenting @sourcery-ai review.

@cursor

cursor Bot commented Sep 4, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_1aab45b8-1a62-46a6-9cc2-3e4f0410e087)

@sourcery-ai

sourcery-ai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Reviewer's Guide

Restores the missing shared type module with verified DeviceAuthResponse, TokenResponse, WaveConfig, and OutputFormat definitions, and updates configuration reflection helpers to work with the nominal WaveConfig type. Type-check errors drop from 148 to 142, while build and tests pass; the remaining SDK API-surface drift is intentionally out of scope and dependency pins are unchanged.

Flow diagram for restored type-checking path

flowchart LR
  Imports["Device flow, config manager, output imports"] --> Types["src/types/index.ts restored"]
  Types --> Typed["DeviceAuthResponse, TokenResponse, WaveConfig, OutputFormat"]
  Typed --> Reflection["getNestedValue / setNestedValue accept object"]
  Reflection --> TypeCheck["Type-check errors: 148 → 142"]
  Types --> Build["npm run build passes"]
  Types --> Tests["npm run test passes"]
Loading

File-Level Changes

Change Details Files
Restored the missing shared type definitions used by authentication, configuration, and output modules.
  • Added typed device authorization and token response interfaces.
  • Added WaveConfig matching the configuration schema.
  • Added the OutputFormat union used by output handling.
src/types/index.ts
Adjusted configuration reflection helpers to accept nominal configuration objects without weakening type safety.
  • Changed helper inputs from Record<string, unknown> to object.
  • Kept reflective indexing localized behind an explicit cast inside setNestedValue.
src/commands/config/index.ts
Validated the SDK dependency hypothesis without changing dependency versions.
  • Measured that upgrading the SDK did not resolve the existing API drift errors.
  • Left package manifests unchanged and scoped the PR to missing-type restoration.
package.json
package-lock.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 3c4af75d-b9d7-479c-a39d-8027ebb37b0e

📥 Commits

Reviewing files that changed from the base of the PR and between 5899f5b and 0406bfe.

📒 Files selected for processing (2)
  • src/commands/config/index.ts
  • src/types/index.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: smoke (22)
  • GitHub Check: smoke (20)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
src/types/index.ts (1)

1-8: LGTM!

Also applies to: 10-15, 17-28, 30-30

src/commands/config/index.ts (1)

66-66: LGTM!

Also applies to: 78-80


📝 Summary

Summary by CodeRabbit

  • Refactor
    • Improved internal configuration handling without changing runtime behavior.
    • Added shared type definitions to improve consistency across authorization, token, configuration, and output-format handling.
  • Compatibility
    • Existing configuration behavior and supported functionality remain unchanged.
    • No user-facing workflow or interface changes are included in this update.

Walkthrough

The change adds exported TypeScript types for authentication, configuration, and output formats. It also updates configuration helper parameter types and adds explicit casts for indexed access. Runtime behavior remains unchanged.

Changes

Configuration and Type Definitions

Layer / File(s) Summary
Public type contracts
src/types/index.ts
Adds exported types for device authorization responses, token responses, Wave configuration, telemetry settings, default settings, and supported output formats.
Configuration helper typing
src/commands/config/index.ts
Updates nested configuration helpers to accept object values and uses Record<string, unknown> casts for indexed access and mutation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 0406b

This restores exported SDK type contracts and adjusts internal helper typing without changing runtime behavior. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the restored shared types, the configuration helper changes, the measured type-check improvement, and the remaining SDK errors.
Title check ✅ Passed The title accurately identifies the main change: restoring src/types/index.ts. It also provides relevant context about the SDK drift measurement.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sdk-pin-and-types
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/sdk-pin-and-types

Comment @coderabbitai help to get the list of available commands.

@macroscopeapp

macroscopeapp Bot commented Sep 4, 2026

Copy link
Copy Markdown

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a narrowly scoped type-only restoration and annotation correction; the added declarations and casts are erased from the production bundle, while existing configuration, authentication, and output behavior remain unchanged. Both modified files are within the author's ownership area.

Not approved because:

  • Credit balance exhausted. Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

@gitar-bot

gitar-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

Note

Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by October 1. Add seats for more headroom.
Learn more

Code Review ✅ Approved

Restores the missing src/types/index.ts module with verified type definitions for device auth, tokens, configuration, and output formats, resolving three import errors. Updates getNestedValue and setNestedValue in src/commands/config/index.ts to accept object parameters with internal casting, eliminating assignability issues with the nominal WaveConfig type. Type-check errors reduced from 148 to 142 with no regressions; build and tests pass.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@yakimoto
yakimoto merged commit 32d4ab7 into main Sep 4, 2026
23 of 24 checks passed
@yakimoto
yakimoto deleted the fix/sdk-pin-and-types branch September 4, 2026 02:51
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