From 336ec44645e7e9bae4692dee377995dbb248b7c2 Mon Sep 17 00:00:00 2001 From: Olorunfemi20 Date: Sun, 30 Aug 2026 12:30:07 +0100 Subject: [PATCH] contrib: docs and examples for issues 293, 300, 281, 276 Adds four self-contained contrib/examples entries, each scoped to one assigned issue: - issue-293: a guide and fixture-backed test for detecting and migrating locally cached policy data across schema versions, following the same versioned-storage pattern used by the session store. - issue-300: a reusable deprecation pattern (JSDoc @deprecated plus a one-time console warning) for SDK methods superseded by a newer client, with the CHANGELOG entry shape a maintainer would add alongside it. - issue-281: a stage-by-stage release process guide (versioning, changelog, build/verify, publish) describing what's automated versus manual, with links to the actual CI/publish/verify-merged workflow files. - issue-276: a timeout-budget wrapper for the policy deployment RPC calls (simulate, deployInstance, recordDeployment), with a distinct typed PolicyDeployTimeoutError and tests verifying the timeout triggers per configured budget. closes #293 closes #300 closes #281 closes #276 --- .../issue-276-rpc-timeout-budget/README.md | 106 ++++++++++++ .../policy-deploy-timeout.test.ts | 98 +++++++++++ .../policy-deploy-timeout.ts | 153 ++++++++++++++++++ .../RELEASE_PROCESS.md | 121 ++++++++++++++ .../README.md | 99 ++++++++++++ .../policy-schema-migration.test.ts | 87 ++++++++++ .../policy-schema-migration.ts | 95 +++++++++++ .../README.md | 79 +++++++++ .../deprecation-warning.test.ts | 61 +++++++ .../deprecation-warning.ts | 66 ++++++++ 10 files changed, 965 insertions(+) create mode 100644 contrib/examples/issue-276-rpc-timeout-budget/README.md create mode 100644 contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.test.ts create mode 100644 contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.ts create mode 100644 contrib/examples/issue-281-release-process-guide/RELEASE_PROCESS.md create mode 100644 contrib/examples/issue-293-policy-schema-migration-guide/README.md create mode 100644 contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.test.ts create mode 100644 contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.ts create mode 100644 contrib/examples/issue-300-deprecation-warning-pattern/README.md create mode 100644 contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.test.ts create mode 100644 contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.ts diff --git a/contrib/examples/issue-276-rpc-timeout-budget/README.md b/contrib/examples/issue-276-rpc-timeout-budget/README.md new file mode 100644 index 0000000..64304e2 --- /dev/null +++ b/contrib/examples/issue-276-rpc-timeout-budget/README.md @@ -0,0 +1,106 @@ +# Timeout budgets for policy-client deployment RPC calls + +Closes #276. + +`src/policy-client.ts`'s `createPolicyClient` issues `simulate`, +`deployInstance`, and `recordDeployment` requests through a shared +`req()` helper that calls the injected `fetch` with no timeout at all — +a stalled network connection or a slow gateway hangs the call indefinitely, +with no way for a caller to bound how long they're willing to wait. Because +contributor changes are confined to `contrib/`, this entry provides a +self-contained, directly-portable implementation of the fix +([`policy-deploy-timeout.ts`](policy-deploy-timeout.ts)) that a maintainer +can fold into `src/policy-client.ts`. + +## What changes in `policy-client.ts` + +1. **A configurable timeout budget per RPC call**, not one blanket timeout + for the whole client — `simulate`, `deployInstance`, and + `recordDeployment` have different expected latencies (simulate is a dry + run; deploy/record involve on-chain interaction server-side), so each + gets its own default and its own override: + + ```ts + export interface PolicyDeployTimeoutBudgets { + simulate: number; + deployInstance: number; + recordDeployment: number; + } + + export const DEFAULT_POLICY_DEPLOY_TIMEOUTS: PolicyDeployTimeoutBudgets = { + simulate: 10_000, + deployInstance: 30_000, + recordDeployment: 15_000, + }; + ``` + + `PolicyClientOptions` would grow an optional `timeouts?: Partial` + field, merged over the defaults the same way `options.timeouts` is merged + in [`createTimedPolicyDeployClient`](policy-deploy-timeout.ts). + +2. **A distinct typed timeout error**, `PolicyDeployTimeoutError`, thrown via + `AbortController` + `setTimeout` inside the shared request helper. It is + deliberately **not** a `PolicyApiError` subclass: + + - `PolicyApiError` means the server responded and said no — per the + existing `retryable` logic in `src/policy-types.ts`, that's sometimes + retryable (5xx, 429, 408) and sometimes not (4xx in general). + - `PolicyDeployTimeoutError` means no response was ever received — we + never learned what the server decided. A caller can choose to retry + with a longer budget, but should not conflate this with "the server + rejected the deploy." + + Keeping them as separate types (rather than, say, `PolicyApiError` with + `status: 0`, which is already used for transport failures) lets a caller + `catch` and branch on `instanceof` without inspecting a status code, and + keeps "we gave up waiting" legible as its own failure mode when read out + of a stack trace or an error-tracking dashboard. + +## Applying this to the real client + +In `src/policy-client.ts`, the `req()` helper's `doFetch(...)` call would +gain an `AbortController` scoped to the timeout for that specific call site, +with `deployInstance`/`recordDeployment`/`simulate` each passing their own +budget from `PolicyClientOptions.timeouts` (falling back to +`DEFAULT_POLICY_DEPLOY_TIMEOUTS`), exactly as shown in +[`createTimedPolicyDeployClient`](policy-deploy-timeout.ts). The +`AbortError` case is caught and re-thrown as `PolicyDeployTimeoutError` +before it can surface as an opaque `AbortError` to the caller. + +## README documentation for the option + +The SDK's top-level README's policy-client usage section should document +the new option next to `apiUrl`/`network`/`fetch`: + +```ts +const policyClient = createPolicyClient({ + apiUrl: "https://api.example.com", + network: "testnet", + // Optional per-call timeout budgets (ms) for the deployment RPC calls. + // Falls back to DEFAULT_POLICY_DEPLOY_TIMEOUTS for any field not given. + timeouts: { deployInstance: 45_000 }, +}); +``` + +## Run it + +```sh +npx tsx policy-deploy-timeout.ts +``` + +Demonstrates a `simulate` call against a mock fetch that takes 5s to +resolve, configured with a 200ms budget — the call throws +`PolicyDeployTimeoutError` instead of hanging. + +## Tests + +```sh +npx vitest run contrib/examples/issue-276-rpc-timeout-budget +``` + +Covers: a call resolving normally inside its budget, a call timing out as +configured, budgets tracked independently per call (`simulate` timing out +doesn't affect `deployInstance`'s separate budget), unset budgets falling +back to the documented defaults, the timeout error carrying the path and +configured timeout, and a non-timeout error (e.g. DNS failure) propagating +unchanged rather than being misreported as a timeout. diff --git a/contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.test.ts b/contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.test.ts new file mode 100644 index 0000000..b84a205 --- /dev/null +++ b/contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from "vitest"; +import { + DEFAULT_POLICY_DEPLOY_TIMEOUTS, + PolicyDeployTimeoutError, + createTimedPolicyDeployClient, +} from "./policy-deploy-timeout"; + +/** A mock fetch that resolves after `delayMs` with a JSON body, respecting + * the abort signal the way the real global fetch does. */ +function createDelayedJsonFetch(delayMs: number, body: unknown): typeof fetch { + return ((_url: string, init?: RequestInit) => + new Promise((resolve, reject) => { + const timer = setTimeout(() => resolve(new Response(JSON.stringify(body))), delayMs); + init?.signal?.addEventListener("abort", () => { + clearTimeout(timer); + reject(new DOMException("The operation was aborted", "AbortError")); + }); + })) as typeof fetch; +} + +describe("createTimedPolicyDeployClient — timeout triggers as configured", () => { + it("simulate() resolves normally when the response arrives inside the budget", async () => { + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: createDelayedJsonFetch(10, { ok: true, minResourceFee: "100" }), + timeouts: { simulate: 200 }, + }); + const result = await client.simulate("policy-1", "GWALLET1"); + expect(result).toEqual({ ok: true, minResourceFee: "100" }); + }); + + it("simulate() throws PolicyDeployTimeoutError when the response exceeds its budget", async () => { + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: createDelayedJsonFetch(1000, { ok: true }), + timeouts: { simulate: 20 }, + }); + await expect(client.simulate("policy-1", "GWALLET1")).rejects.toThrow(PolicyDeployTimeoutError); + }); + + it("deployInstance() respects its own configured budget independent of simulate", async () => { + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: createDelayedJsonFetch(1000, { contractId: "C123" }), + timeouts: { simulate: 5000, deployInstance: 20 }, + }); + await expect(client.deployInstance("policy-1", "GWALLET1")).rejects.toThrow( + PolicyDeployTimeoutError, + ); + }); + + it("recordDeployment() succeeds within its budget", async () => { + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: createDelayedJsonFetch(10, { policy: { id: "p1" } }), + timeouts: { recordDeployment: 500 }, + }); + await expect(client.recordDeployment("policy-1", "tx-hash")).resolves.toEqual({ + policy: { id: "p1" }, + }); + }); + + it("falls back to DEFAULT_POLICY_DEPLOY_TIMEOUTS for any budget not overridden", async () => { + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: createDelayedJsonFetch(10, { ok: true }), + timeouts: { simulate: 50 }, + }); + // deployInstance/recordDeployment keep their defaults; a fast mock still + // resolves well inside DEFAULT_POLICY_DEPLOY_TIMEOUTS.deployInstance. + expect(DEFAULT_POLICY_DEPLOY_TIMEOUTS.deployInstance).toBeGreaterThan(10); + await expect(client.deployInstance("policy-1", "GWALLET1")).resolves.toBeDefined(); + }); + + it("the timeout error names the path and configured timeout", async () => { + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: createDelayedJsonFetch(1000, {}), + timeouts: { simulate: 15 }, + }); + await expect(client.simulate("policy-42", "GWALLET1")).rejects.toMatchObject({ + name: "PolicyDeployTimeoutError", + path: "/policy-42/simulate", + timeoutMs: 15, + }); + }); + + it("propagates a non-timeout error unchanged", async () => { + const failingFetch: typeof fetch = (async () => { + throw new Error("DNS failure"); + }) as typeof fetch; + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: failingFetch, + }); + await expect(client.simulate("policy-1", "GWALLET1")).rejects.toThrow("DNS failure"); + }); +}); diff --git a/contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.ts b/contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.ts new file mode 100644 index 0000000..0e3cd31 --- /dev/null +++ b/contrib/examples/issue-276-rpc-timeout-budget/policy-deploy-timeout.ts @@ -0,0 +1,153 @@ +// Example: add a configurable timeout budget to the policy-deployment RPC +// calls in src/policy-client.ts (`simulate`, `deployInstance`, +// `recordDeployment`), which today have no explicit timeout and can hang +// indefinitely on a stalled network request. +// +// This mirrors src/policy-client.ts's own `req()` helper shape (base URL, +// injected fetch, PolicyApiError on non-2xx) but adds an AbortController-based +// timeout with a distinct error type, so a caller can tell "the server said +// no" (PolicyApiError) apart from "we gave up waiting" (PolicyDeployTimeoutError) +// and react differently — the former is not safely retryable in general, the +// latter usually is. +// +// Run with: npx tsx policy-deploy-timeout.ts + +/** Thrown when a policy-deployment RPC call exceeds its timeout budget. + * Deliberately NOT a PolicyApiError subclass — a timeout means we never + * learned what the server decided, which is a different situation than a + * server response we didn't like. */ +export class PolicyDeployTimeoutError extends Error { + readonly path: string; + readonly timeoutMs: number; + + constructor(path: string, timeoutMs: number) { + super(`Policy deployment request to "${path}" did not complete within ${timeoutMs}ms`); + this.name = "PolicyDeployTimeoutError"; + this.path = path; + this.timeoutMs = timeoutMs; + } +} + +/** Per-call timeout budgets (ms) for each deployment-path RPC call. Deploy + * and record involve on-chain interaction on the server side and are given + * more room than the lighter simulate call. Every value is overridable. */ +export interface PolicyDeployTimeoutBudgets { + simulate: number; + deployInstance: number; + recordDeployment: number; +} + +export const DEFAULT_POLICY_DEPLOY_TIMEOUTS: PolicyDeployTimeoutBudgets = { + simulate: 10_000, + deployInstance: 30_000, + recordDeployment: 15_000, +}; + +export interface TimedPolicyDeployClientOptions { + apiUrl: string; + fetch?: typeof fetch; + timeouts?: Partial; +} + +/** Minimal stand-ins for the real GeneratedPolicy/SimulateResult shapes + * (src/policy-types.ts) — kept local so this example has no dependency on + * src/, per the contrib sandbox rules. */ +export interface SimulateResultLike { + ok: boolean; + minResourceFee?: string; + error?: string; +} + +/** + * A timeout-aware wrapper around the policy-deployment RPC calls + * (`simulate`, `deployInstance`, `recordDeployment`), demonstrating the + * budget-per-call pattern that should be applied to `createPolicyClient` in + * src/policy-client.ts. + */ +export function createTimedPolicyDeployClient(options: TimedPolicyDeployClientOptions) { + const base = options.apiUrl.replace(/\/+$/, ""); + const doFetch = options.fetch ?? fetch; + const budgets: PolicyDeployTimeoutBudgets = { + ...DEFAULT_POLICY_DEPLOY_TIMEOUTS, + ...options.timeouts, + }; + + async function reqWithTimeout(path: string, timeoutMs: number, init?: RequestInit): Promise { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + const res = await doFetch(`${base}/policies${path}`, { + headers: init?.body ? { "content-type": "application/json" } : undefined, + ...init, + signal: controller.signal, + }); + const payload = (await res.json().catch(() => ({}))) as { message?: string; error?: string } & T; + if (!res.ok) { + throw new Error(payload.message ?? payload.error ?? `Request failed (${res.status})`); + } + return payload; + } catch (err) { + if (err instanceof Error && err.name === "AbortError") { + throw new PolicyDeployTimeoutError(path, timeoutMs); + } + throw err; + } finally { + clearTimeout(timer); + } + } + + return { + simulate(policyId: string, wallet: string): Promise { + return reqWithTimeout(`/${policyId}/simulate`, budgets.simulate, { + method: "POST", + body: JSON.stringify({ wallet }), + }); + }, + async deployInstance(policyId: string, wallet: string): Promise<{ contractId: string }> { + const { contractId } = await reqWithTimeout<{ contractId: string }>( + `/${policyId}/deploy-instance`, + budgets.deployInstance, + { method: "POST", body: JSON.stringify({ wallet }) }, + ); + return { contractId }; + }, + recordDeployment(policyId: string, txHash: string, contractId?: string): Promise { + return reqWithTimeout(`/deploy`, budgets.recordDeployment, { + method: "POST", + body: JSON.stringify({ policyId, txHash, contractId }), + }); + }, + }; +} + +async function main() { + const slowFetch: typeof fetch = (async (_url: string, init?: RequestInit) => { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => resolve(new Response(JSON.stringify({ ok: true }))), 5000); + init?.signal?.addEventListener("abort", () => { + clearTimeout(timer); + reject(new DOMException("The operation was aborted", "AbortError")); + }); + }); + }) as typeof fetch; + + const client = createTimedPolicyDeployClient({ + apiUrl: "https://api.example.com", + fetch: slowFetch, + timeouts: { simulate: 200 }, + }); + + try { + await client.simulate("policy-1", "GWALLET..."); + } catch (err) { + if (err instanceof PolicyDeployTimeoutError) { + console.log(`Timed out as expected: ${err.message}`); + } else { + throw err; + } + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(); +} diff --git a/contrib/examples/issue-281-release-process-guide/RELEASE_PROCESS.md b/contrib/examples/issue-281-release-process-guide/RELEASE_PROCESS.md new file mode 100644 index 0000000..3756066 --- /dev/null +++ b/contrib/examples/issue-281-release-process-guide/RELEASE_PROCESS.md @@ -0,0 +1,121 @@ +# Release process stages + +Closes #281. + +Contributor changes are confined to `contrib/`, so this doc lives here as a +reference rather than editing `CONTRIBUTING.md` directly — a maintainer can +fold this content into `CONTRIBUTING.md` verbatim once reviewed. It +describes, stage by stage, how a change lands in `main` and ends up +published to npm, based on the workflows actually defined in +`.github/workflows/`. + +## Overview + +``` +PR merged to dev/drips + | + v + maintainer promotes to main (manual) + | + v + verify-merged.yml (automated — post-merge content check) + | + v + version bump + CHANGELOG.md (manual) + | + v + git tag vX.Y.Z + push (manual) + | + v + publish.yml (automated — build, verify, npm publish) +``` + +## Stage 1 — Versioning (manual) + +The package version lives in `package.json` (`"version": "0.6.1"` as of this +writing). A maintainer bumps it by hand as part of preparing a release — +following semver: patch for fixes, minor for additive features, major for +breaking changes. This repo does not use an automated version-bump bot; the +number in `package.json` is the source of truth and is checked against the +release tag at publish time (see Stage 3). + +## Stage 2 — Changelog (manual) + +`CHANGELOG.md` gets a new top-level entry (`## X.Y.Z — YYYY-MM-DD`) +summarizing what changed, written for someone deciding whether to upgrade — +see the existing `0.6.0` entry for the level of detail expected on a +breaking or security-relevant change (it explains the vulnerability, the +threat model, and what wasn't affected). This is a manual, human-written +step; there is no changelog-generation tooling in this repo today. + +## Stage 3 — Build and verify (automated) + +Two workflows do the automated verification work, at different points: + +- **[`.github/workflows/ci.yml`](../../../.github/workflows/ci.yml)** runs + on every push to `main`/`drips` and on every pull request: + `npm ci` → `npm run typecheck` → `npm test` → `npm run build`. This is the + gate every PR must pass before merge, and it's the same sequence + `CONTRIBUTING.md` already asks contributors to run locally before opening + a PR. + +- **[`.github/workflows/verify-merged.yml`](../../../.github/workflows/verify-merged.yml)** + runs after every push to `main` and re-verifies, *by content* rather than + by merge status, that each PR referenced in the new commits actually + landed. This exists because merge status alone was once misleading: two + PRs both reported "merged" on the same day, but one was stacked on a base + that had merged moments earlier and its content never actually reached + `main`. The workflow greps merged commit messages for `#`, then + runs `scripts/verify-merged.mjs` against each one, including a + self-test step that asserts the known-bad case still fails — so a + regression in the checker itself doesn't silently start passing everything. + +Both are fully automated; no manual step is required to trigger them. + +## Stage 4 — Publish (automated, tag-triggered) + +**[`.github/workflows/publish.yml`](../../../.github/workflows/publish.yml)** +runs only when a `v*` tag is pushed — never on an ordinary commit to `main`. +A maintainer creates the release by tagging manually +(`git tag v0.6.2 && git push origin v0.6.2`); everything downstream of that +tag push is automated: + +1. Checkout, install, `npm audit --audit-level=high`, typecheck, test, build + — the same gates as CI, run again independently rather than trusted from + the earlier PR run. +2. **Tag/version match check** — the workflow reads `package.json`'s + `version` and compares it against the pushed tag (`v0.6.2` must match + `0.6.2`). A mismatch fails the workflow before anything is published, + so a forgotten version bump can't ship under the wrong tag. +3. `npm publish --provenance --access public` — publishes with npm + provenance, which cryptographically attests the published tarball back + to this exact workflow run and commit. This matters here specifically: + a compromised publish would run inside every consuming app's browser and + inside any MCP server holding a signing key, so every release needs to + be traceable to a specific, auditable build. + +The `id-token: write` permission on this job is what allows minting the +OIDC token npm exchanges for that provenance attestation — it's scoped to +the publish job only. + +## What's automated vs. manual, at a glance + +| Stage | Trigger | Automated? | +| --- | --- | --- | +| Version bump in `package.json` | maintainer decision | Manual | +| `CHANGELOG.md` entry | maintainer writes it | Manual | +| Typecheck / test / build on every PR | push / PR event | Automated (`ci.yml`) | +| Post-merge content verification | push to `main` | Automated (`verify-merged.yml`) | +| Git tag creation | maintainer decision | Manual | +| Build, audit, tag/version match check, npm publish | tag push (`v*`) | Automated (`publish.yml`) | + +## Notes for a first-time contributor + +- You will never run the publish stage yourself — it's gated to tag pushes, + and only a maintainer with `NPM_TOKEN` access can complete it. +- The check you *can* run locally before opening a PR is the same one CI + runs: `npm install && npm run typecheck && npm test && npm run build` + (already documented in `CONTRIBUTING.md`). +- If your PR doesn't touch `package.json`'s version or `CHANGELOG.md`, + that's normal — those are release-time steps a maintainer handles when + cutting an actual version, not something every PR needs to include. diff --git a/contrib/examples/issue-293-policy-schema-migration-guide/README.md b/contrib/examples/issue-293-policy-schema-migration-guide/README.md new file mode 100644 index 0000000..f144308 --- /dev/null +++ b/contrib/examples/issue-293-policy-schema-migration-guide/README.md @@ -0,0 +1,99 @@ +# Backfill guide: migrating cached policy data across schema versions + +Closes #293. + +If your app caches a `GeneratedPolicy` / `PolicyDefinition` locally (e.g. in +`localStorage`, a file, or a mobile app's on-device store) between SDK +upgrades, an older cached blob can predate a schema change. This guide shows +how to detect that and migrate the cached shape forward, following the same +pattern the SDK already uses for `WalletSession` storage in +[`src/session.ts`](../../../src/session.ts): a `load()` that never throws on +unreadable/outdated data, paired with an explicit shape check +(`isWalletSession`) before trusting what came back from storage. + +## Why this is needed + +Nothing in the policy domain types (`src/policy-types.ts`, `src/types.ts`) +carries a version tag today — a policy blob written to storage by an older +app version and a current one are structurally indistinguishable unless the +consumer adds versioning itself. Without a documented convention, every +consumer that caches policies has to invent their own detection and +migration logic (or, worse, skips it and ships a runtime crash the first +time a returning user's cached policy doesn't match the shape the current +SDK expects). + +## The pattern + +1. **Stamp a `schemaVersion` field on every policy you write to storage**, + starting now. See [`stampCurrentVersion`](policy-schema-migration.ts). +2. **On load, detect the version before trusting the shape.** Missing + `schemaVersion` means "written before this convention existed" — treat it + as the oldest known shape, not an error. See + [`detectSchemaVersion`](policy-schema-migration.ts). +3. **Migrate forward through each version in sequence**, the same way you'd + write a database migration — never jump straight from "unknown old shape" + to "current shape" with one big conditional. See + [`migratePolicyToCurrent`](policy-schema-migration.ts), which is + idempotent: running it on already-current data is a no-op, so it's safe + to call unconditionally on every load. +4. **Never let a corrupt or unrecognized cache crash the app.** Wrap the + load + migrate step the same way `SessionStorageAdapter.restore()` does in + `src/session.ts` — catch, log, and fall back to treating it as "no cached + policy", not an unhandled exception. + +## Detecting an outdated cached schema + +```ts +import { detectSchemaVersion, CURRENT_POLICY_SCHEMA_VERSION } from "./policy-schema-migration"; + +const cached: unknown = JSON.parse(localStorage.getItem("myapp.cachedPolicy") ?? "null"); + +if (cached !== null) { + const version = detectSchemaVersion(cached); + if (version < CURRENT_POLICY_SCHEMA_VERSION) { + console.warn(`Cached policy is schema v${version}, current is v${CURRENT_POLICY_SCHEMA_VERSION} — migrating`); + } +} +``` + +## Migrating a fixture (v1 -> v2) + +The old shape (`PolicyV1`) had a single string owner and flat limit fields. +The current shape (`PolicyV2`) supports multiple owners and nests limits +under `spendingLimits`, mirroring `src/types.ts`'s `PolicyDefinition`: + +```ts +import { migratePolicyToCurrent } from "./policy-schema-migration"; + +const legacyCached = { + policyOwner: "GOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + dailyLimit: "100", + perTxLimit: "20", + allowlistedContracts: ["CUSDCAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"], +}; + +const migrated = migratePolicyToCurrent(legacyCached); +// { +// schemaVersion: 2, +// owners: ["GOWNERAAAA..."], +// spendingLimits: { dailyXlm: "100", perTxXlm: "20" }, +// allowlistedContracts: ["CUSDCAAAA..."], +// } +``` + +## Run it + +```sh +npx tsx policy-schema-migration.ts +``` + +## Tests + +```sh +npx vitest run contrib/examples/issue-293-policy-schema-migration-guide +``` + +The test suite runs the documented migration steps against the fixture +above end to end: version detection on an unversioned blob, field-by-field +migration correctness, idempotency on already-current data, and rejection +of an unrecognized future schema version. diff --git a/contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.test.ts b/contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.test.ts new file mode 100644 index 0000000..2d295e5 --- /dev/null +++ b/contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from "vitest"; +import { + CURRENT_POLICY_SCHEMA_VERSION, + detectSchemaVersion, + migratePolicyToCurrent, + stampCurrentVersion, + type PolicyV1, +} from "./policy-schema-migration"; + +// Fixture: a policy blob as it would have been written to local storage by +// an older consumer, before schemaVersion existed at all. +const legacyFixture: PolicyV1 = { + policyOwner: "GOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + dailyLimit: "100", + perTxLimit: "20", + allowlistedContracts: ["CUSDCAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"], +}; + +describe("detectSchemaVersion", () => { + it("treats a blob with no schemaVersion field as v1", () => { + expect(detectSchemaVersion(legacyFixture)).toBe(1); + }); + + it("treats an explicit schemaVersion: 1 as v1", () => { + expect(detectSchemaVersion({ ...legacyFixture, schemaVersion: 1 })).toBe(1); + }); + + it("detects a current v2 blob", () => { + expect( + detectSchemaVersion({ + schemaVersion: 2, + owners: [legacyFixture.policyOwner], + spendingLimits: { dailyXlm: "100", perTxXlm: "20" }, + allowlistedContracts: legacyFixture.allowlistedContracts, + }), + ).toBe(2); + }); + + it("rejects an unrecognized schemaVersion", () => { + expect(() => detectSchemaVersion({ schemaVersion: 99 })).toThrow(RangeError); + }); + + it("rejects a non-object value", () => { + expect(() => detectSchemaVersion(null)).toThrow(TypeError); + expect(() => detectSchemaVersion("not an object")).toThrow(TypeError); + }); +}); + +describe("migratePolicyToCurrent — documented migration steps against the fixture", () => { + it("wraps the single owner string into the owners array", () => { + const migrated = migratePolicyToCurrent(legacyFixture); + expect(migrated.owners).toEqual([legacyFixture.policyOwner]); + }); + + it("nests dailyLimit/perTxLimit under spendingLimits", () => { + const migrated = migratePolicyToCurrent(legacyFixture); + expect(migrated.spendingLimits).toEqual({ dailyXlm: "100", perTxXlm: "20" }); + }); + + it("carries allowlistedContracts through unchanged", () => { + const migrated = migratePolicyToCurrent(legacyFixture); + expect(migrated.allowlistedContracts).toEqual(legacyFixture.allowlistedContracts); + }); + + it("stamps schemaVersion 2 on the migrated result", () => { + const migrated = migratePolicyToCurrent(legacyFixture); + expect(migrated.schemaVersion).toBe(CURRENT_POLICY_SCHEMA_VERSION); + }); + + it("is idempotent — migrating an already-current blob returns it unchanged", () => { + const once = migratePolicyToCurrent(legacyFixture); + const twice = migratePolicyToCurrent(once); + expect(twice).toEqual(once); + }); +}); + +describe("stampCurrentVersion", () => { + it("attaches the current schema version to a freshly built policy", () => { + const stamped = stampCurrentVersion({ + owners: [legacyFixture.policyOwner], + spendingLimits: { dailyXlm: "100", perTxXlm: "20" }, + allowlistedContracts: legacyFixture.allowlistedContracts, + }); + expect(stamped.schemaVersion).toBe(CURRENT_POLICY_SCHEMA_VERSION); + expect(detectSchemaVersion(stamped)).toBe(2); + }); +}); diff --git a/contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.ts b/contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.ts new file mode 100644 index 0000000..d92656b --- /dev/null +++ b/contrib/examples/issue-293-policy-schema-migration-guide/policy-schema-migration.ts @@ -0,0 +1,95 @@ +// Example: detect and migrate a locally cached PolicyDefinition that was +// written to storage by an older version of a consuming app, following the +// same versioned-migration shape used for session storage (see +// src/session.ts's SessionStorageAdapter: load() returns null/throws are +// treated as "nothing usable", never a crash). +// +// Cached policy blobs have no version tag today, so a consumer opting into +// this pattern should start stamping a `schemaVersion` field going forward +// (see `stampCurrentVersion` below) and use `detectSchemaVersion` to handle +// whatever was written before that started. +// +// Run with: npx tsx policy-schema-migration.ts + +export const CURRENT_POLICY_SCHEMA_VERSION = 2; + +/** v1 shape: a single string owner, flat limit fields. Predates + * `owners: string[]` and the nested `spendingLimits` object. */ +export interface PolicyV1 { + schemaVersion?: 1; // absent on the very first cached shape, pre-dating the field itself + policyOwner: string; + dailyLimit: string; + perTxLimit: string; + allowlistedContracts: string[]; +} + +/** v2 (current) shape, mirroring src/types.ts PolicyDefinition's structure. */ +export interface PolicyV2 { + schemaVersion: 2; + owners: string[]; + spendingLimits: { dailyXlm: string; perTxXlm: string }; + allowlistedContracts: string[]; +} + +export type StoredPolicy = PolicyV1 | PolicyV2; + +/** + * Detects the schema version of a cached policy blob. Unversioned blobs + * (no `schemaVersion` field at all) are assumed v1, since that was the only + * shape ever written before versioning was introduced. + */ +export function detectSchemaVersion(cached: unknown): 1 | 2 { + if (typeof cached !== "object" || cached === null) { + throw new TypeError("Cached policy is not an object — cannot detect schema version"); + } + const version = (cached as { schemaVersion?: unknown }).schemaVersion; + if (version === 2) return 2; + if (version === 1 || version === undefined) return 1; + throw new RangeError(`Unrecognized policy schemaVersion: ${String(version)}`); +} + +function migratePolicyV1ToV2(v1: PolicyV1): PolicyV2 { + return { + schemaVersion: 2, + owners: [v1.policyOwner], + spendingLimits: { dailyXlm: v1.dailyLimit, perTxXlm: v1.perTxLimit }, + allowlistedContracts: v1.allowlistedContracts, + }; +} + +/** + * Migrates a cached policy blob of unknown vintage up to the current schema. + * Idempotent — calling it on an already-current blob returns it unchanged. + */ +export function migratePolicyToCurrent(cached: unknown): PolicyV2 { + const version = detectSchemaVersion(cached); + if (version === 2) return cached as PolicyV2; + return migratePolicyV1ToV2(cached as PolicyV1); +} + +/** Stamps the current schema version onto a policy before writing it back to + * storage, so future reads can detect its version directly. */ +export function stampCurrentVersion(policy: Omit): PolicyV2 { + return { ...policy, schemaVersion: CURRENT_POLICY_SCHEMA_VERSION }; +} + +function main() { + const legacyCached: unknown = { + policyOwner: "GOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + dailyLimit: "100", + perTxLimit: "20", + allowlistedContracts: ["CUSDCAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"], + }; + + console.log("Detected version:", detectSchemaVersion(legacyCached)); + const migrated = migratePolicyToCurrent(legacyCached); + console.log("Migrated:", migrated); + console.log( + "Re-running migration on already-current data is a no-op:", + migratePolicyToCurrent(migrated), + ); +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(); +} diff --git a/contrib/examples/issue-300-deprecation-warning-pattern/README.md b/contrib/examples/issue-300-deprecation-warning-pattern/README.md new file mode 100644 index 0000000..5460f6b --- /dev/null +++ b/contrib/examples/issue-300-deprecation-warning-pattern/README.md @@ -0,0 +1,79 @@ +# Deprecation pattern: legacy methods superseded by a newer client + +Closes #300. + +`src/payments.ts` currently exports pure types and amount-parsing helpers +(`parseTokenAmount`, `PaymentReview`) that `src/payments-client.ts`'s +`createPaymentClient` builds on — they aren't duplicated or superseded by +each other today. Rather than invent a deprecation for methods that aren't +actually redundant, this contrib entry documents the **reusable pattern** a +maintainer can apply directly to `payments.ts` (or any other module) the +moment a method there does get superseded by `payments-client.ts` or a +future client, without needing to design the mechanism from scratch each +time. + +## The pattern + +1. **JSDoc `@deprecated` tag** on the superseded method, naming its + replacement by exact identifier so an IDE can link it, e.g.: + + ```ts + /** + * @deprecated Superseded by `PaymentClient.preparePayment()`. Will be + * removed in the next major version — see CHANGELOG.md. + */ + export function oldMethod() { ... } + ``` + +2. **A one-time runtime warning**, via [`warnOnce`](deprecation-warning.ts), + so callers who never see the JSDoc (compiled JS, no IDE hints) still get + told. It fires once per process — not once per call — so a method called + in a hot loop doesn't flood the console. See `legacySend` in + [`deprecation-warning.ts`](deprecation-warning.ts) for the full + call-site pattern. + +3. **A CHANGELOG.md entry stating the removal timeline**, e.g. under an + `## Unreleased` or the next version heading: + + ```md + ### Deprecated + - `oldMethod()` in `payments.ts` is deprecated in favor of + `PaymentClient.preparePayment()` in `payments-client.ts`. It will + continue to work through the current major version and be removed in + the next major release. Migrate by replacing `oldMethod(a, b)` with + `client.preparePayment({ ... })`. + ``` + + Because contributor changes are confined to `contrib/`, the actual + `CHANGELOG.md` edit is left to a maintainer applying this pattern — + this README shows the exact entry shape to add. + +## Why a one-time warning, not every call + +A method invoked once at startup and one invoked per-transaction both need +the same nudge, but logging on every call of a hot method would drown a +consuming app's own logs and make the warning look like a bug in itself. +[`warnOnce`](deprecation-warning.ts) tracks fired warnings by an id unique +to each deprecated method, in a module-level `Set`, so the cost of checking +"have I warned about this yet" is O(1) and the warning still reaches every +caller at least once per process. + +## Run it + +```sh +npx tsx deprecation-warning.ts +``` + +Expected output — the warning fires on the first call and is suppressed on +the second, even though `legacySend` still executes and returns a result +both times. + +## Tests + +```sh +npx vitest run contrib/examples/issue-300-deprecation-warning-pattern +``` + +Verifies: the warning fires exactly once per id, separate ids are tracked +independently, and the deprecated method's return value is unaffected by +the warning path. diff --git a/contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.test.ts b/contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.test.ts new file mode 100644 index 0000000..2adb7cb --- /dev/null +++ b/contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.test.ts @@ -0,0 +1,61 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { _resetWarnOnceForTests, legacySend, warnOnce } from "./deprecation-warning"; + +describe("warnOnce", () => { + beforeEach(() => { + _resetWarnOnceForTests(); + vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("logs the message the first time a given id fires", () => { + warnOnce("example-id", "this is a deprecation notice"); + expect(console.warn).toHaveBeenCalledTimes(1); + expect(console.warn).toHaveBeenCalledWith("this is a deprecation notice"); + }); + + it("does not log again for the same id on a second call", () => { + warnOnce("example-id", "first message"); + warnOnce("example-id", "first message"); + warnOnce("example-id", "first message"); + expect(console.warn).toHaveBeenCalledTimes(1); + }); + + it("tracks separate ids independently", () => { + warnOnce("id-a", "message a"); + warnOnce("id-b", "message b"); + expect(console.warn).toHaveBeenCalledTimes(2); + }); +}); + +describe("legacySend deprecation warning", () => { + beforeEach(() => { + _resetWarnOnceForTests(); + vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("fires a deprecation warning on first call", () => { + legacySend("GDEST111", 100n); + expect(console.warn).toHaveBeenCalledTimes(1); + expect(console.warn).toHaveBeenCalledWith(expect.stringContaining("legacySend() is deprecated")); + }); + + it("does not repeat the warning on subsequent calls in the same process", () => { + legacySend("GDEST111", 100n); + legacySend("GDEST222", 200n); + legacySend("GDEST333", 300n); + expect(console.warn).toHaveBeenCalledTimes(1); + }); + + it("still returns the correct result despite the warning", () => { + const result = legacySend("GDEST111", 100n); + expect(result).toEqual({ to: "GDEST111", amount: 100n }); + }); +}); diff --git a/contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.ts b/contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.ts new file mode 100644 index 0000000..be0f5e9 --- /dev/null +++ b/contrib/examples/issue-300-deprecation-warning-pattern/deprecation-warning.ts @@ -0,0 +1,66 @@ +// Example: a reusable pattern for deprecating an SDK method that has been +// superseded by a newer client, without breaking existing callers. +// +// Two parts: +// 1. A `@deprecated` JSDoc tag so IDEs strike the call through and point +// callers at the replacement. +// 2. A one-time runtime console warning, so callers who don't read JSDoc +// (or who called it before the tag was added) still find out — logged +// once per process, not once per call, so a hot path doesn't spam. +// +// Run with: npx tsx deprecation-warning.ts + +/** Tracks which deprecation warnings have already fired this process, keyed + * by an id unique to each deprecated method. */ +const warnedOnce = new Set(); + +/** + * Logs `message` to console.warn the first time it's called for a given + * `id`; every subsequent call for that `id` is a silent no-op. Exported + * separately from any specific deprecated method so it can be unit-tested + * (and reset between tests) independent of what's calling it. + */ +export function warnOnce(id: string, message: string): void { + if (warnedOnce.has(id)) return; + warnedOnce.add(id); + console.warn(message); +} + +/** Test-only escape hatch: clears the fired-once state so each test can + * assert on a fresh warning. Not needed in application code. */ +export function _resetWarnOnceForTests(): void { + warnedOnce.clear(); +} + +// --- Example: applying the pattern to a superseded method ----------------- +// +// Imagine `legacySend` below lives in a "v1 client" module and has been +// superseded by a `send` method on a newer client. It still works — this +// is a deprecation notice, not a removal — but every call after the first +// nudges the caller toward the replacement. + +/** + * @deprecated Superseded by `NewClient.send()`. `legacySend` will be removed + * in the next major version — see CHANGELOG.md for the timeline. Migrate by + * replacing `legacySend(to, amount)` with `new NewClient(options).send({ to, amount })`. + */ +export function legacySend(to: string, amount: bigint): { to: string; amount: bigint } { + warnOnce( + "legacySend", + "legacySend() is deprecated and will be removed in the next major version. " + + "Use NewClient.send() instead. See CHANGELOG.md for the removal timeline.", + ); + return { to, amount }; +} + +function main() { + console.log("First call — warning fires:"); + legacySend("GDEST...", 100n); + + console.log("\nSecond call — same process, warning suppressed:"); + legacySend("GDEST...", 200n); +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(); +}