Refactor Linear API token: pass through stack instead of setting env var#591
Refactor Linear API token: pass through stack instead of setting env var#591
Conversation
Complexity AssessmentClassification: COMPLEX Metrics:
Reasoning: This is a COMPLEX issue due to cross-cutting change requirements. The API token must flow through a deep call stack: LinearService → linear.ts functions (14+ functions) → LinearClient, affecting ~20 production call sites across 6 files (~3 in LinearService.ts, ~15 in LinearIssueManagementProvider.ts, ~1 in list-children.ts). Additionally, linear.ts is a large utility file (828 LOC) that will require coordinated updates to multiple function signatures. Note: issues.ts and list-children.ts already pass |
Analysis Phase
Estimated time: ~8 minutes |
Analysis Phase (Deep Investigation)
Executive SummaryThis refactor eliminates the fragile
Impact Summary:
Complete Technical Reference (click to expand for implementation details)Problem Space ResearchProblem Understanding
Architectural ContextThe codebase has two distinct execution contexts for Linear operations:
The refactor only affects context #1. Context #2 has its own env var injection mechanism that works correctly. Edge Cases Identified
Codebase Research FindingsAffected Area:
|
Implementation Plan for Issue #590SummaryRefactor Linear API token passing to eliminate the fragile Questions and Key Decisions
High-Level Execution Phases
Quick Stats
Complete Implementation Guide (click to expand for step-by-step details)Automated Test Cases to CreateTest File:
|
| Function | Line (signature) | Line (createLinearClient()) |
Notes |
|---|---|---|---|
fetchLinearIssue |
128 | 131 | Add apiToken?: string after identifier: string |
createLinearIssue |
178-183 | 186 | Add apiToken?: string after _labels?: string[] |
createLinearChildIssue |
240-245 | 249 | Add apiToken?: string after _labels?: string[] |
createLinearComment |
300-302 | 306 | Add apiToken?: string after body: string |
updateLinearIssueState |
347-349 | 353 | Add apiToken?: string after stateName: string |
getLinearComment |
396 | 399 | Add apiToken?: string after commentId: string |
updateLinearComment |
428-430 | 434 | Add apiToken?: string after body: string |
fetchLinearIssueComments |
464 | 467 | Add apiToken?: string after identifier: string |
createLinearIssueRelation |
560-562 | 566 | Add apiToken?: string after blockedIssueId: string |
getLinearIssueDependencies |
594-596 | 600 | Add apiToken?: string after direction param |
deleteLinearIssueRelation |
684 | 687 | Add apiToken?: string after relationId: string |
findLinearIssueRelation |
785-787 | 791 | Add apiToken?: string after blockedIdentifier: string |
Pattern for each function:
// Before:
export async function fetchLinearIssue(identifier: string): Promise<LinearIssue> {
// ...
const client = createLinearClient()
// After:
export async function fetchLinearIssue(identifier: string, apiToken?: string): Promise<LinearIssue> {
// ...
const client = createLinearClient(apiToken)No changes needed for: getLinearChildIssues (line 501, already has options?.apiToken), fetchLinearIssueList (line 721, already has options?.apiToken).
Also update JSDoc @param for each function to document the new apiToken parameter.
2. src/lib/LinearService.ts
Change 1 -- Line 26-27: Update JSDoc for apiToken field.
// Before:
/** Linear API token (lin_api_...). If provided, sets process.env.LINEAR_API_TOKEN */
// After:
/** Linear API token (lin_api_...). If provided, passed to Linear API calls */Change 2 -- Line 38: Add private instance property.
// Add after line 39 (private prompter):
private apiToken?: stringChange 3 -- Lines 45-51: Replace env var side effect with property storage.
// Before:
this.config = config ?? {}
this.prompter = options?.prompter ?? promptConfirmation
// Set API token from config if provided (follows mcp.ts pattern)
if (this.config.apiToken) {
process.env.LINEAR_API_TOKEN = this.config.apiToken
}
// After:
this.config = config ?? {}
this.prompter = options?.prompter ?? promptConfirmation
this.apiToken = this.config.apiTokenChange 4 -- Line 101: Pass apiToken to fetchLinearIssue.
// Before:
const linearIssue = await fetchLinearIssue(String(identifier))
// After:
const linearIssue = await fetchLinearIssue(String(identifier), this.apiToken)Change 5 -- Line 166: Pass apiToken to createLinearIssue.
// Before:
const result = await createLinearIssue(title, body, this.config.teamId, labels)
// After:
const result = await createLinearIssue(title, body, this.config.teamId, labels, this.apiToken)Change 6 -- Line 192: Pass apiToken to updateLinearIssueState.
// Before:
await updateLinearIssueState(String(identifier), 'In Progress')
// After:
await updateLinearIssueState(String(identifier), 'In Progress', this.apiToken)3. src/lib/LinearService.test.ts
Full rewrite. Replace all 5 env var assertion tests (lines 11-67) with behavior-focused tests that verify apiToken is passed to mocked linear.ts functions.
Key changes:
- Remove:
beforeEach/afterEachenv var save/restore (lines 13-29) - Remove: All 5 tests asserting
process.env.LINEAR_API_TOKENvalue (lines 31-66) - Add: Test that constructor does NOT set
process.env.LINEAR_API_TOKEN - Add: Tests verifying
fetchLinearIssue,createLinearIssue,updateLinearIssueStateare called with apiToken argument - Keep: Existing
vi.mocksetup (lines 4-9)
Click to expand complete test structure (55 lines)
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { LinearService } from './LinearService.js'
import { fetchLinearIssue, createLinearIssue, updateLinearIssueState } from '../utils/linear.js'
vi.mock('../utils/linear.js', () => ({
fetchLinearIssue: vi.fn(),
createLinearIssue: vi.fn(),
updateLinearIssueState: vi.fn(),
}))
describe('LinearService', () => {
const testToken = 'lin_api_test_token_123'
describe('constructor', () => {
it('should not set process.env.LINEAR_API_TOKEN', () => {
// Ensure env var is clean
delete process.env.LINEAR_API_TOKEN
new LinearService({ apiToken: testToken })
expect(process.env.LINEAR_API_TOKEN).toBeUndefined()
})
})
describe('fetchIssue', () => {
it('should pass apiToken to fetchLinearIssue', async () => {
// mock fetchLinearIssue to return a valid LinearIssue
vi.mocked(fetchLinearIssue).mockResolvedValue({
id: 'uuid', identifier: 'ENG-123', title: 'Test',
url: 'https://linear.app/issue/ENG-123', createdAt: '...', updatedAt: '...',
})
const service = new LinearService({ apiToken: testToken })
await service.fetchIssue('ENG-123')
expect(fetchLinearIssue).toHaveBeenCalledWith('ENG-123', testToken)
})
})
describe('createIssue', () => {
it('should pass apiToken to createLinearIssue', async () => {
vi.mocked(createLinearIssue).mockResolvedValue({
identifier: 'ENG-456', url: 'https://linear.app/issue/ENG-456',
})
const service = new LinearService({ apiToken: testToken, teamId: 'ENG' })
await service.createIssue('Title', 'Body', undefined, ['bug'])
expect(createLinearIssue).toHaveBeenCalledWith('Title', 'Body', 'ENG', ['bug'], testToken)
})
})
describe('moveIssueToInProgress', () => {
it('should pass apiToken to updateLinearIssueState', async () => {
vi.mocked(updateLinearIssueState).mockResolvedValue(undefined)
const service = new LinearService({ apiToken: testToken })
await service.moveIssueToInProgress('ENG-123')
expect(updateLinearIssueState).toHaveBeenCalledWith('ENG-123', 'In Progress', testToken)
})
})
})Detailed Execution Order
Step 1: Update linear.ts function signatures and LinearService.ts behavior
Files: src/utils/linear.ts, src/lib/LinearService.ts
src/utils/linear.ts: AddapiToken?: stringparam + pass tocreateLinearClient(apiToken)for all 12 functions listed in table above -> Verify: each function signature has the new param, eachcreateLinearClient()call passes itsrc/utils/linear.ts: Update JSDoc@paramblocks for each modified function -> Verify: docs mention apiTokensrc/lib/LinearService.ts:26: Update JSDoc comment for apiToken config field -> Verify: no mention of setting env varsrc/lib/LinearService.ts:38-39: Addprivate apiToken?: stringproperty -> Verify: property declaredsrc/lib/LinearService.ts:45-51: Replace env var side effect withthis.apiToken = this.config.apiToken-> Verify: noprocess.env.LINEAR_API_TOKENassignmentsrc/lib/LinearService.ts:101: Passthis.apiTokentofetchLinearIssue-> Verify: second arg isthis.apiTokensrc/lib/LinearService.ts:166: Passthis.apiTokentocreateLinearIssue-> Verify: sixth arg isthis.apiTokensrc/lib/LinearService.ts:192: Passthis.apiTokentoupdateLinearIssueState-> Verify: third arg isthis.apiToken
Step 2: Rewrite tests
Files: src/lib/LinearService.test.ts
- Replace all constructor env var tests with behavior tests per test structure above -> Verify:
pnpm test src/lib/LinearService.test.tspasses - Add tests for
fetchIssue,createIssue,moveIssueToInProgressverifying apiToken passing -> Verify: tests pass
Step 3: Build verification
Files: None (build step)
- Run
pnpm build-> Verify: TypeScript compiles with no errors - Run
pnpm test-> Verify: all tests pass
Execution Plan
- Run Step 1 (sequential -- modify
linear.tsandLinearService.tstogether since LinearService imports from linear.ts and the signatures must match) - Run Step 2 (sequential -- tests depend on Step 1 signature changes being complete)
- Run Step 3 (sequential -- build/test verification depends on all code changes)
Dependencies and Configuration
None
Implementation CompleteSummaryRefactored Linear API token handling to pass the token explicitly through the call stack instead of setting Changes Made
Validation Results
Detailed Changes by File (click to expand)src/utils/linear.tsChanges: Added optional
src/lib/LinearService.tsChanges: Removed env var side effect, explicit token passing
src/lib/LinearService.test.tsChanges: Full rewrite of test assertions
|
…env var Remove process.env.LINEAR_API_TOKEN side effect from LinearService constructor. Add optional apiToken parameter to 12 functions in linear.ts, store token as instance property in LinearService, and pass it explicitly to each utility call. MCP subprocess continues using env var injection via mcp.ts. Fixes #590
01f9d65 to
57054b7
Compare
Fixes #590
Refactor Linear API token: pass through stack instead of setting env var
Problem
LinearServiceconstructor setsprocess.env.LINEAR_API_TOKENas a side effect (src/lib/LinearService.ts:50), and all functions insrc/utils/linear.tsrely on this env var being set viagetLinearApiToken()→createLinearClient().This caused a bug (#590) where
il list --childrencalledgetLinearChildIssueswithout first instantiatingLinearService, so the token was never in the environment.Setting env vars as a side effect is fragile — any new code path that calls Linear utility functions without going through
LinearServicewill hit the sameUNAUTHORIZEDerror.Desired behavior
All functions in
src/utils/linear.tsshould accept the API token as a parameter (with env var as fallback), passed down through the call stack.LinearServiceshould stop settingprocess.env.LINEAR_API_TOKEN.Scope
src/utils/linear.ts— Update all exported functions to accept an optionalapiTokenparameter and pass it tocreateLinearClient():fetchLinearIssuecreateLinearIssuecreateLinearChildIssuecreateLinearCommentupdateLinearIssueStategetLinearCommentupdateLinearCommentfetchLinearIssueCommentsgetLinearChildIssues(already done in Refactor Linear API token: pass through stack instead of setting env var #590)createLinearIssueRelationgetLinearIssueDependenciesdeleteLinearIssueRelationfetchLinearIssueList(already acceptsapiToken)findLinearIssueRelationsrc/lib/LinearService.ts— Stop settingprocess.env.LINEAR_API_TOKEN. Instead, store the token as an instance property and pass it to eachlinear.tsfunction call.src/utils/image-processor.ts— Check if it readsLINEAR_API_TOKENfrom env; if so, update to accept it as a parameter.Tests — Update
LinearService.test.tsto remove env var assertions. Test files that set the env var for test setup are fine.This PR was created automatically by iloom.