diff --git a/openspec/specs/core/spec.md b/openspec/specs/core/spec.md index ea8d036..946431c 100644 --- a/openspec/specs/core/spec.md +++ b/openspec/specs/core/spec.md @@ -109,15 +109,15 @@ The system SHALL support committing images to a dedicated orphan branch. - AND no existing `gh-attach-assets` branch - WHEN `upload(file, target)` is called - THEN the system SHALL create an orphan branch `gh-attach-assets` -- AND commit the image file -- AND return the `raw.githubusercontent.com` URL +- AND commit the image file to a unique path on that branch +- AND return the GitHub raw URL rooted at `refs/heads/gh-attach-assets` #### Scenario: Subsequent upload - GIVEN an existing `gh-attach-assets` branch - WHEN `upload(file, target)` is called -- THEN the system SHALL commit the image to the existing branch -- AND return the raw URL with the commit SHA for immutability +- THEN the system SHALL commit the image to a new unique path on the existing branch +- AND return the GitHub raw URL for that branch path ### Requirement: Strategy Selection and Fallback diff --git a/openspec/specs/testing/spec.md b/openspec/specs/testing/spec.md index 41fa2ad..39f2ffb 100644 --- a/openspec/specs/testing/spec.md +++ b/openspec/specs/testing/spec.md @@ -54,7 +54,7 @@ Each upload strategy SHALL have comprehensive unit tests. - THEN tests SHALL cover: - Creating orphan branch - Committing to existing branch - - URL generation with commit SHA + - URL generation using the GitHub raw URL format #### Scenario: Cookie extraction strategy unit tests diff --git a/src/core/strategies/repoBranch.ts b/src/core/strategies/repoBranch.ts index 0d18a30..efc6440 100644 --- a/src/core/strategies/repoBranch.ts +++ b/src/core/strategies/repoBranch.ts @@ -1,4 +1,5 @@ import { Octokit } from "@octokit/rest"; +import { randomUUID } from "crypto"; import { readFileSync } from "fs"; import { basename } from "path"; import { formatAttachmentMarkdown } from "../attachment.js"; @@ -9,7 +10,7 @@ const BRANCH_NAME = "gh-attach-assets"; /** * Repository Branch upload strategy using GitHub's REST API. - * Commits attachments to a dedicated orphan branch and returns raw.githubusercontent.com URLs. + * Commits attachments to a dedicated orphan branch and returns GitHub raw URLs. * * @param token GitHub API token with `contents:write` permission * @returns UploadStrategy implementation @@ -40,20 +41,20 @@ export function createRepoBranchStrategy(token: string): UploadStrategy { // Commit the file to the branch const filename = basename(filePath); + const assetPath = createAssetPath(filename); const fileContent = readFileSync(filePath, "base64"); - const commitSha = await commitFile( + await commitFile( octokit, target, filename, + assetPath, fileContent, branchSha, ); - // Generate the raw.githubusercontent.com URL with commit SHA - const url = - `https://raw.githubusercontent.com/${target.owner}/${target.repo}/` + - `${commitSha}/${filename}`; + // Use GitHub's authenticated raw URL so attachments resolve for private repositories. + const url = buildAssetUrl(target, assetPath); // Generate markdown const markdown = formatAttachmentMarkdown(filePath, url); @@ -83,6 +84,19 @@ export function createRepoBranchStrategy(token: string): UploadStrategy { }; } +function createAssetPath(filename: string): string { + return `${randomUUID()}/${filename}`; +} + +function buildAssetUrl(target: UploadTarget, assetPath: string): string { + const encodedAssetPath = assetPath + .split("/") + .map((segment) => encodeURIComponent(segment)) + .join("/"); + + return `https://github.com/${target.owner}/${target.repo}/raw/refs/heads/${BRANCH_NAME}/${encodedAssetPath}`; +} + /** * Ensures the assets branch exists, creating it if necessary. * @@ -185,9 +199,10 @@ async function commitFile( octokit: InstanceType, target: UploadTarget, filename: string, + assetPath: string, content: string, baseSha: string, -): Promise { +): Promise { try { // Create blob const { data: blob } = await octokit.rest.git.createBlob({ @@ -211,7 +226,7 @@ async function commitFile( base_tree: baseTree.sha, tree: [ { - path: filename, + path: assetPath, mode: "100644", type: "blob", sha: blob.sha, @@ -235,8 +250,6 @@ async function commitFile( ref: `heads/${BRANCH_NAME}`, sha: commit.sha, }); - - return commit.sha; } catch (err) { throw new UploadError( `Failed to commit file: ${err instanceof Error ? err.message : String(err)}`, diff --git a/test/e2e/upload.test.ts b/test/e2e/upload.test.ts index 47a4abd..fe71f0f 100644 --- a/test/e2e/upload.test.ts +++ b/test/e2e/upload.test.ts @@ -250,7 +250,7 @@ describe.skipIf(!E2E_ENABLED)("E2E Upload Tests", () => { // Verify result expect(result.url).toMatch( - /^https:\/\/raw\.githubusercontent\.com\/.+\/[a-f0-9]{40}\/test-image\.png$/, + /^https:\/\/github\.com\/.+\/raw\/refs\/heads\/gh-attach-assets\/[^/]+\/test-image\.png$/, ); expect(result.markdown).toMatch(/^!\[test-image\.png\]\(https:\/\//); expect(result.strategy).toBe("repo-branch"); @@ -273,10 +273,10 @@ describe.skipIf(!E2E_ENABLED)("E2E Upload Tests", () => { const result1 = await strategy.upload(TEST_IMAGE_PATH, target); const result2 = await strategy.upload(TEST_IMAGE_PATH, target); - // Both should succeed with different commit SHAs (URLs differ) + // Both should succeed with different unique branch paths expect(result1.strategy).toBe("repo-branch"); expect(result2.strategy).toBe("repo-branch"); - // The URLs will have different commit SHAs + // The URLs will have different asset paths expect(result1.url).not.toBe(result2.url); // Verify both URLs are accessible diff --git a/test/unit/core/strategies/repoBranch.test.ts b/test/unit/core/strategies/repoBranch.test.ts index 2ad0c41..10ebb48 100644 --- a/test/unit/core/strategies/repoBranch.test.ts +++ b/test/unit/core/strategies/repoBranch.test.ts @@ -9,6 +9,10 @@ import type { UploadTarget } from "../../../../src/core/types.js"; // Create a shared mock object that will be reused let mockOctokitInstance: Record; +vi.mock("crypto", () => ({ + randomUUID: vi.fn(() => "asset-id"), +})); + // Mock the Octokit module vi.mock("@octokit/rest", () => { return { @@ -130,8 +134,9 @@ describe("Repository Branch Strategy", () => { const result = await strategy.upload(mockFilePath, mockTarget); expect(result.strategy).toBe("repo-branch"); - expect(result.url).toContain("commit-sha"); - expect(result.url).toContain("test.png"); + expect(result.url).toBe( + "https://github.com/testowner/testrepo/raw/refs/heads/gh-attach-assets/asset-id/test.png", + ); expect(result.markdown).toContain("![test.png]"); }); @@ -173,8 +178,9 @@ describe("Repository Branch Strategy", () => { const result = await strategy.upload(mockFilePath, mockTarget); expect(result.strategy).toBe("repo-branch"); - expect(result.url).toContain("commit-sha"); - expect(result.url).toContain("test.mp4"); + expect(result.url).toBe( + "https://github.com/testowner/testrepo/raw/refs/heads/gh-attach-assets/asset-id/test.mp4", + ); expect(result.markdown).toBe(result.url); });