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 <noreply@paperclip.ing>
This commit is contained in:
parent
3b03c4b9eb
commit
96bba78fba
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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`,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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/<readable-branch>-<hash>/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/<run-id>-<attempt>/`.
|
||||
Bookmark URLs use `storybook/branches/<branch>/`, 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/<readable-branch>-<hash>/builds/<run-id>-<attempt>/`.
|
||||
`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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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/);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue