mirror of
https://github.com/paperclipai/paperclip-docs.git
synced 2026-11-07 06:16:07 -10:00
fix(sync): pin drift check to an immutable SHA, key cache by SHA (#62)
Ports the drift-checker fix to `main` (same change already merged to `nightly` in #61). `check-drift.mjs` is byte-identical across branches, so this cherry-pick applies cleanly. ## Problem The drift checker printed the parent's **live** SHA in its header but keyed its file cache by the moving name `drift-master` with no invalidation — reusing days-old file contents while claiming to check today's commit, silently hiding real drift. ## Fix - Resolve the ref to a full commit SHA before any reads; cache key = `drift-<sha>`, never `drift-master`. - Fail closed if the ref can't be resolved. - Mode-aware ref from `.sync-state.json` (release → `base_release_sha`, nightly → `last_seen_parent_sha`) before falling back to the default branch. `main` is release-mode, so its checks now pin to the release SHA instead of moving `master`. - Testable cache path (`PAPERCLIP_SYNC_CACHE_DIR` + fixture SHA stub); +2 regression tests. `npm run sync:test`: 37 passed, 0 failed. ## Triage note Running the fixed checker on `main`'s released docs against the release tag `v2026.707.0` returns **0 drift** — released docs are correct about the shipped product; this is a tooling fix only, no doc content changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
773f72838a
commit
55de2cbb81
2 files changed
+151
-15
No files matched your search
@@ -34,8 +34,12 @@ import { fileURLToPath } from "node:url";
|
||||
const SELF_DIR = dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = resolve(SELF_DIR, "../..");
|
||||
const DOCS = join(ROOT, "docs");
|
||||
const CACHE_ROOT = "/tmp/paperclip-sync";
|
||||
const CACHE_ROOT = process.env.PAPERCLIP_SYNC_CACHE_DIR || "/tmp/paperclip-sync";
|
||||
const FIXTURE_DIR = process.env.PAPERCLIP_SYNC_FIXTURE_DIR || null;
|
||||
// Caching is normally off in fixture mode (fixtures are already deterministic
|
||||
// per ref, so there's nothing to save). Tests that need to exercise the
|
||||
// cache-key path opt in explicitly by setting PAPERCLIP_SYNC_CACHE_DIR.
|
||||
const CACHE_ENABLED = !FIXTURE_DIR || !!process.env.PAPERCLIP_SYNC_CACHE_DIR;
|
||||
|
||||
// --- gh wrapper -------------------------------------------------------------
|
||||
|
||||
@@ -140,6 +144,31 @@ function ghDefaultBranch(repo) {
|
||||
return r.stdout.trim() || "master";
|
||||
}
|
||||
|
||||
function ghResolveSha(repo, ref) {
|
||||
// Resolve a ref (branch, tag, or SHA) to a full immutable commit SHA.
|
||||
// Returns null if it cannot be resolved (caller fails closed on null).
|
||||
if (FIXTURE_DIR) {
|
||||
// Fixture stub: _fixtures/commit-<ref>.json → { "sha": "<full sha>" }.
|
||||
// When absent, the ref is returned as-is — this keeps ref-keyed fixtures
|
||||
// working and means tests that don't care about SHA resolution are
|
||||
// unaffected by pinning.
|
||||
const file = join(FIXTURE_DIR, `commit-${ref}.json`);
|
||||
if (existsSync(file)) {
|
||||
try {
|
||||
const body = JSON.parse(readFileSync(file, "utf8"));
|
||||
if (body && typeof body.sha === "string" && body.sha) return body.sha;
|
||||
} catch {
|
||||
// fall through to returning the ref unchanged
|
||||
}
|
||||
}
|
||||
return ref;
|
||||
}
|
||||
const r = spawnSync("gh", ["api", `repos/${repo}/commits/${ref}`, "-q", ".sha"], { encoding: "utf8" });
|
||||
if (r.status !== 0) return null;
|
||||
const sha = r.stdout.trim();
|
||||
return sha || null;
|
||||
}
|
||||
|
||||
function ghTreeFiles(repo, ref) {
|
||||
if (FIXTURE_DIR) {
|
||||
const file = join(FIXTURE_DIR, `tree-${ref}.json`);
|
||||
@@ -174,7 +203,7 @@ function ghTreeFiles(repo, ref) {
|
||||
// --- caching ---------------------------------------------------------------
|
||||
|
||||
function cacheGet(refSlug, key) {
|
||||
if (FIXTURE_DIR) return null;
|
||||
if (!CACHE_ENABLED) return null;
|
||||
const dir = join(CACHE_ROOT, `drift-${refSlug}`);
|
||||
const file = join(dir, `${pathSlug(key)}.json`);
|
||||
if (!existsSync(file)) return null;
|
||||
@@ -186,7 +215,7 @@ function cacheGet(refSlug, key) {
|
||||
}
|
||||
|
||||
function cacheSet(refSlug, key, value) {
|
||||
if (FIXTURE_DIR) return;
|
||||
if (!CACHE_ENABLED) return;
|
||||
const dir = join(CACHE_ROOT, `drift-${refSlug}`);
|
||||
try {
|
||||
mkdirSync(dir, { recursive: true });
|
||||
@@ -719,6 +748,29 @@ function readDefaultRepo() {
|
||||
return "paperclipai/paperclip";
|
||||
}
|
||||
|
||||
function readSyncStateRef() {
|
||||
// Mode-aware ref from .sync-state.json (the sync flow's own state file):
|
||||
// - release mode → the immutable release SHA/tag whose surface these docs ship
|
||||
// - nightly mode → the quarantine-settled parent SHA we last synced against
|
||||
// This keeps the drift check pinned to the same ref the sync flow operated on,
|
||||
// instead of the moving default branch. Returns null when the file is
|
||||
// absent/unreadable or carries no usable ref (caller then falls back to the
|
||||
// default branch).
|
||||
try {
|
||||
const state = JSON.parse(readFileSync(join(ROOT, ".sync-state.json"), "utf8"));
|
||||
if (state.branch_mode === "release") {
|
||||
return state.base_release_sha || state.base_release_tag || null;
|
||||
}
|
||||
return state.last_seen_parent_sha || null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function shortRef(ref) {
|
||||
return /^[0-9a-f]{7,40}$/i.test(ref) ? ref.slice(0, 7) : ref;
|
||||
}
|
||||
|
||||
function usage() {
|
||||
return `usage: check-drift.mjs [--json] [--repo OWNER/REPO] [--against REF]\n`;
|
||||
}
|
||||
@@ -732,8 +784,30 @@ function main() {
|
||||
process.exit(0);
|
||||
}
|
||||
const repo = args.repo || readDefaultRepo();
|
||||
const against = args.against || ghDefaultBranch(repo);
|
||||
const refSlug = pathSlug(against);
|
||||
|
||||
// Determine the ref to check against, then pin it to an immutable commit SHA
|
||||
// BEFORE any reads. Precedence:
|
||||
// 1. --against (explicit override)
|
||||
// 2. .sync-state.json (mode-aware immutable ref)
|
||||
// 3. the parent repo's default branch (moving)
|
||||
// Whatever we land on is resolved to a full SHA and used as BOTH the fetch
|
||||
// ref and the cache key. Caching by a moving name (e.g. "master") is a
|
||||
// correctness bug: a stale entry from an earlier commit is reused forever
|
||||
// while the SHA printed in the header advances, so the report claims to have
|
||||
// checked today's commit using days-old file contents.
|
||||
const requestedRef = args.against || readSyncStateRef() || ghDefaultBranch(repo);
|
||||
const resolvedSha = ghResolveSha(repo, requestedRef);
|
||||
if (!resolvedSha) {
|
||||
process.stderr.write(
|
||||
`error: could not resolve "${requestedRef}" to a commit SHA in ${repo}. ` +
|
||||
`Refusing to run against an unresolvable ref (would risk false-clean results).\n`
|
||||
);
|
||||
process.exit(2);
|
||||
}
|
||||
const against = resolvedSha;
|
||||
const refSlug = pathSlug(resolvedSha);
|
||||
const checkedAgainst =
|
||||
requestedRef === resolvedSha ? shortRef(resolvedSha) : `${requestedRef} (${shortRef(resolvedSha)})`;
|
||||
|
||||
const docFiles = walk(DOCS);
|
||||
const drift = [];
|
||||
@@ -941,16 +1015,6 @@ function main() {
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve the SHA of `against` for the output header. Skip in fixture mode.
|
||||
let checkedAgainst = against;
|
||||
if (!FIXTURE_DIR) {
|
||||
const r = spawnSync("gh", ["api", `repos/${repo}/commits/${against}`, "-q", ".sha"], { encoding: "utf8" });
|
||||
if (r.status === 0) {
|
||||
const sha = r.stdout.trim().slice(0, 7);
|
||||
if (sha) checkedAgainst = `${against} (${sha})`;
|
||||
}
|
||||
}
|
||||
|
||||
if (args.json) {
|
||||
process.stdout.write(
|
||||
JSON.stringify({ checked_against: checkedAgainst, stats, drift }, null, 2) + "\n"
|
||||
|
||||
@@ -758,6 +758,78 @@ test("check-drift: env vars can verify against tree-discovered package sources",
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("check-drift: cache is keyed by resolved SHA, never a moving ref name", () => {
|
||||
// Regression guard for the stale-`master` cache bug: `--against master`
|
||||
// resolves to an immutable SHA, and the cache directory is keyed by that SHA
|
||||
// (drift-<sha>), never by the moving branch name (drift-master). Fetches also
|
||||
// use the resolved SHA — proven below by foo.ts resolving with no drift even
|
||||
// though its content fixture is keyed by the SHA, not "master".
|
||||
const { root, fixtures, scriptInRoot } = makeDriftFixture();
|
||||
const SHA = "0123456789abcdef0123456789abcdef01234567"; // 40 hex
|
||||
writeFile(root, "docs/reference/sample.md", "See cli/src/commands/foo.ts for details.\n");
|
||||
// Resolve `master` → SHA via the commit stub.
|
||||
writeFile(root, "_fixtures/commit-master.json", JSON.stringify({ sha: SHA }));
|
||||
// Content fixture keyed by the RESOLVED SHA (not "master").
|
||||
writeFixtureContents(fixtures, "cli/src/commands/foo.ts", SHA, { status: 200 });
|
||||
|
||||
const cacheDir = join(root, "_cache");
|
||||
const r = spawnSync(process.execPath, [scriptInRoot, "--json", "--against", "master"], {
|
||||
encoding: "utf8",
|
||||
env: {
|
||||
...process.env,
|
||||
PAPERCLIP_SYNC_FIXTURE_DIR: fixtures,
|
||||
PAPERCLIP_SYNC_CACHE_DIR: cacheDir,
|
||||
},
|
||||
});
|
||||
assert(r.status === 0, `expected exit 0, got ${r.status}; stderr=${r.stderr}`);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert(
|
||||
out.checked_against === `master (${SHA.slice(0, 7)})`,
|
||||
`expected checked_against "master (${SHA.slice(0, 7)})", got ${out.checked_against}`
|
||||
);
|
||||
const pp = out.drift.filter((d) => d.kind === "parent-path-missing");
|
||||
assert(pp.length === 0, `expected no parent-path drift (fetch used SHA), got ${JSON.stringify(pp)}`);
|
||||
assert(existsSync(join(cacheDir, `drift-${SHA}`)), `expected cache dir drift-${SHA}`);
|
||||
assert(
|
||||
!existsSync(join(cacheDir, "drift-master")),
|
||||
"cache must NOT be keyed by the moving ref name 'master'"
|
||||
);
|
||||
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("check-drift: ref comes from .sync-state.json when --against is absent", () => {
|
||||
// Mode-awareness: with no --against, the ref is taken from .sync-state.json
|
||||
// (nightly → last_seen_parent_sha), not the moving default branch. repo.json
|
||||
// still says the default branch is "master", so if mode-awareness were
|
||||
// ignored the fetch would use "master" and foo.ts (keyed by the state SHA)
|
||||
// would 404 into drift.
|
||||
const { root, fixtures, scriptInRoot } = makeDriftFixture();
|
||||
const SHA = "89abcdef0123456789abcdef0123456789abcdef"; // 40 hex
|
||||
writeFile(
|
||||
root,
|
||||
".sync-state.json",
|
||||
JSON.stringify({ branch_mode: "nightly", last_seen_parent_sha: SHA })
|
||||
);
|
||||
writeFile(root, "docs/reference/sample.md", "See cli/src/commands/foo.ts for details.\n");
|
||||
writeFixtureContents(fixtures, "cli/src/commands/foo.ts", SHA, { status: 200 });
|
||||
|
||||
const r = spawnSync(process.execPath, [scriptInRoot, "--json"], {
|
||||
encoding: "utf8",
|
||||
env: { ...process.env, PAPERCLIP_SYNC_FIXTURE_DIR: fixtures },
|
||||
});
|
||||
assert(r.status === 0, `expected exit 0, got ${r.status}; stderr=${r.stderr}`);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert(
|
||||
out.checked_against === SHA.slice(0, 7),
|
||||
`expected checked_against ${SHA.slice(0, 7)}, got ${out.checked_against}`
|
||||
);
|
||||
const pp = out.drift.filter((d) => d.kind === "parent-path-missing");
|
||||
assert(pp.length === 0, `expected no parent-path drift (fetch used state SHA), got ${JSON.stringify(pp)}`);
|
||||
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------------------------
|
||||
// verify-edit (unit, fixture-driven)
|
||||
// ----------------------------------------------------------------------------
|
||||
|
||||
Reference in new issue
Block a user