Repository navigation
Correct column move, question update, lineup marker, project timesheet and event boost contracts - #991
Conversation
github-actions
Bot
added
typescript
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🔵 Needs a closer look
Breaking API changes across seven SDKs warrant human approval once the outstanding cross-language verification is complete.
1 open finding
What changed in this PR
Aligns five API contracts across the Smithy specification, generated SDKs, and Go wrappers.
Changes:
- Corrects column-move targets and question updates, and returns saved lineup markers.
- Exposes project timesheet availability and documents event-boost restrictions.
- Updates regression tests, fixtures, and migration guidance.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| typescript/tests/services/lineup.test.ts | Asserts returned markers. |
| typescript/tests/services/checkins.test.ts | Updates questions by title. |
| typescript/tests/services/card-columns.test.ts | Checks table target and position. |
| typescript/src/generated/services/lineup.ts | Returns markers from writes. |
| typescript/src/generated/services/checkins.ts | Removes paused updates. |
| typescript/src/generated/services/card-columns.ts | Corrects move documentation. |
| typescript/src/generated/services/boosts.ts | Updates boost documentation. |
| typescript/src/generated/schema.d.ts | Reflects corrected contracts. |
| typescript/src/generated/openapi-stripped.json | Synchronizes OpenAPI contracts. |
| typescript/src/generated/metadata.ts | Refreshes generation timestamp. |
| swift/Sources/Basecamp/Generated/Services/LineupService.swift | Returns saved markers. |
| swift/Sources/Basecamp/Generated/Models/UpdateQuestionRequest.swift | Removes paused input. |
| swift/Sources/Basecamp/Generated/Models/Project.swift | Adds timesheet availability. |
| spec/fixtures/projects/get.json | Includes enabled timesheet. |
| spec/basecamp.smithy | Defines the five contract corrections. |
| rust/basecamp-sdk/src/generated/types.rs | Updates request and response models. |
| rust/basecamp-sdk/src/generated/services/lineup.rs | Decodes saved markers. |
| rust/basecamp-sdk/src/generated/services/checkins.rs | Documents pause/resume alternatives. |
| rust/basecamp-sdk/src/generated/services/card_columns.rs | Updates move documentation. |
| rust/basecamp-sdk/src/generated/services/boosts.rs | Documents boost restrictions. |
| rust/basecamp-sdk/src/generated/routes.rs | Marks marker responses as JSON. |
| ruby/test/basecamp/services/lineup_service_test.rb | Asserts returned markers. |
| ruby/test/basecamp/services/checkins_service_test.rb | Removes paused update input. |
| ruby/test/basecamp/services/card_columns_service_test.rb | Uses table destination. |
| ruby/lib/basecamp/generated/types.rb | Adds timesheet availability. |
| ruby/lib/basecamp/generated/services/lineup_service.rb | Returns marker response data. |
| ruby/lib/basecamp/generated/services/checkins_service.rb | Removes paused updates. |
| ruby/lib/basecamp/generated/services/card_columns_service.rb | Corrects move parameter documentation. |
| ruby/lib/basecamp/generated/services/boosts_service.rb | Updates boost documentation. |
| ruby/lib/basecamp/generated/metadata.json | Refreshes generation timestamp. |
| python/src/basecamp/generated/types.py | Updates project and question types. |
| python/src/basecamp/generated/services/lineup.py | Returns markers synchronously and asynchronously. |
| python/src/basecamp/generated/services/checkins.py | Removes paused updates. |
| python/src/basecamp/generated/services/card_columns.py | Corrects move documentation. |
| python/src/basecamp/generated/services/boosts.py | Documents boost restrictions. |
| openapi.json | Publishes corrected wire contracts. |
| MIGRATING.md | Explains compatibility changes. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/Types.kt | Removes paused input. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/lineup.kt | Decodes saved markers. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/checkins.kt | Removes paused serialization. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/card-columns.kt | Updates move documentation. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/boosts.kt | Updates boost documentation. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Project.kt | Adds timesheet availability. |
| go/pkg/generated/client.gen.go | Updates generated models and parsers. |
| go/pkg/basecamp/projects.go | Exposes timesheet availability. |
| go/pkg/basecamp/projects_test.go | Tests timesheet field propagation. |
| go/pkg/basecamp/lineup.go | Returns markers through shared conversion. |
| go/pkg/basecamp/lineup_test.go | Tests marker write responses. |
| go/pkg/basecamp/checkins.go | Rejects empty question updates. |
| go/pkg/basecamp/checkins_test.go | Tests update validation and serialization. |
| go/pkg/basecamp/catalog/catalog.json | Synchronizes operation contracts and documentation. |
| go/pkg/basecamp/cards.go | Defaults move target to card table. |
| go/pkg/basecamp/cards_test.go | Tests default and explicit targets. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Local verification at 07dd413: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07dd413386
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Review threads: 4 resolved (4 fixed; one also declines a part, with the reasoning in its thread). |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1a8e78d to
126cbb8
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Nothing open on 126cbb8. |
… timesheet and event boost semantics - MoveCardColumn: target_id is the card table (bc3 Kanban::Moves::ColumnMove refuses any other destination with 422); position is the 1-based slot among the columns. Go Move defaults an unset TargetID to the card table. - UpdateQuestion: drop paused, which bc3 never permits on update; Go refuses an update with neither title nor schedule before the wire. - Create/UpdateLineupMarker return the saved marker bc3 renders. - Project gains timesheet_enabled. - Event boost operations document which events are boostable and the 403/404s.
The TypeScript, Ruby and Kotlin service generators keep only the first line of an operation's description, which cut the event boost and question update docs mid-sentence.
…docs Generators other than Go drop member docs, so the card-table target_id and the whole-schedule replacement on question updates now live in the operation descriptions every generator keeps. Position docs no longer promise that a negative slot is accepted.
126cbb8 to
425d18c
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Five SDK contracts that disagree with what bc3 actually does. A production every-tool pass on the Basecamp MCP server (v0.2.7) found them. Each cause is in this SDK's spec or Go wrapper, so each fix belongs here. The MCP server only documents what the SDK sends and returns. All cited bc3 behavior is inside the revision
spec/api-provenance.jsoncurrently pins, and the cited code is unchanged on bc3 HEAD, so no repin.Changes
A. MoveCardColumn:
target_idis the card table,positionis a 1-based slotapp/models/kanban/moves/column_move.rbvalid_destinationrequiresdestination.kanban_board?.Kanban::MovesController#createrescuesActiveModel::ValidationErrorto 422.Recording::Positioner#rescoretreats position<= 1as the top and> countas the bottom.doc/api/sections/card_table_columns.mdsays "target_id- the id of the card table" and "position- index among the columns (ignoring Triage, Not Now or Done). Default: 1".target_idasCardColumnId. The Go doc said "the column ID to move relative to" and "zero-indexed position". A caller who followed the docs got 422 on every call; the MCP tool built on them failed every time.target_id: CardTableId, plus doc comments onsource_id,target_idandposition. GoMoveColumnRequestdocs are corrected, andCardColumnsService.Movefills an unset (zero)TargetIDwith the card table it is called with. The TS and Ruby tests now send the card table as the target.B. UpdateQuestion: drop
pausedapp/controllers/questions_controller.rbquestion_paramspermits only:titleandschedule: [...].doc/api/sections/questions.md"Update a question" says thequestionobject "must includetitle,schedule, or both".pausedsent with a title was dropped silently and the call reported success, so the question stayed unpaused. Pausing has its own operations (PauseQuestion/ResumeQuestion).pausedis removed fromUpdateQuestionInput, and the doc points to pause/resume. GoUpdateQuestionnow refuses an update with neither a title nor a schedule before it reaches the wire (ErrUsage).C. Create/UpdateLineupMarker return the marker
app/controllers/lineup/markers_controller.rbcreaterenders@marker.as_json(except: :account_id), status: :created, andupdaterenders the same with:ok. This has been true since bc3 f85369fbad4 (#9946, 2026-03-18).LineupMarker. GoCreateMarkerandUpdateMarkerreturn(*LineupMarker, error).doc/api/sections/lineup_markers.mdstill says "201 Createdwith an empty response body" and "200 OKwith an empty response body". Those lines are stale.D. Project gains
timesheet_enableddoc/api/sections/projects.mdresponse examples includetimesheet_enabled.TimesheetsController#set_recordingraisesRecordNotFoundunlessproject.timesheet_recordingexists, andProject::Timesheetable#record_timesheetcreates it only once the project hastimesheet_enabled.GET /projects/:id/timesheet.jsonanswered 404.Project.timesheet_enabledis added, with a doc comment about the 404. Go:Project.TimesheetEnabled bool.ProjectOperations#create_project_paramspermitstimesheet_enabledon project create and update, but only for people withcan_access_timesheet?.doc/apidoesn't document it as a write parameter, so this PR doesn't add it to the inputs.E. Event boosts: which events are boostable
app/models/event.rb:23BOOSTABLE_ACTIONS = %w[ completed adopted column_changed moved_in ].BoostsController#ensure_boostableanswers 403 on index and create for any other event.#ensure_comments_are_openanswers 403 on create when the recording's comments are closed.#set_boostableanswers 404 (render_not_found) for a recording the person can't see.ListEventBoosts,CreateEventBoostandEvent.boosts_count/boosts_url. The fields were already in the spec. This change is docs only.doc/api/sections/boosts.mdlistscompleted,adoptedandcolumn_changedbut leaves outmoved_in.Breaking changes (in MIGRATING.md under v0.25.0)
UpdateQuestionRequest.Pausedis removed. Every SDK's update-question request type losespaused.LineupService.CreateMarkerandUpdateMarkernow return(*LineupMarker, error)instead oferror. The other SDKs return the marker where they returned nothing.Verification
TestCardColumnsService_MoveTargetsTheCardTable,TestCheckinsService_UpdateQuestionRequiresTitleOrSchedule,TestLineupService_MarkerWritesReturnTheMarker,TestProjectFromGenerated_TimesheetEnabled, plus theTimesheetEnabledassertion againstspec/fixtures/projects/get.json.make generatepasses (exit 0).ruby/lib/basecamp/generated/metadata.json,types.rbandtypescript/src/generated/metadata.ts, resolved by rerunningmake generateat each commit. All 62make checktargets pass at 425d18c.Overlap
#990 and #988 have merged; this PR is rebased on top of both, and the combined
ListMarkerskeeps #990's empty-not-nil initialization.Summary by cubic
Corrects five SDK contracts that disagreed with what Basecamp actually does, found in a production pass on the MCP server. Each fix brings the spec, generated SDKs, and Go wrapper in line with bc3 behavior.
Contract fixes
MoveCardColumn.target_idis now the card table, not the column; Basecamp rejects any other destination with 422, andpositionis the 1-based slot, with negative values refused. Go'sMovefills an unset target with the card table it is called with.UpdateQuestiondropspaused, which Basecamp never reads on update; Go refuses an update with neither title nor schedule before the wire, and a schedule replaces the whole schedule.Projectgainstimesheet_enabledso callers can tell why the timesheet report answers 404.Breaking changes
pausedis removed from every SDK's update-question request type; Go'sUpdateQuestionRequest.Pausedis gone.LineupService.CreateMarker/UpdateMarkerreturn(*LineupMarker, error); other SDKs return the marker where they returned nothing.MIGRATING.mdunder v0.25.0.Written for commit 425d18c. Summary will update on new commits.