feat(ado): add work item attachment upload

[claude]

Summary

• Adds pncli ado work add-attachment --id <n> --file <path> [--comment <text>] command
• Implements a two-step ADO upload flow: POST file to _apis/wit/attachments, then PATCH the work item to add the AttachedFile relation
• Adds adoUpload<T> method to HttpClient for binary uploads through the ADO authenticating fetcher
• Includes 2 unit tests covering the upload + link flow and optional comment attribute

Closes #204

Generated with Claude Code

[claude]

Claude finished @claude[bot]'s task in 3m 53s —— View job

---

Review Complete

• Clean up previous Claude comments
• Read changed files and review code
• Post inline comments on specific findings
• Formal review: could not approve (GitHub blocks self-approval — a human reviewer should approve)

---

Overall verdict: Approve — the implementation is correct and clean. Two minor inline suggestions.

What was verified:

• Auth headers: the ADO fetcher correctly merges Content-Type: <mime> from adoUpload over its own default application/json, so binary uploads get the right MIME type
• URL construction: fileName is encodeURIComponent-encoded in the path before buildUrl, which uses new URL() — no double-encoding occurs
• Step 2 PATCH: uses application/json-patch+json which correctly overrides the ADO fetcher's default application/json content type
• Test coverage: both the upload+link flow and optional comment attribute are exercised with a stubbed fetch

Inline comments posted:

File	Line	Type	Note
src/services/ado/client/work.ts	152	Suggestion	Pre-check file existence before readFileSync to surface a PncliError instead of raw ENOENT
src/services/ado/client/work.test.ts	355	Nit	Wrap assertions in try/finally so temp files are cleaned up on failure

Neither is blocking.

[claude]

Suggestion — no file-existence check before the synchronous read

readFileSync throws a raw Node.js ENOENT error (not a PncliError) when the file doesn't exist. The fail() handler will still catch it, but the output will be a raw system error message rather than the usual PncliError format.

A lightweight pre-check improves the UX without much overhead:

Suggested change

	const fileContent = readFileSync(filePath);
	if (!require('fs').existsSync(filePath)) {
	throw new PncliError(`File not found: ${filePath}`, 1);
	}
	const fileContent = readFileSync(filePath);

Or even simpler — import existsSync alongside readFileSync at the top. Not a blocker since the error still surfaces, just noisier than a PncliError.

[kolatts]

@claude[agent] adjust this

[claude]

Nit — temp file isn't cleaned up if an assertion throws

fs.unlinkSync(tmpFile) is called after the assertions, so a failing assertion will leak the temp file. Wrapping in try/finally is the safe pattern here:

Suggested change

	expect(capturedRequests[1].method).toBe('PATCH');
	try {
	expect(result.id).toBe('att-guid-123');
	expect(result.url).toBe('https://ado.example.com/myorg/_apis/wit/attachments/att-guid-123');
	// First request: upload to attachments endpoint
	expect(capturedRequests[0].url).toContain('_apis/wit/attachments');
	expect(capturedRequests[0].url).toContain('fileName=pncli-test-report.txt');
	expect(capturedRequests[0].method).toBe('POST');
	expect(capturedRequests[0].contentType).toBe('text/plain');
	// Second request: PATCH work item to link the attachment
	expect(capturedRequests[1].url).toContain('_apis/wit/workitems/42');
	expect(capturedRequests[1].method).toBe('PATCH');
	const patch = capturedRequests[1].body as Array<{ op: string; path: string; value: { rel: string; url: string } }>;
	expect(patch[0].op).toBe('add');
	expect(patch[0].path).toBe('/relations/-');
	expect(patch[0].value.rel).toBe('AttachedFile');
	expect(patch[0].value.url).toBe(attachmentResponse.url);
	} finally {
	fs.unlinkSync(tmpFile);
	}

Same applies to the second test (pncli-test-notes.txt). Low-risk since these run in tmpdir, but worth doing right.

[kolatts]

@claude[agent] can we abstract this outside the ado client

[Claude]

Abstracted guessMimeType to a shared utility module at src/lib/mime.ts. The function is now used by both ADO and Jira clients, eliminating code duplication. Added 9 comprehensive unit tests covering various file types and edge cases.

Commit: 7f596bb

[kolatts]

@claude[agent] add this to the scale reference.