Skip to content

Commit 5d83a07

Browse files
kyle-sextonclaude
andcommitted
fix(review): track the previous hosted page whenever pages-publish returns a new id
A second wrong public landing returned a new id on the same host, so the first public copy was overwritten in the sidecar and never deleted. Any id change now moves the old copy under `stale` for deletion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 69ec517 commit 5d83a07

3 files changed

Lines changed: 13 additions & 2 deletions

File tree

‎docs/conventions/rendered-views/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ Producers on a type: `/visualization:present` (Slides).
459459
from a private repository when a caller skips the gate.
460460
- **The page id lives in the plugin's data directory**, in
461461
`hosted/<owner>__<repo>__<pr>.json` holding `pages-publish`'s JSON line, so a rebuild of the same
462-
pull request's page replaces it. When the returned visibility differs from the sidecar's, the
462+
pull request's page replaces it. When the returned id differs from the sidecar's, the
463463
lane writes the new sidecar with the old id listed under `stale`, then deletes it from the old
464464
host; an id whose delete fails stays under `stale` and is retried on the next publish.
465465
- **The save button stays on a top-level https page**, because the host's policy allows downloads.

‎plugins/review/skills/explain-change/scripts/publish-hosted.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export function publishHosted({ page, repo, pr, repoVisibility, dataDir }) {
100100
// The old copy is recorded as stale before its delete runs, so a failed or interrupted delete is retried next time.
101101
const current = { id, visibility: landed, url };
102102
const stale = (old?.stale ?? []).filter((e) => e.id !== id);
103-
if (old && old.visibility !== landed) stale.push(entry(old));
103+
if (old && old.id !== id) stale.push(entry(old));
104104
writeSidecar(sidecar, current, stale);
105105

106106
const result = { medium: "hosted", visibility: landed, url, reason: gate.reason };

‎plugins/review/tests/explain-change.test.mjs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,17 @@ process.exitCode = Number((process.argv[2] === "--delete" ? process.env.FAKE_DEL
556556
]);
557557
assert.deepEqual(JSON.parse(readFileSync(at.sidecarPath, "utf8")), JSON.parse(answer(idB, "private")));
558558
});
559+
test("a second wrong public landing under a new id still deletes the first", () => {
560+
const idC = "C".repeat(22);
561+
const at = setup(sample, answer(idA, "public"));
562+
const out = publish(at, { visibility: "PRIVATE", out: answer(idC, "public") });
563+
assert.equal(out.status, 1);
564+
assert.deepEqual(calls(), [
565+
[at.page, "--visibility", "private"],
566+
["--delete", idA, "--visibility", "public"],
567+
]);
568+
assert.deepEqual(JSON.parse(readFileSync(at.sidecarPath, "utf8")), JSON.parse(answer(idC, "public")));
569+
});
559570
test("a private page pages-publish puts on the public host exits 1 and names the URL; the sidecar keeps it for cleanup", () => {
560571
const at = setup();
561572
const out = publish(at, { visibility: "PRIVATE", out: answer(idB, "public") });

0 commit comments

Comments
 (0)