Conversation
Switch from pinned git commit hash to npm registry. Spans three breaking releases: API spec sync, Person.Id type fix, and card step assignees→assignee_ids rename (no impact — plugin uses Card, not CardStep).
v0.7.x types cover Person.name, CampfireLine.id, Comment.id, Todo.id, Todo.title, Message.id, Message.subject, Boost.id, QuestionAnswer.id, and ListResult<T> extends Array<T>. Remove 17 as-any casts across six files and use SDK's Person type in the resolver adapter.
There was a problem hiding this comment.
Pull request overview
This PR updates the plugin to use the npm-published @37signals/basecamp SDK v0.7.2 and leverages improved upstream typings to remove unsafe casts and align adapters with SDK entity types.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Bump
@37signals/basecampfrom a pinned Git hash to npm version0.7.2. - Remove multiple
as anycasts by relying on updated SDK response types. - Switch resolver adapter matching to the SDK’s
Persontype.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/outbound/send.ts | Uses typed SDK result IDs for outbound message/comment posting. |
| src/basecamp-client.ts | Uses typed profile.name from people.me() when resolving the current person. |
| src/adapters/resolver.ts | Moves person resolution to SDK Person typing and removes list casts. |
| src/adapters/directory.ts | Removes prior ad-hoc shapes/casts and relies on SDK list return types. |
| src/adapters/agent-tools.ts | Removes as any from tool results and returns typed fields. |
| src/adapters/actions.ts | Removes as any for boost creation result handling. |
| package.json | Switches Basecamp SDK dependency to 0.7.2 from npm registry. |
| package-lock.json | Updates lockfile to reflect registry-resolved SDK and its peer dependency metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Bare `let` declarations were implicitly `any`, losing the type-safety gained from removing the `as any` casts. Import and annotate with SDK types so the compiler checks property access end-to-end.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@37signals/basecampfrom v0.6.0 (pinned git hash) to v0.7.2 (npm registry)as anycasts across six files now that the SDK's types cover the accessed propertiesPersontype in the resolver adapter instead of the plugin-localBasecampPersonv0.7.x brings API spec sync,
Person.Idtype fix, andCardStep.assignees→assignee_idsrename (no impact — plugin usesCard, notCardStep).What's NOT adopted
Test plan
tsc --noEmitpasses cleanbiome checkpasses (no new warnings introduced)