Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-15 | claude/db-remediation-phase-0-wfaiyl | f390d6cd5faddd8b30320f60a082374688dc1f71 | Ledger reconciliation and push guard: final current-base merge | Verified parallel-cancellation and rewritten-history handling with isolated Git reproductions; merged latest required base with no conflicts or new P0-P2 findings | git diff --check; ledger-inbox self-test; ineffective-cancellation assertion; rewritten-history guard assertion; ledger-inbox; outstanding-issues; branch-review-ledger; ledger-write-discipline |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-15 | PR #1978 | ef1bb2eac059d1c3ad0d4cae756bb069b5b8edb2 | review-and-fix | Fixed applied cancel-of-cancel acceptance and restored guard-push executable mode; verified rewritten-history and parallel-reconcile semantics; merged latest main | focused Vitest 39 passed, 1 Windows skip; guard self-tests; ledger and issue guards; format; manual adversarial review (CodeRabbit CLI unavailable) |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-15 | claude/db-remediation-phase-0-wfaiyl | becdb68610b27e7c38e9607c8bb0210911107581 | PR #1978 base sync | Merged main 6f7b7deefaf7e0cd062b748f18fc6ca8988093f6 into the reviewed PR head; merge tree was clean. | git merge-tree --write-tree exact-head main: clean; git diff --check; ledger guards. |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-15 | claude/db-remediation-phase-0-wfaiyl | 1a8f7ae02c2bd7aa22e51edfb3b4a7b249ede578 | required base sync through main 17402395 | Approved — required main update merged; prior ledger guard remediation review remains applicable with no PR-path conflict | git diff --check; ci-change-scope self-test; ledger/inbox/outstanding/discipline guards passed |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-15 | claude/db-remediation-phase-0-wfaiyl | b55f7a4b5c02c8a0ca8e59fd2d9edfb7eaac0234 | Required base sync through main d301d8f4 | approved | git diff --check; guard self-test; ledger and issue guards |
41 changes: 35 additions & 6 deletions scripts/guard-push.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -116,17 +116,46 @@ export function pushedBranchNames(ranges, fallbackBranch = "") {
return [...branches];
}

/** Exported for tests: existing branches compare from their remote tip; new
* branches compare from the PR merge base so newer main-only commits are out of
* scope for transaction guards that accept explicit base/head commits. */
export function guardBaseForRange(range, cwd = PROJECT_ROOT) {
if (range.remoteSha && range.remoteSha !== ZERO_SHA) return range.remoteSha;
/** True when `ancestor` is reachable from `descendant`, i.e. the push fast-forwards. */
function isAncestor(ancestor, descendant, cwd = PROJECT_ROOT) {
try {
execFileSync("git", ["merge-base", "--is-ancestor", ancestor, descendant], { cwd, stdio: "ignore" });
return true;
} catch {
return false;
}
}

/** Merge base with origin/main — the base a PR is actually evaluated against, and
* the same one CI passes as LEDGER_WRITE_BASE_SHA (.github/workflows/ci.yml). */
function mainMergeBase(range, cwd = PROJECT_ROOT) {
if (!tryGit(["rev-parse", "--verify", "--quiet", MAIN_REMOTE_REF], cwd)) return undefined;
return tryGit(["merge-base", MAIN_REMOTE_REF, range.localSha], cwd);
}

/** Exported for tests: a fast-forward push compares from its remote tip; a new
* branch, or one whose history was rewritten, compares from the PR merge base so
* newer main-only commits are out of scope for transaction guards that accept
* explicit base/head commits.
*
* The rewritten-history case matters: after a force-push the old remote tip is an
* abandoned line, so every request it carried reads as deleted and the ledger
* transaction guard can never pass — no matter how clean the rebuild is. Falling
* back to the merge base asks the question CI asks instead of an unanswerable one. */
export function guardBaseForRange(range, cwd = PROJECT_ROOT) {
if (range.remoteSha && range.remoteSha !== ZERO_SHA) {
if (isAncestor(range.remoteSha, range.localSha, cwd)) return range.remoteSha;
return mainMergeBase(range, cwd);
}
return mainMergeBase(range, cwd);
}

export function changedFilesForRange(range, cwd = PROJECT_ROOT) {
const existingRemote = range.remoteSha && range.remoteSha !== ZERO_SHA;
// A rewritten history is treated like a new branch here for the same reason as
// guardBaseForRange: `<abandoned tip>..<local>` is not the set of files this push
// actually introduces relative to main.
const existingRemote =
range.remoteSha && range.remoteSha !== ZERO_SHA && isAncestor(range.remoteSha, range.localSha, cwd);
const hasOriginMain = !existingRemote && tryGit(["rev-parse", "--verify", "--quiet", MAIN_REMOTE_REF], cwd);
const spec = existingRemote
? `${range.remoteSha}..${range.localSha}`
Expand Down
52 changes: 50 additions & 2 deletions scripts/ledger-inbox.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -99,12 +99,43 @@ function mutationConflicts(requests) {
return [...byIssue.entries()].filter(([, requestIds]) => requestIds.length > 1);
}

/**
* Ids of requests that a previous reconciliation already applied. Used to tell a
* cancellation that lost a race (its target landed via another branch's batch)
* apart from one that names a request which never existed.
*/
function appliedRequestsById() {
try {
return new Map(
loadRequestsIn(APPLIED_DIR)
.map((entry) => entry.request)
.filter((request) => request?.id)
.map((request) => [request.id, request]),
);
} catch {
return new Map();
}
}

/**
* Resolve immutable cancellation decisions before applying a pending request batch.
* A cancellation request is itself retained in the applied audit trail, while the
* targeted request is moved unchanged but deliberately not applied to the ledger.
*
* Parallel reconciliations are normal here: several branches queue requests and one
* of them reconciles first. A cancellation whose target was applied by that earlier
* batch has therefore *failed* — but throwing would wedge every consumer of this
* function (check:docs-links, check:ledger-write-discipline and reconcile itself)
* with no legal way out, because write discipline forbids deleting the queued file.
* So an already-applied target is reported loudly and skipped rather than fatal; a
* genuinely unknown target still throws.
*
* @param {Array<object>} requests pending requests in the batch
* @param {{ appliedRequests?: Map<string, object>, warn?: (message: string) => void }} [options]
*/
export function planRequestBatch(requests) {
export function planRequestBatch(requests, options = {}) {
const applied = options.appliedRequests ?? appliedRequestsById();
const warn = options.warn ?? ((message) => console.warn(message));
const byId = new Map();
for (const request of requests) {
const problems = validateRequest(request);
Expand All@@ -115,10 +146,27 @@ export function planRequestBatch(requests) {

const cancelledIds = new Set();
const cancellations = [];
const ineffective = [];
for (const request of requests) {
if (request.action !== "cancel") continue;
const target = byId.get(request.payload.requestId);
if (!target) {
const appliedTarget = applied.get(request.payload.requestId);
if (appliedTarget) {
if (appliedTarget.action === "cancel") {
throw new Error(`cancel request ${request.id} cannot cancel another cancellation request`);
}
// Lost the race. Say so plainly: the correction this cancellation was
// protecting did NOT take effect, and whoever queued it needs to fix the
// row with a fresh update rather than assume the cancel did its job.
ineffective.push({ requestId: request.id, targetId: request.payload.requestId });
warn(
`ledger inbox: cancel request ${request.id} did not take effect — its target ` +
`${request.payload.requestId} was already applied by an earlier reconciliation. ` +
`The cancellation is recorded but changed nothing; correct the affected row with a new update request.`,
);
continue;
}
throw new Error(`cancel request ${request.id} targets missing pending request ${request.payload.requestId}`);
}
if (target.action === "cancel") {
Expand All@@ -145,7 +193,7 @@ export function planRequestBatch(requests) {
);
}

return { active, cancellations, cancelledIds: [...cancelledIds] };
return { active, cancellations, cancelledIds: [...cancelledIds], ineffectiveCancellations: ineffective };
}

export function applyRequestBatch(markdown, requests) {
Expand Down
43 changes: 43 additions & 0 deletions tests/guard-push.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -154,6 +154,49 @@ describe("push-range parsing", () => {
expect(parsePushRanges("\n \n")).toHaveLength(0);
});

it("compares a fast-forward push from its remote tip", () => {
const { root, git } = gitFixture();
git("update-ref", "refs/remotes/origin/main", "HEAD");
git("switch", "--quiet", "-c", "feature");
writeFileSync(join(root, "one.md"), "one\n");
git("add", "one.md");
git("commit", "--quiet", "-m", "one");
const remoteSha = git("rev-parse", "HEAD");
writeFileSync(join(root, "two.md"), "two\n");
git("add", "two.md");
git("commit", "--quiet", "-m", "two");
const localSha = git("rev-parse", "HEAD");

// Ordinary push: the remote tip is reachable, so it stays the base and only
// the newly pushed commit is in scope.
expect(guardBaseForRange({ localSha, remoteSha }, root)).toBe(remoteSha);
expect(changedFilesForRange({ localSha, remoteSha }, root)).toEqual(["two.md"]);
});

// A force-push abandons the old remote tip. Comparing against it makes every
// file the discarded history carried look deleted, which is unanswerable for
// transaction guards; the merge base is the question CI actually asks.
it("falls back to the merge base when the remote tip was discarded by a force-push", () => {
const { root, git, baseSha } = gitFixture();
git("update-ref", "refs/remotes/origin/main", "HEAD");
git("switch", "--quiet", "-c", "feature");
writeFileSync(join(root, "abandoned.md"), "abandoned\n");
git("add", "abandoned.md");
git("commit", "--quiet", "-m", "abandoned");
const discardedSha = git("rev-parse", "HEAD");

git("reset", "--quiet", "--hard", baseSha);
writeFileSync(join(root, "rebuilt.md"), "rebuilt\n");
git("add", "rebuilt.md");
git("commit", "--quiet", "-m", "rebuilt");
const localSha = git("rev-parse", "HEAD");

expect(discardedSha).not.toBe(localSha);
expect(guardBaseForRange({ localSha, remoteSha: discardedSha }, root)).toBe(baseSha);
// abandoned.md must not read as a deletion introduced by this push.
expect(changedFilesForRange({ localSha, remoteSha: discardedSha }, root)).toEqual(["rebuilt.md"]);
});

it("keeps a Windows new-branch static command scoped to the PR side of an advanced main", () => {
const { root, git, baseSha } = gitFixture();
git("switch", "--quiet", "-c", "feature");
Expand Down
113 changes: 113 additions & 0 deletions tests/ledger-inbox-cancellation.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
import { describe, expect, it } from "vitest";

import { planRequestBatch } from "../scripts/ledger-inbox.mjs";

const UPDATE_ID = "11111111-1111-4111-8111-111111111111";
const CANCEL_ID = "22222222-2222-4222-8222-222222222222";
const APPLIED_ID = "33333333-3333-4333-8333-333333333333";
const UNKNOWN_ID = "44444444-4444-4444-8444-444444444444";

function update(id: string, issueId = "#316") {
return {
version: 1,
id,
createdOn: "2026-08-14",
action: "update",
payload: { id: issueId, detail: `detail for ${issueId}` },
};
}

function cancel(id: string, requestId: string) {
return {
version: 1,
id,
createdOn: "2026-08-14",
action: "cancel",
payload: { requestId, reason: "superseded" },
};
}

// planRequestBatch reads the applied directory itself by default; every test here
// injects the requests so the assertions do not depend on the repo's real inbox.
function plan(requests: object[], appliedRequests: ReturnType<typeof update | typeof cancel>[] = []) {
const warnings: string[] = [];
const result = planRequestBatch(requests, {
appliedRequests: new Map(appliedRequests.map((request) => [request.id, request])),
warn: (message: string) => warnings.push(message),
});
return { ...result, warnings };
}

describe("ledger inbox cancellation planning", () => {
it("cancels a pending request in the same batch", () => {
const result = plan([update(UPDATE_ID), cancel(CANCEL_ID, UPDATE_ID)]);

expect(result.active).toHaveLength(0);
expect(result.cancelledIds).toEqual([UPDATE_ID]);
expect(result.cancellations).toEqual([{ requestId: CANCEL_ID, targetId: UPDATE_ID, reason: "superseded" }]);
expect(result.warnings).toHaveLength(0);
expect(result.ineffectiveCancellations).toHaveLength(0);
});

// The race this exists for: another branch reconciled first and applied the
// target, so this cancellation can no longer take effect. Throwing here wedges
// check:docs-links, check:ledger-write-discipline and reconcile at once, with no
// legal way out because write discipline forbids deleting the queued request.
it("does not throw when the cancelled target was already applied elsewhere", () => {
const result = plan([update(UPDATE_ID), cancel(CANCEL_ID, APPLIED_ID)], [update(APPLIED_ID)]);

expect(result.active).toHaveLength(1);
expect(result.cancelledIds).toHaveLength(0);
expect(result.ineffectiveCancellations).toEqual([{ requestId: CANCEL_ID, targetId: APPLIED_ID }]);
});

it("says loudly that an already-applied cancellation changed nothing", () => {
const result = plan([cancel(CANCEL_ID, APPLIED_ID)], [update(APPLIED_ID)]);

expect(result.warnings).toHaveLength(1);
expect(result.warnings[0]).toContain(CANCEL_ID);
expect(result.warnings[0]).toContain(APPLIED_ID);
expect(result.warnings[0]).toContain("did not take effect");
// The operator must be told to fix the row another way, not left assuming the
// cancellation did its job.
expect(result.warnings[0]).toContain("new update request");
});

it("still throws when the target never existed", () => {
expect(() => plan([cancel(CANCEL_ID, UNKNOWN_ID)], [update(APPLIED_ID)])).toThrow(
/targets missing pending request/,
);
});

it("still refuses to cancel another cancellation", () => {
expect(() => plan([update(UPDATE_ID), cancel(CANCEL_ID, UPDATE_ID), cancel(APPLIED_ID, CANCEL_ID)])).toThrow(
/cannot cancel another cancellation/,
);
});

it("still refuses to cancel a cancellation that was already applied", () => {
expect(() => plan([cancel(CANCEL_ID, APPLIED_ID)], [cancel(APPLIED_ID, UPDATE_ID)])).toThrow(
/cannot cancel another cancellation/,
);
});

it("still refuses to cancel the same request twice", () => {
expect(() => plan([update(UPDATE_ID), cancel(CANCEL_ID, UPDATE_ID), cancel(APPLIED_ID, UPDATE_ID)])).toThrow(
/cancelled more than once/,
);
});

it("keeps requiring a decision when two mutations target one row", () => {
expect(() => plan([update(UPDATE_ID), update(CANCEL_ID)])).toThrow(
/multiple pending mutations require an explicit cancellation decision/,
);
});

it("an ineffective cancellation does not resolve a competing-mutation conflict", () => {
// The cancellation lost its race, so both updates are still active and the
// conflict must still be raised rather than silently half-applied.
expect(() =>
plan([update(UPDATE_ID), update(CANCEL_ID), cancel(UNKNOWN_ID, APPLIED_ID)], [update(APPLIED_ID)]),
).toThrow(/multiple pending mutations require an explicit cancellation decision/);
});
});
Loading