solid

SOLID Skill

Overview

SOLID is Robert C. Martin's five object-oriented design principles. In a TypeScript codebase like Flight School — which mixes a Next.js API surface, a Copilot SDK adapter layer, GitHub data fetching, and a streaming UI — SOLID is mostly a tool for keeping modules swappable, testable, and free of hidden coupling. We don't have deep class hierarchies, but we do have:

• multiple AI providers behind one interface,
• multiple persistence backends (in-memory dev, Cosmos in production),
• many API routes that share auth + rate-limit + audit concerns, and
• streams generated by very different sources (Copilot SDK, mocks, tests).

SOLID gives us shared vocabulary for when to split a module, when to add an interface, and when not to.

Authoritative reference

This skill is reference material. The source of truth for code style, naming, exports, and module organization is .github/instructions/typescript.instructions.md.

If anything here appears to conflict with that file, the instructions win. In particular:

• Single Responsibility Functions, module organization, and DRY rules are already covered by typescript.instructions.md — this skill links to them rather than restating them.
• Documentation conventions (TSDoc) are covered by documentation.instructions.md.

---

S — Single Responsibility Principle

A module should have one, and only one, reason to change.

In practice: split modules along axes of change. If two pieces of logic change for different reasons (different team, different deploy cadence, different test setup), they don't belong in the same file.

Spot it

• A file imports both fs/fetch and business rules.
• One module exports a "god object" used by everything (utils.ts, helpers.ts, ambient singletons).
• A test for one function must mock five unrelated subsystems.
• A change to UI copy forces a change in the data layer.

Example in this codebase

The GitHub client used to be a process-wide singleton Octokit that also owned ambient token resolution — reading GITHUB_TOKEN from the environment or shelling out to gh auth token. That single module owned three responsibilities at once: ambient token discovery, Octokit construction, and request instrumentation — and shared one cached instance across users. The "reason to change" was different for each (multitenancy, telemetry, auth), but they were entangled.

The current src/lib/github/client.ts has no ambient auth surface at all. Token resolution lives in the auth layer (src/lib/auth/context.ts); this module only knows how to use a token it was handed:

// src/lib/github/client.ts
export function getOctokitForToken(token: string): Octokit {
  const instance = new Octokit({ auth: token });
  instrumentOctokitRequests(instance);
  return instance;
}

export async function getOctokitForRequest(): Promise<Octokit> {
  const { accessToken } = await requireUserContext();
  return getOctokitForToken(accessToken);
}

Each function has one job: build an Octokit for a given token, or bind to the current request's authenticated user. The file-level docstring is explicit about what was removed and why:

There is intentionally no ambient token resolution in this module — no GITHUB_TOKEN env fallback, no gh auth token CLI lookup, no process-wide cache. Every caller must supply a token that came from the authenticated user context.

The previous module collapsed three axes of change (instance lifecycle, ambient auth discovery, per-user authorisation) into one file. Splitting them — and deleting the ambient path entirely — is SRP in service of a security boundary: a leak test (src/lib/github/client.test.ts's 'does not export legacy token resolvers') now pins that the helpers can never come back.

See also: Single Responsibility Functions rule.

---

O — Open/Closed Principle

Modules should be open for extension, but closed for modification.

In TypeScript this rarely means inheritance. It usually means: design the shape of a call site so new behaviour can be added by composing new pieces, not by editing the existing function.

Spot it

• Adding a new feature requires editing a switch / if-else ladder in a shared function.
• Every new API route copies the same setup boilerplate (auth, rate limit, audit) and then drifts.
• The only way to add behaviour is a new boolean parameter.

Example in this codebase

withUserGuards in src/lib/security/guard.ts is built so any new expensive API route gets the same auth + rate-limit + concurrent-cap + audit treatment without modifying the wrapper itself. Cross-cutting concerns are configured, not re-implemented:

// src/lib/security/guard.ts
export async function withUserGuards<T>(
  opts: GuardOptions,
  work: (ctx: UserContext) => Promise<T>,
): Promise<T> {
  const ctx = await requireUserContext();
  // ...rate limit (skipped if opts.rateLimit is omitted)...
  // ...concurrent cap (skipped if opts.concurrentCap is omitted)...
  auditLog({ type: opts.eventType, /* ... */ });
  try { return await work(ctx); } finally { release?.(); }
}

A new route adds the wrapper and supplies its own eventType / limits. The guard module stays closed; the route extends it via composition:

export async function POST(request: NextRequest) {
  return withUserGuards(
    { rateLimit: { limit: 10, windowMs: 60_000 }, concurrentCap: 2,
      eventType: 'copilot.session.create' },
    async ({ userId }) => { /* the actual work */ },
  );
}

When we need a new guard (say, billing checks), we extend GuardOptions or compose another wrapper — without touching every route.

---

L — Liskov Substitution Principle

Subtypes must be usable in place of their supertypes without surprising callers.

In TypeScript this is mostly about interface contracts: every implementation of an interface must honour the same observable behaviour. Throwing where the contract said you return is an LSP violation, even if the types compile.

Spot it

• Implementations of an interface differ in which methods actually work.
• A "real" implementation throws on calls that the in-memory one silently returns null for (or vice versa).
• Callers add instanceof checks to recover.

Example in this codebase

TokenStore in src/lib/auth/token-store.ts has two implementations:

// src/lib/auth/token-store.ts
export interface TokenStore {
  getToken(userId: string): Promise<StoredToken | null>;
  setToken(userId: string, token: StoredToken): Promise<void>;
  deleteToken(userId: string): Promise<void>;
  cleanupExpired(): Promise<number>;
}

export class InMemoryTokenStore implements TokenStore { /* process-local Map */ }

export class CosmosTokenStore implements TokenStore {
  // Cosmos DB-backed with envelope encryption:
  //  - per-record AES-256-GCM data encryption key (DEK)
  //  - DEK wrapped with an Azure Key Vault KEK via `wrapKey(A256KW)`
  //  - documents partitioned by userId so a buggy or hostile caller
  //    cannot decrypt another user's row.
}

The intent — captured in the file docstring — is explicitly Liskov:

The {@link TokenStore} contract is the Liskov boundary: callers MUST be able to swap implementations without behavioural change.

Both implementations honour the same observable contract: unknown users return null, expired records return null (callers treat "not found" and "expired" identically), setToken upserts, deleteToken is idempotent. The Cosmos store adds confidentiality at rest (envelope encryption, AAD bound to userId/expiresAt/kekId so tampered documents fail decryption), but it does not change the observable behaviour — that is the whole point. A caller wired through getTokenStore() cannot tell which one it has, and tests can swap the in-memory store in without rewriting code.

Where LSP could go wrong here: if CosmosTokenStore started throwing on "user not found" instead of returning null, or made deleteToken non-idempotent, every caller of getTokenStore() would need to learn the difference. Keeping the contract identical is what makes the substitution invisible.

---

I — Interface Segregation Principle

Don't force consumers to depend on methods they don't use.

In TypeScript: prefer several small, focused interfaces over one fat one. A function that only needs an ID and a token shouldn't have to accept the entire SessionOptions bag.

Spot it

• Callers pass undefined for half the fields of an options object.
• Tests build huge fixture objects to call a small function.
• Adding a field to a "context" type ripples through unrelated call sites.

Example in this codebase

src/lib/copilot/server.ts defines a narrow SessionIdentity interface that is the only per-request input most session factories need:

// src/lib/copilot/server.ts
export interface SessionIdentity {
  userId: string;
  gitHubToken: string;
}

export async function createLoggedCoachSession(
  identity: SessionIdentity,
  operationName = 'Coach Session',
  inputPrompt = '',
) { /* ... */ }

Internally the Copilot SDK takes a wider SessionOptions (from src/lib/copilot/types.ts):

// src/lib/copilot/types.ts
export interface SessionOptions {
  // Multi-tenant invariant — BOTH required:
  userId: string;       // partitions the session cache
  gitHubToken: string;  // ghu_ token forwarded to the SDK / MCP
  // Optional configuration:
  systemMessage?: string;
  includeMcpTools?: boolean;
  tools?: string[];
  model?: string;
}

But callers in API routes — src/app/api/focus/route.ts, src/app/api/jobs/job-executors.ts — only ever see SessionIdentity (the narrow { userId, gitHubToken } pair, which happens to coincide w

---

Content truncated.