From 96bba78fbaffcd3cb9b001bd07d292cba3822773 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:19:36 -0500 Subject: [PATCH] feat: add readable Storybook branch bookmarks (#13231) ## Thinking Path > - Paperclip helps people manage AI agents for work. > - Maintainers use Storybook previews to review the board UI. > - Branch previews need stable bookmarks that people can read. > - The current publisher only provides a hashed branch path. > - This pull request adds a readable branch bookmark after each successful upload. > - Existing branch and build links keep working. ## Linked Issues or Issue Description Refs #13226. **What existing behavior does this improve?** Manual Storybook publication for repository branches. **Current behavior** The stable branch path contains a hash. The expected `/storybook/branches/master/` URL does not exist. **Proposed behavior** Each publication updates a readable bookmark. The action summary and Markdown artifact link it. Master uses `/storybook/branches/master/`. Other names use a safe path segment that preserves case and escapes special characters. **Reason and benefit** Maintainers can save and share a readable URL that opens the latest published branch build. **Breaking changes** None. Existing hashed branch entries still update. Existing build URLs remain valid. **Additional context** This follows the publisher in #13226. A duplicate search found no related bookmark change. It does not overlap planned core work in ROADMAP.md. ## What Changed - Generate readable branch bookmarks without collisions with existing build directories. - Upload the bookmark only after the full build and compatibility entry uploads succeed. - Link the bookmark in the existing summary and Markdown artifact. - Document branch-name escaping and test path isolation, stable links, and upload order. ## Verification - `node --test scripts/__tests__/storybook-deploy.test.mjs`: 20 tests pass. - `actionlint .github/workflows/storybook-deploy.yml .github/workflows/storybook-visual.yml`: passes. - `git diff --check`: passes. - [Master bookmark publication](https://github.com/paperclipai/paperclip/actions/runs/34613344758): passed. Opened `/storybook/branches/master/` in the browser and confirmed a story renders. Downloaded the Markdown report and verified its bookmark link. - [Feature branch bookmark publication](https://github.com/paperclipai/paperclip/actions/runs/34613449034): passed. Its separate bookmark uses `codex~2Fstorybook-bookmarks`. - Greptile: 5/5 on `dccaf10413ecf447cb34e622b6b3c505791abb51`, with no unresolved review threads. All current-head Paperclip CI gates pass, including typecheck, tests, build, browser suites, and the canary dry run. - Full local repository checks were not repeated for this focused publisher change. The preceding run passed typecheck but encountered unrelated native-session test failures. ## Risks - Special characters in branch names use `~HH` byte escapes. For example, `feature/foo` becomes `feature~2Ffoo`. - Names that could overlap an existing hashed build directory escape the final hyphen. Very long names retain a hash suffix. - The two branch entries update separately. If the final upload fails, the workflow fails and a rerun can repair the bookmark. ## Model Used OpenAI GPT-6 via Codex, with reasoning, shell tools, and live deployment verification. The exact runtime model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- .github/scripts/publish-storybook.cjs | 2 ++ .github/scripts/storybook-destination.cjs | 16 +++++++++++++-- doc/DEVELOPING.md | 13 +++++++++--- doc/STORYBOOK-DEPLOYMENT.md | 8 ++++++++ scripts/__tests__/storybook-deploy.test.mjs | 22 +++++++++++++++++++-- 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/.github/scripts/publish-storybook.cjs b/.github/scripts/publish-storybook.cjs index e0578a4bc7..f4fdeb1c03 100644 --- a/.github/scripts/publish-storybook.cjs +++ b/.github/scripts/publish-storybook.cjs @@ -35,6 +35,8 @@ const indexFile = path.join(process.env.RUNNER_TEMP, 'storybook-branch-index.htm fs.writeFileSync(indexFile, branchIndex(destination.buildUrl)); aws(['s3', 'cp', indexFile, `s3://${destination.bucket}/${destination.prefix}/index.html`, '--content-type', 'text/html; charset=utf-8', '--cache-control', 'no-cache,max-age=0,must-revalidate', '--only-show-errors']); +aws(['s3', 'cp', indexFile, `s3://${destination.bucket}/${destination.bookmarkPrefix}/index.html`, + '--content-type', 'text/html; charset=utf-8', '--cache-control', 'no-cache,max-age=0,must-revalidate', '--only-show-errors']); const report = `[Branch Storybook](${destination.url})\n\n[This build](${destination.buildUrl})\n\nCommit: \`${destination.sha}\`\n`; const reportPath = path.join(process.env.RUNNER_TEMP, 'storybook-deployment.md'); fs.writeFileSync(reportPath, report); diff --git a/.github/scripts/storybook-destination.cjs b/.github/scripts/storybook-destination.cjs index 681fe4bb36..600cd30763 100644 --- a/.github/scripts/storybook-destination.cjs +++ b/.github/scripts/storybook-destination.cjs @@ -18,9 +18,21 @@ function storybookDestination({ branch, sha, runId, runAttempt, bucket, baseUrl const branchKey = `${label}-${digest}`; const prefix = `storybook/branches/${branchKey}`; const buildPrefix = `${prefix}/builds/${runId}-${runAttempt}`; + // Use one reversible path segment: slashes and special characters become + // ~HH UTF-8 bytes, so feature/foo and feature-foo never share a bookmark. + let bookmarkKey = [...Buffer.from(branch)].map((byte) => + /[A-Za-z0-9_-]/.test(String.fromCharCode(byte)) + ? String.fromCharCode(byte) : `~${byte.toString(16).toUpperCase().padStart(2, '0')}`).join(''); + // Reserve the existing hashed directories, including all immutable builds. + bookmarkKey = bookmarkKey.replace(/-([a-f0-9]{16})$/, '~2D$1'); + // Keep arbitrarily long ref names within S3's object-key limit. ~long cannot + // occur in the reversible encoding, whose escapes contain only hex digits. + if (bookmarkKey.length > 900) bookmarkKey = `${bookmarkKey.slice(0, 800)}~long-${digest}`; + const bookmarkPrefix = `storybook/branches/${bookmarkKey}`; return { - branch, sha, bucket, branchKey, prefix, buildPrefix, - url: `${base.origin}/${prefix}/index.html`, + branch, sha, bucket, branchKey, prefix, buildPrefix, bookmarkPrefix, + url: `${base.origin}/${bookmarkPrefix}/`, + legacyUrl: `${base.origin}/${prefix}/index.html`, buildUrl: `${base.origin}/${buildPrefix}/index.html`, }; } diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 2b8fdc6f75..1cf4ccbce5 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -137,9 +137,16 @@ branch updates its stable URL only after all files for the new build are uploade Previous build links keep working. The branch entry preserves Storybook query parameters and fragments when redirecting to the completed build. -URLs use `storybook/branches/-/index.html`. The hash preserves -the distinction between branch names such as `feature/foo`, `feature-foo`, and -`Feature/foo`. Build files live under that branch's `builds/-/`. +Bookmark URLs use `storybook/branches//`, for example +`https://d1p6rlowie26tp.cloudfront.net/storybook/branches/master/`. +Copy the **stable branch URL** from the run summary when saving a bookmark; +opening it redirects to the latest published build. Branch names preserve case. +Characters other than letters, digits, `_`, and `-` use `~HH` UTF-8 escapes, so +`feature/foo` becomes `feature~2Ffoo` and stays distinct from `feature-foo`. +Names ending in a hyphen and 16 lowercase hex digits escape that hyphen to +reserve the existing build directories. Very long names use a hash suffix. +Existing hashed branch URLs keep updating and remain valid. Build files remain +under `storybook/branches/-/builds/-/`. `deployment.json` in each build records its branch, source commit and URLs. Builds run independently; publication is serialized per branch. Retained builds are not automatically deleted and will accumulate until an operator prunes them. diff --git a/doc/STORYBOOK-DEPLOYMENT.md b/doc/STORYBOOK-DEPLOYMENT.md index cbac54cdd0..00a958b9c8 100644 --- a/doc/STORYBOOK-DEPLOYMENT.md +++ b/doc/STORYBOOK-DEPLOYMENT.md @@ -16,6 +16,14 @@ The distribution's default behavior disables edge caching and rewrites directory URLs to `index.html`. Stable branch indexes send `no-cache`; unique build objects send `immutable`. No invalidations or CloudFront write permissions are needed. +Each publication updates a readable bookmark, such as +`https://d1p6rlowie26tp.cloudfront.net/storybook/branches/master/`, after the +immutable build upload completes. It also updates the previous hashed branch +entry for compatibility. The run summary and Markdown artifact link the bookmark. +Branch names use one escaped path segment, preserving case and separating slashes +from hyphens; see [the branch publishing guide](DEVELOPING.md#publish-a-branch-storybook) +for the encoding. No additional AWS permissions or distribution changes are needed. + ## GitHub configuration Create environment `storybook-deploy` with required reviewers set to the diff --git a/scripts/__tests__/storybook-deploy.test.mjs b/scripts/__tests__/storybook-deploy.test.mjs index edbe830f13..9bade4ce87 100644 --- a/scripts/__tests__/storybook-deploy.test.mjs +++ b/scripts/__tests__/storybook-deploy.test.mjs @@ -106,7 +106,23 @@ test('different branches have distinct stable URLs, including names that sanitiz const branches = ['feature/foo', 'feature-foo', 'Feature/foo', 'master', 'feature_foo', 'a'.repeat(100), 'a'.repeat(101)]; const urls = branches.map(branch => storybookDestination({ ...input, branch }).url); assert.equal(new Set(urls).size, branches.length); - assert.ok(urls.every(url => /^https:\/\/example.cloudfront.net\/storybook\/branches\/[a-z0-9-]+\/index.html$/.test(url))); + assert.ok(urls.every(url => /^https:\/\/example.cloudfront.net\/storybook\/branches\/[A-Za-z0-9_~\-]+\/$/.test(url))); + assert.equal(storybookDestination({ ...input, branch: 'master' }).url, + 'https://example.cloudfront.net/storybook/branches/master/'); + assert.equal(storybookDestination(input).url, + 'https://example.cloudfront.net/storybook/branches/feature~2Ffoo/'); +}); +test('bookmark paths cannot collide with other branches or existing immutable build directories', () => { + const branches = ['feature/foo', 'feature~2Ffoo', '../master', 'master/index.html', + 'master', storybookDestination({ ...input, branch: 'master' }).branchKey, + 'a'.repeat(1000), 'a'.repeat(1001), 'café', 'caf~C3~A9']; + const destinations = branches.map(branch => storybookDestination({ ...input, branch })); + assert.equal(new Set(destinations.map(d => d.url)).size, branches.length); + for (const d of destinations) { + assert.doesNotMatch(d.bookmarkPrefix.slice('storybook/branches/'.length), /[/.]/); + assert.ok(Buffer.byteLength(`${d.bookmarkPrefix}/index.html`) <= 1024); + assert.ok(destinations.every(other => d.bookmarkPrefix !== other.prefix)); + } }); test('redeploying a branch preserves its entry URL and creates a new build URL', () => { const a = storybookDestination(input); @@ -166,10 +182,12 @@ function publishFixture(options = {}) { test('publisher uploads a complete build then updates only that branch entry', () => { const { result, uploads } = publishFixture(); assert.equal(result.status, 0, result.stderr); - assert.equal(uploads.length, 2); + assert.equal(uploads.length, 3); const d = storybookDestination(input); assert.ok(uploads[0].includes(`s3://${input.bucket}/${d.buildPrefix}/`)); assert.ok(uploads[1].includes(`s3://${input.bucket}/${d.prefix}/index.html`)); + assert.ok(uploads[2].includes(`s3://${input.bucket}/${d.bookmarkPrefix}/index.html`)); + assert.ok(uploads[2].includes('no-cache,max-age=0,must-revalidate')); assert.ok(uploads[0].includes('--no-follow-symlinks')); assert.doesNotMatch(JSON.stringify(uploads), /--delete/); });