test: close remaining coverage gaps in strategies, CLI upload, and MCP login

- Release-asset strategy: add tests for non-Error rate limit detection
  (String(err) branch in isRateLimitError) and asset listing failure catch
  block that falls through with original filename
- Browser-session strategy: add test for generic Error wrapping through
  confirmUpload JSON parse failure path (CONFIRM_UPLOAD_FAILED code)
- CLI upload command: add test for no-strategies-available path when config
  strategy-order yields only token-requiring strategies without auth
- MCP login tool: add tests for elicitation decline action and empty token
  fallback

Coverage: 97.05% → 97.5% statements, 92.16% → 92.76% branches
upload.ts: 94.3% → 99.36%, releaseAsset.ts: 98.89% → 99.63%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Addono

IMPLEMENTATION_PLAN.md

@@ -512,3 +512,19 @@ This plan lists prioritized tasks required to bring the implementation into full
     - **Raised coverage thresholds**: lines 75→90%, functions 85→90%, branches 78→85%, statements 75→90%. All thresholds pass.
     - **Overall coverage**: statements 95.68→97.05%, branches 88.79→92.16%.
     - All validation passes: `typecheck`, `lint` (0 errors), `format:check`, `build`, `test` (418 tests), `npm audit --production` (0 vulnerabilities).
+
+## 34. Coverage Gap Closure and Edge Case Testing
+
+- **Task:** Close remaining coverage gaps in upload command, release-asset strategy, browser-session strategy, and MCP login tool edge cases. **[COMPLETE]**
+  - **Spec:** Testing/spec.md (Unit Test Coverage ≥90%), Core/spec.md (Strategy error handling), MCP/spec.md (Login Tool), CLI/spec.md (Upload Command)
+  - **Files:** test/unit/core/strategies/releaseAsset.test.ts, test/unit/core/strategies/browserSession.test.ts, test/unit/cli/commands/upload.test.ts, test/unit/mcp/handlers.test.ts
+  - **Tests:** 6 new tests
+  - **Dependencies:** None
+  - **Notes:**
+    - **Targets Test Coverage (30/100), Spec Compliance (0/100)** from Score-Maximisation Context.
+    - **Release-asset strategy**: Added test for non-Error rate limit detection via `String(err).toLowerCase()` branch (line 36), and test for asset listing failure catch block (line 289) that verifies original filename is used on listing error.
+    - **Browser-session strategy**: Added test for generic Error (non-Auth/Upload) wrapping through the confirmUpload JSON parse failure path, verifying CONFIRM_UPLOAD_FAILED error code.
+    - **CLI upload command**: Added test for no-strategies-available path (lines 147-154) when config strategy-order yields only token-requiring strategies without a token set.
+    - **MCP login tool**: Added tests for elicitation decline action and empty token elicitation fallback.
+    - **Coverage improvements**: Overall 97.05→97.5% statements, 92.16→92.76% branches. upload.ts 94.3→99.36%, releaseAsset.ts 98.89→99.63%.
+    - All validation passes: `typecheck`, `lint` (0 errors), `format:check`, `test` (424 tests), `npm audit --production` (0 vulnerabilities).

test/unit/cli/commands/upload.test.ts

@@ -248,6 +248,20 @@ describe("uploadCommand unit tests", () => {
     delete process.env.GH_TOKEN;
   });
 
+  it("throws NoStrategyAvailableError when config strategy-order yields no available strategies", async () => {
+    delete process.env.GITHUB_TOKEN;
+    delete process.env.GH_TOKEN;
+    delete process.env.GH_ATTACH_COOKIES;
+    // Only token-requiring strategies in the order, but no token is set
+    vi.mocked(loadConfig).mockReturnValue({
+      "strategy-order": ["release-asset", "repo-branch"],
+    });
+
+    await expect(
+      uploadCommand(["file.png"], { target: "owner/repo#1" }),
+    ).rejects.toThrow(NoStrategyAvailableError);
+  });
+
   it("handles stdin mode by writing temp file and cleaning up", async () => {
     const fakeStdin = new Readable({ read() {} });
     const origStdin = process.stdin;

test/unit/core/strategies/browserSession.test.ts

@@ -526,6 +526,49 @@ describe("Browser Session Strategy", () => {
       );
     });
 
+    it("wraps generic Error (non-Auth/Upload) in UploadError with BROWSER_SESSION_FAILED code", async () => {
+      const strategy = createBrowserSessionStrategy("test-cookie");
+      const mockFilePath = "/tmp/test.png";
+
+      // Simulate a successful 3-step flow where confirmUpload's json() throws a TypeError
+      // Step 1: getRepositoryId succeeds
+      (global.fetch as ReturnType<typeof vi.fn>)
+        .mockResolvedValueOnce({
+          ok: true,
+          status: 200,
+          json: async () => ({ id: 12345 }),
+        })
+        // Step 2: getUploadPolicy succeeds
+        .mockResolvedValueOnce({
+          ok: true,
+          status: 200,
+          json: async () => ({
+            upload_url: "https://s3.example.com/upload",
+            form: { key: "value" },
+            token: "csrf-token",
+          }),
+        })
+        // Step 3: uploadToS3 succeeds
+        .mockResolvedValueOnce({
+          ok: true,
+          status: 204,
+        })
+        // Step 4: confirmUpload succeeds but json() throws TypeError
+        .mockResolvedValueOnce({
+          ok: true,
+          status: 200,
+          json: async () => {
+            throw new TypeError("Cannot read properties of undefined");
+          },
+        });
+
+      const error = await strategy
+        .upload(mockFilePath, mockTarget)
+        .catch((e: unknown) => e);
+      expect(error).toBeInstanceOf(UploadError);
+      expect((error as UploadError).code).toBe("CONFIRM_UPLOAD_FAILED");
+    });
+
     it("re-throws AuthenticationError without wrapping", async () => {
       const strategy = createBrowserSessionStrategy("test-cookie");
       const mockFilePath = "/tmp/test.png";

test/unit/core/strategies/releaseAsset.test.ts

@@ -464,5 +464,57 @@ describe("Release Asset Strategy", () => {
           err instanceof UploadError && err.code === "RELEASE_LOOKUP_FAILED",
       );
     });
+
+    it("detects rate limit from non-Error thrown value with rate limit message", async () => {
+      const strategy = createReleaseAssetStrategy("token");
+      // A non-Error object with status 403 and "rate limit" in its string form
+      // This exercises the String(err).toLowerCase() branch in isRateLimitError
+      const nonErrorObj = Object.create(null) as {
+        status: number;
+        message: string;
+        toString: () => string;
+      };
+      nonErrorObj.status = 403;
+      nonErrorObj.message = "rate limit exceeded";
+      nonErrorObj.toString = () => "rate limit exceeded";
+      mockOctokitInstance.rest.repos.getReleaseByTag.mockRejectedValue(
+        nonErrorObj,
+      );
+
+      await expect(
+        strategy.upload("/tmp/test.png", mockTarget),
+      ).rejects.toSatisfy(
+        (err: UploadError) =>
+          err instanceof UploadError && err.code === "RATE_LIMIT_EXCEEDED",
+      );
+    });
+
+    it("continues with original filename when listing assets throws", async () => {
+      const strategy = createReleaseAssetStrategy("valid-token");
+      const mockFilePath = "/tmp/test.png";
+      const mockRelease = { id: 123 };
+      const mockAsset = {
+        name: "test.png",
+        browser_download_url: "https://github.com/releases/download/test.png",
+      };
+
+      mockOctokitInstance.rest.repos.getReleaseByTag.mockResolvedValue({
+        data: mockRelease,
+      });
+      // Listing assets fails — should silently fall through and use original filename
+      mockOctokitInstance.rest.repos.listReleaseAssets.mockRejectedValue(
+        new Error("Network failure"),
+      );
+      mockOctokitInstance.rest.repos.uploadReleaseAsset.mockResolvedValue({
+        data: mockAsset,
+      });
+
+      const result = await strategy.upload(mockFilePath, mockTarget);
+      expect(result.url).toBe(mockAsset.browser_download_url);
+      // Should use original filename since listing failed
+      const uploadCall =
+        mockOctokitInstance.rest.repos.uploadReleaseAsset.mock.calls[0][0];
+      expect(uploadCall.name).toBe("test.png");
+    });
   });
 });

test/unit/mcp/handlers.test.ts

@@ -606,4 +606,32 @@ describe("MCP server handlers", () => {
     expect(response.isError).toBeUndefined();
     expect(response.content[0]?.text).toMatch(/GITHUB_TOKEN/);
   });
+
+  it("login tool handles elicitation decline action", async () => {
+    hoisted.mockServerGetClientCapabilities.mockReturnValue({
+      elicitation: { form: true },
+    });
+    hoisted.mockServerElicitInput.mockResolvedValue({ action: "decline" });
+
+    const { call } = await startServerAndGetHandlers();
+    const response = await call({ params: { name: "login", arguments: {} } });
+
+    expect(response.content[0]?.text).toContain("cancelled");
+  });
+
+  it("login tool handles elicitation with empty token", async () => {
+    hoisted.mockServerGetClientCapabilities.mockReturnValue({
+      elicitation: { form: true },
+    });
+    hoisted.mockServerElicitInput.mockResolvedValue({
+      action: "accept",
+      content: { token: "" },
+    });
+
+    const { call } = await startServerAndGetHandlers();
+    const response = await call({ params: { name: "login", arguments: {} } });
+
+    // Empty token should fall through to static guidance
+    expect(response.content[0]?.text).toMatch(/GITHUB_TOKEN/);
+  });
 });