Skip to content

Latest commit

 

History

History
656 lines (522 loc) · 27.2 KB

File metadata and controls

656 lines (522 loc) · 27.2 KB

Contributing to Basecamp SDK

Thank you for your interest in contributing to the Basecamp SDK. This document provides guidelines and instructions for contributing.

Development Setup

Prerequisites

SDK Requirements
Go Go 1.26+, golangci-lint
TypeScript Node.js 22.12+, npm
Ruby Ruby 3.2+, Bundler
Swift Swift 6.0+, Xcode 16+
Kotlin JDK 17+, Kotlin 2.0+
Python Python 3.11+, uv
Rust Rust 1.88+ (MSRV; CI pins 1.98.1 via rust/rust-toolchain.toml), cargo-deny

Shared tooling: jq, and bash >= 4.4 on PATH for the pairwise-canary scripts that make check runs (macOS ships bash 3.2 at /bin/bash — brew install bash; the scripts fail fast with this exact hint).

A Basecamp account is optional (for integration testing only).

Getting Started

  1. Clone the repository:

    git clone https://github.com/basecamp/basecamp-sdk.git
    cd basecamp-sdk
  2. Install dependencies and run tests for each SDK:

    Go:

    cd go && go mod download
    make test
    make check   # formatting, linting, tests

    TypeScript:

    cd typescript && npm ci
    npm test
    npm run typecheck
    npm run lint

    Ruby:

    cd ruby && bundle install
    bundle exec rake test
    bundle exec rubocop

    Swift:

    cd swift
    swift build
    swift test

    Kotlin:

    cd kotlin
    ./gradlew :sdk:jvmTest

    Python:

    cd python && uv sync && cd ..
    make py-test
    make py-check   # tests, types, lint, format, drift

    Rust:

    make rs-test
    make rs-check   # fmt, clippy, tests, docs, deny, drift, publish dry-run
  3. Run all SDKs at once from the repo root:

    make check        # all 7 SDK test suites
    make conformance  # cross-SDK conformance tests

Code Style

Python Code

  • Target Python 3.11+
  • Use ruff for linting and formatting (line length: 120)
  • All service method parameters are keyword-only (after *)
  • Use type annotations for function signatures
  • Generated code under src/basecamp/generated/ is exempt from style rules

Go Code

  • Follow standard Go conventions and Effective Go
  • Use gofmt for formatting (run make fmt)
  • Keep functions focused and small
  • Document all exported types, functions, and methods
  • Use meaningful variable names

Naming Conventions

  • Service types: *Service (e.g., ProjectsService, TodosService)
  • Request types: Create*Request, Update*Request
  • Options types: *Options or *ListOptions
  • Error constructors: Err* (e.g., ErrNotFound, ErrAuth)

Error Handling

  • Return structured *Error types with appropriate codes
  • Include helpful hints for user-facing errors
  • Use ErrUsageHint() for configuration/usage errors
  • Wrap underlying errors with context

Testing

  • Write unit tests for all new functionality
  • Use table-driven tests where appropriate
  • Mock HTTP responses using httptest
  • Test both success and error paths

Never schedule a cancellation on a wall clock

A test that arms setTimeout(… abort …, N) and races it against a mocked response is asserting machine load, not behavior. Abort from a seam that proves the request is already in flight: from inside the MSW handler (typescript/tests/client.test.ts), or from the retry hook the loop fires immediately before it sleeps (typescript/tests/retry-after.test.ts, typescript/tests/middleware-lifecycle.test.ts). Then replace any accompanying Date.now() - startedAt < N bound. Usually there is nothing to put in its place: the error's identity (AbortError vs TimeoutError, or err === reason) and the attempt ledger already discriminate, and the ceiling was contributing only load sensitivity.

But check what the ceiling was carrying before you delete it. If the named behavior is promptness — "rejects as soon as the abort lands" rather than "rejects with the right reason, eventually" — identity and the ledger do not cover it: a sleep that notices the abort and then waits out its timer anyway satisfies both, late. Assert promptness by freezing the clock instead of bounding it. Fake setTimeout/clearTimeout (vi.useFakeTimers({ toFake: […] })), never advance it, and assert the request settled after a barrier counted in event-loop turns (setImmediate), not milliseconds — a delay that only the test can release cannot have been waited out. middleware-lifecycle.test.ts's "rejects promptly when the caller aborts during a retry backoff" is the worked example; the mutant it kills is precisely the sleep described above.

Restore real timers from the suite's afterEach, not from the test's own finally: a timed-out test is never resumed, so its finally does not run — and a hung test is exactly the failure a broken abort produces — which would leak a frozen clock into every test after it.

The same rule reads sideways in Ruby: a value written by a server thread is read by the test only across an explicit happens-before edge — a Queue, or a join — never "in practice, via the socket close" (#739).

This half is enforced, so you should not need to remember it. typescript/lint-rules/no-timer-scheduled-abort.js is an oxlint rule that fails the build on a timer-scheduled abort() anywhere under typescript/tests/:

cd typescript && npm run test:lint-rules && npm run lint:test-timers

Both run in make ts-check and as their own steps in the TypeScript CI job. Run the self-test first and always — oxlint's JS plugin API is alpha, so a version bump could disarm the rule silently, and the self-test is what turns that into a build failure instead of a green gate enforcing nothing.

A test whose subject is a caller's own timer-driven abort is a legitimate exception; suppress it at the site with the reason, never by widening the rule:

// oxlint-disable-next-line basecamp-tests/no-timer-scheduled-abort -- why

#655 tried to scope this class with an rg typed into an issue body that required a no-argument abort(). Both survivors passed a reason, so they shipped and one later went red in CI (#783) — which is why the rule reads syntax rather than text: it can tell a timer that schedules an abort from one the abort is merely racing, and a proximity selector cannot.

What the rule does not cover is stated in its own header rather than restated here: a renamed timer, a callback passed by reference, a computed .abort access whose key is not a literal. The honest population bound is rg -n "abort\s*\(|\[[\"']abort[\"']\]\s*\(" typescript/tests — every spelling the rule treats as an abort, not dot-member access alone, so a later sweep cannot come back clean while a bare abort(), a controller["abort"]() or a controller['abort']() is live (the rule reads the literal's value, so either quote style is an abort to it). Classify each site by what makes the abort land, not by the shape it is written in. The rest of this rule, above, is judgment the linter cannot hold: which assertion is the discriminating one, and when a wall-clock ceiling is redundant with it.

Commit Conventions

We follow Conventional Commits for clear, structured commit history.

Format

<type>(<scope>): <description>

[optional body]

[optional footer(s)]

Types

  • feat: New feature
  • fix: Bug fix
  • docs: Documentation changes
  • style: Code style changes (formatting, semicolons, etc.)
  • refactor: Code changes that neither fix bugs nor add features
  • perf: Performance improvements
  • test: Adding or updating tests
  • build: Build system or dependency changes
  • ci: CI configuration changes
  • chore: Other changes that don't modify src or test files

Scope

Use the service or component name:

  • projects, todos, campfires, webhooks, etc.
  • auth, client, config, errors
  • docs, ci, deps

Examples

feat(schedules): add GetEntryOccurrence method

fix(timesheet): use bucket-scoped endpoints for reports

docs(readme): add error handling section

test(cards): add coverage for move operations

Pull Request Process

Before Submitting

  1. Run all checks locally:

    make check  # runs all 7 SDK test suites from repo root
  2. Ensure conformance tests pass:

    make conformance
  3. Update documentation if adding new features

Submitting a PR

  1. Create a feature branch from main:

    git checkout -b feat/my-feature
  2. Make your changes with clear, focused commits

  3. Push and open a pull request against main

  4. Fill out the PR template with:

    • Summary of changes
    • Motivation and context
    • Testing performed
    • Breaking changes (if any)

Review Process

  • All PRs require at least one review
  • CI must pass (tests, linting, security checks)
  • Address review feedback promptly
  • Squash commits if requested

Adding New API Coverage

All SDKs are generated from a single Smithy specification. When adding support for new Basecamp API endpoints:

  1. Edit the Smithy model (spec/basecamp.smithy)

    • Define the resource, operations, and shapes
    • Follow patterns from existing resources (e.g., Project, Todo)
  2. Regenerate everything in one step:

    make generate

    This runs Smithy build, behavior model, URL routes, provenance sync, the per-language generators (TypeScript, Ruby, Python, Kotlin, Swift, Go, Rust), and finally a constants sync — which has to come last, because some marked doc constants are derived from the generated accessors those generators produce. Read the generate target for the exact order rather than reproducing it.

  3. Run per-SDK generators individually if you only need one:

    • Go: make go-check-drift — Go services are hand-written wrappers around the generated client; the drift check verifies all generated operations are covered
    • TypeScript: make ts-generate-services
    • Ruby: make rb-generate-services
    • Swift: make swift-generate
    • Kotlin: make kt-generate-services
    • Python: make py-generate
    • Rust: make rs-generate

    go/pkg/basecamp/eventfeed/ is outside that drift check by design: it is hand-written §23 infrastructure rather than a wrapper over generated operations, so nothing about it is derivable from the spec. It is verified instead by the tier-2 conformance driver in the package, which replays every fixture under conformance/event-feed/fixtures/.

  4. Add tests for each SDK

  5. Add conformance tests (conformance/tests/) covering the new operations

    A NEW fixture file also needs a row in SPEC.md §19's Test Categories table and a row in its Appendix D, naming the spec sections that own it. make doc-constants-check fails until both exist. Until this checklist item and that gate, the convention was written down nowhere, which is why three fixtures in a row landed with one row, the other, or neither.

  6. Update documentation:

    • Add to the services table in each SDK's README

    • Note which release-note section the change belongs in. There is no CHANGELOG in this repo — release notes are generated from PR labels by .github/release.yml, and an unlabeled PR falls into Other. Applying labels needs triage rights, so if you don't have them, say which section fits in the PR description and a maintainer will label it:

      Section Labels
      ⚠️ Breaking Changes breaking
      Features enhancement
      Bug Fixes bug
      CI & Infrastructure ci, dependencies, github-actions
      Documentation documentation
      Other anything else

      A PR may carry several — breaking plus bug is common. A PR lands in the first matching section in the table's order, so an extra low-priority label is harmless once the right one is applied. Language and area labels (go, python, spec, conformance, …) are applied automatically from changed paths and don't affect categorization; github-actions is the one auto-applied label that does, filing otherwise-unlabeled workflow PRs under CI & Infrastructure. documentation and dependencies are applied by hand (Dependabot labels its own PRs), never by the path labeler — a feature that touches docs or a manifest should read as a feature.

    • If the change breaks consumers, add a section to MIGRATING.md. Label-generated notes list what merged; they cannot say which changes a consumer must react to, what wrong behaviour they get if they don't, or which breaks are silent. That is what MIGRATING.md carries, and it is the only place it lives — there is no CHANGELOG, and the GitHub Release body is built entirely by .github/workflows/release-github.yml. That workflow links MIGRATING.md from every release automatically, so nothing has to be remembered at tag time; what does have to happen is that the section exists before the tag is pushed.

Spec-shape lints

The repo enforces a small set of structural invariants on the OpenAPI spec beyond the language-specific drift checks. These run as part of make check:

  • Bucket↔flat parity (make check-bucket-flat-parity): every GET /{accountId}/buckets/{bucketId}/<resource>(/...).json list operation must have a flat counterpart at /{accountId}/<resource>.json, or be justified in spec/bucket-scoped-allowlist.txt. The intent is that cross-project SDK consumers shouldn't have to walk every project to query account-wide resources.

    When adding a bucket-scoped list endpoint, either add the matching flat endpoint or append a one-line entry to the allowlist with a justification comment.

Live canary

The TypeScript runner also drives a live canary against a real Basecamp backend. It dispatches every operation in conformance/tests/live-my-surface.json through the SDK's typed surface, captures the raw wire response (bytes + headers), and validates each response body against the OpenAPI response schema. Forward-compat additions on the wire surface as "extras observed" in the run summary — never as failures — so new BC5 fields don't break the canary while still being visible.

The canary is opt-in and does not run as part of make check:

BASECAMP_LIVE=1 \
BASECAMP_TOKEN=<your-token> \
BASECAMP_ACCOUNT_ID=<your-account> \
make conformance-typescript-live

Optional env:

  • BASECAMP_HOST — backend origin only (e.g. https://3.basecampapi.com); the runner appends /{accountId} to mirror createBasecampClient's default URL composition. Optional for the bare conformance-typescript-live target only — when unset the SDK client falls back to its default origin. The conformance-canary orchestrator and the CI workflow require it (no hard-coded default), so the production BC5 origin is always explicit there.
  • BASECAMP_BACKEND=bc5 — the Backend label that namespaces persisted snapshots and selects the BASECAMP_<BACKEND>_* fixture overrides. Unset defaults to unknown, which disables only those per-backend overrides; generic BASECAMP_<FIXTURE> overrides (and then discovery) still apply. The canary uses bc5 — production runs BC5.
  • LIVE_RECORD_DIR=<path> — persists wire snapshots to <path>/<backend>/wire/<test>.json. Consumed by the cross-language replay runners (make conformance-*-replay).
  • BASECAMP_BC5_PROJECT_ID / BASECAMP_PROJECT_ID etc. — explicit fixture-IDs override the runner's discovery walk. The BASECAMP_<BACKEND>_* form keys off BASECAMP_BACKEND (bc5 for the canary). Same pattern applies for TODOSET_ID, TODOLIST_ID, TODO_ID.

Tests skip with a clear skipReason when a fixture-ID can't be resolved (no env override, no discovery match) — they don't fail.

Adding an operation to the live canary requires both a fixture entry in live-my-surface.json and a dispatch case in conformance/runner/typescript/live-dispatch.ts. The runner's startup gate refuses to run if any fixture operation lacks a dispatch.

Because live canary fixtures live in the shared conformance/tests/ directory, offline conformance runners must treat mode as part of the shared schema and execute only mock tests: omitted mode or mode: "mock". mode: "live" entries belong to the TypeScript live wire-capture runner and the cross-language wire-replay runners described next.

Wire replay (cross-language)

The TypeScript live runner is the single canonical wire-capturer. When invoked with LIVE_RECORD_DIR=<path>, it persists every captured response to <path>/<backend>/wire/<test>.json with the snapshot format { operation, pages: [{status, headers, body, bodyText, url}], pages_count }.

The Ruby, Python, Go, and Kotlin runners each have a wire-replay mode that reads those snapshots and decodes each page through their SDK. No HTTP calls, no mock servers — the input is the canonical wire bytes captured by the TS runner. Decode results land at <path>/<backend>/decode/<language>/<test>.json with the format { schema_version, operation, pages: [{decoded, decode_error, missing_required, extras_seen}] }.

Each runner enforces three coverage gates at startup before doing any decode work:

  1. Decoder coverage — every operation in live-my-surface.json has a decode case in this runner.
  2. Snapshot completeness — every operation in live-my-surface.json has a corresponding snapshot file at <path>/<backend>/wire/.
  3. Snapshot recognition — every snapshot's operation field is in live-my-surface.json (catches drift between TS dispatch and the shared fixture).

Each gate prints which operations triggered it so the operator can fix the right side: TS dispatch, the fixture, or the replay runner.

Two-stage flow:

# Step 1: TS captures canonical wire snapshots (live HTTP, requires creds).
BASECAMP_LIVE=1 \
BASECAMP_TOKEN=<token> \
BASECAMP_ACCOUNT_ID=<account> \
BASECAMP_BACKEND=bc5 \
LIVE_RECORD_DIR=tmp/canary \
make conformance-typescript-live

# Step 2: each language replays those snapshots through its SDK (offline).
for lang in ruby python go kotlin; do
  WIRE_REPLAY_DIR=tmp/canary BASECAMP_BACKEND=bc5 \
    make conformance-$lang-replay
done

Step 2 needs no credentials and no network — it's pure decode + walk. The extras-observed output across languages is a consistency check on the hand-rolled schema walkers (which mirror the TS validator's required + extras algorithm in each language). Any per-language divergence in extras_seen points at a walker bug in the diverging language.

When the SDK gains a new operation in live-my-surface.json, it must be added to:

  • conformance/runner/typescript/live-dispatch.ts — TS dispatch case.
  • conformance/runner/ruby/replay-runner.rb — Ruby decoder.
  • conformance/runner/python/replay_runner.py — Python decoder.
  • conformance/runner/go/replay_runner.go — Go decoder.
  • kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt — Kotlin decoder.

Each runner's coverage gate refuses to start until all five are in place — but that gate only fires during a live canary, and the scheduled canary skips whenever its secrets are unconfigured, so for a long stretch four of the five tables sat twenty operations behind the fixture with CI fully green (#553). scripts/check-replay-decoder-parity now compares all five tables against live-my-surface.json statically on every make check and CI run, and runs as a prerequisite of make conformance-live so a mismatch fails in under a second instead of after the capture. It pins the live-operation count too: adding a live fixture entry means bumping expected_live_ops and registering the decoder in the same commit.

Historical note — pairwise BC4↔BC5 comparison (retired). An earlier canary captured snapshots from two live backends (BC4 and BC5) and asserted BC5 was an additive superset of BC4, encoded as pairwiseAssertions in the fixture and applied by scripts/compare-canary-runs.sh. BC5 replaced BC4 in production, so there is no live BC4 backend left to compare against — the pairwise machinery is retired. Its one live rule (the memories additive-only waiver) is settled by documented contract; see COORDINATION.md and spec/api-gaps/memories-emptied-regression.md. The pairwise engine remains recoverable from git history (PR #308) if a reachable legacy backend ever warrants restoring it.

Orchestrator

make conformance-canary runs the full single-backend canary against production BC5 — capture, schema-validate, and 4-language decode-replay in one pass:

BASECAMP_TOKEN=<token> \
BASECAMP_ACCOUNT_ID=<account> \
BASECAMP_HOST=https://3.basecampapi.com \
make conformance-canary

What it runs:

  1. Clears LIVE_RECORD_DIR (default tmp/live-canary, behind a hardened rm -rf guard).
  2. BASECAMP_BACKEND=bc5 LIVE_RECORD_DIR=tmp/live-canary make conformance-live — TS captures wire snapshots from production BC5, then Ruby/Python/Go/Kotlin replay-decode.

Override LIVE_RECORD_DIR (default tmp/live-canary) on the make line. BASECAMP_HOST is required — the production origin has no safe hard-coded default.

Scheduled CI

.github/workflows/live-canary.yml runs conformance-canary nightly via cron and on workflow_dispatch. It is opt-in: the workflow no-ops with a clear log message if the required secrets aren't configured.

Provision the CANARY_BASECAMP_* credentials as environment secrets on the basecamp-canary environment — not as org- or repo-level secrets. Only environment secrets are scoped to the credential: an org/repo secret of the same name is readable by any workflow in this repository, so the environment would gate this job but not the secret. As environment secrets, they are readable only by a job that references environment: basecamp-canary and clears its protection rules. CANARY_BASECAMP_* is a naming convention other app repos can mirror (each on its own <app>-canary environment) — a shared convention, not a shared org secret.

The environment's deployment-branch policy additionally restricts those jobs to the default branch, so a non-default-branch workflow_dispatch is normally refused before it runs (repo admins can bypass environment protection rules by default, so treat that as defense-in-depth, not an absolute gate). No required reviewers, so scheduled runs never hang. The tooling reads the secrets under the fixed runtime env-var names:

  • CANARY_BASECAMP_TOKEN → BASECAMP_TOKEN — OAuth token with read scope for the canary fixtures.
  • CANARY_BASECAMP_ACCOUNT_ID → BASECAMP_ACCOUNT_ID — the numeric account ID.
  • CANARY_BASECAMP_HOST → BASECAMP_HOST — origin of the production BC5 backend.

Decode results are uploaded as a workflow artifact (live-canary-<run-id>, 14-day retention) so failures can be inspected post-hoc without rerunning. Wire snapshots are deliberately excluded — they carry raw response bodies from a live account.

API gap registry (spec/api-gaps/)

When BC ships a new user-visible feature without a JSON API (or with an incomplete one), add an entry under spec/api-gaps/. The registry is the SDK side of the BC3 API parity coordination: the BC3 plan owns server-side delivery; the registry tracks the gap from detection through absorption, with status changes in git history.

To add a new entry:

  1. Copy an existing entry in spec/api-gaps/ as a template.
  2. Set frontmatter status to no-json-contract (or partial-coverage / ambiguous as appropriate). See spec/api-gaps/schema.json for valid statuses.
  3. Add a row to the table in spec/api-gaps/README.md.
  4. Run make validate-api-gaps to confirm frontmatter and required body sections are well-formed. Wired into make check.

For routes that should not warrant an entry (transient nav state, internal endpoints, duplicates of a route already covered elsewhere), add a record to spec/api-gaps/allowlist.yml with a justification.

Releasing the Rust crate for the first time

release-rust.yml publishes to crates.io through Trusted Publishing (OIDC via rust-lang/crates-io-auth-action, on the release-crates environment), and Trusted Publishing can only be configured on a crate that already exists. The first version is therefore pushed by hand, once, by a maintainer with crates.io access; every later release is make release like the other SDKs. Until step 5 below is done, a v* tag FAILS release-rust.yml's publish job — and with it the GitHub Release, which would otherwise publish notes advertising a crate version nobody can install. So the bootstrap happens before the first tag, and steps 1–7 run back-to-back on one SHA. A workflow_dispatch run rehearses packaging from any ref without touching the registry, and never exercises the OIDC exchange; the first real tag after step 5 is the first end-to-end test.

  1. On main, with a clean tree and make check green.
  2. Mint a crates.io API token scoped to publish-new and change-owners, with a one-day expiry.
  3. Publish from the workspace, reproducibly, with the token from step 2 in the environment (never cargo login: that writes it to ~/.cargo/credentials.toml):
    export CARGO_REGISTRY_TOKEN=<token from step 2>
    cd rust && SOURCE_DATE_EPOCH=$(git log -1 --format=%ct) cargo publish -p basecamp-sdk --locked
  4. Hand ownership to the team, still under that token: cargo owner --add github:basecamp:cli basecamp-sdk.
  5. On the crate's settings page, configure Trusted Publishing: owner basecamp, repository basecamp-sdk, workflow release-rust.yml, environment release-crates.
  6. Revoke the token from step 2 and unset CARGO_REGISTRY_TOKEN.
  7. make release VERSION=… from the same SHA, so the tag and the published crate agree.

make bump writes the [package] version in rust/basecamp-sdk/Cargo.toml and refreshes rust/Cargo.lock and conformance/runner/rust/Cargo.lock; make release reads the version back through cargo metadata and runs cargo publish --dry-run --locked before it tags. The workflow publishes only when crates.io answers 404 for that version and fails closed on any other answer.

Recovery after a partial publish. Re-run the failed jobs with gh run rerun <run-id> --failed; the already-published check is idempotent, so a version that did land is skipped rather than re-pushed. Never delete the tag — crates.io versions are permanent, and the tag is the only thing that ties the published bytes back to a commit.

Reporting Issues

  • Use GitHub Issues for bug reports and feature requests
  • Include reproduction steps for bugs
  • Provide Go version and OS information
  • Include relevant error messages and logs

Questions?

Open a GitHub Discussion for questions about contributing or using the SDK.