Skip to content

Clarify publication replay provenance - #15

Merged
raghubetina merged 1 commit into
mainfrom
codex/github-publication-contract
Aug 1, 2026
Merged

Clarify publication replay provenance#15
raghubetina merged 1 commit into
mainfrom
codex/github-publication-contract

Conversation

@raghubetina

Copy link
Copy Markdown
Contributor

Summary

  • pin singleton replays and polling to immutable publication provenance while still requiring local Plan bytes and ETag to match the retained Head
  • treat HTTP 408 and server-error start responses as ambiguous and reconcile through exactly one read-only GET without replaying the PUT
  • reject uncertainty-state regressions and expose both pinned and rejected projections for lifecycle mismatches
  • document the one-Publication, one-retained-Head lifetime plus cancellation, orphan-repository, and exact recovery boundaries

Contract boundary

This is a companion contract correction for firstdraft/firstdraft#283. The server route remains provisional and must serialize retained publication provenance, enforce conditional creation and safe replay, and keep uncertainty states one-way. This PR does not publish npm, create a GitHub repository, or mutate a live First Draft service.

Verification

  • PATH="/Users/sandbox2/.asdf/shims:$PATH" npm run check
  • TypeScript, ESLint, and Prettier passed
  • 132 tests passed
  • package allowlist and installed-tarball smoke passed
  • git diff --check

A singleton response describes the Head that created the Publication,
not necessarily the live Project. Document replay against retained
provenance while keeping the CLI pinned to its local candidate.

Reject recovery-state regressions so uncertainty cannot re-enter a
mutating phase. Treat timeout and server-error responses to the PUT as
ambiguous because they may follow a committed mutation, and reconcile
only through the read-only singleton GET.
@raghubetina

Copy link
Copy Markdown
Contributor Author

Technical review

Cloned and ran it: npm run check exits 0, 132 tests pass. No findings.

The state-machine change is the substance here, and it is a genuinely subtle correctness property rather than tidying.

Uncertainty is made one-way, and only where it needs to be

Progress is ranked in stages:

if (status === "compiling") return 0;
if (status === "provisioning_repository" || status === "repository_unknown") return 1;
if (status === "publishing" || status === "publication_unknown") return 2;
return 3;

then required to be monotonic. Note that each uncertain state shares a rank with its in-progress twin, so publicationStage(to) >= publicationStage(from) alone would permit repository_unknown sliding back to provisioning_repository. Hence:

if (
  (from === "repository_unknown" && to === "provisioning_repository") ||
  (from === "publication_unknown" && to === "publishing")
) {
  return false;
}

This is the right rule and the scoping is exact. repository_unknown means a repository may or may not have been created. A server that then reports provisioning_repository again is announcing a retry of an operation that may already have succeeded, and the outcome of accepting that is a second orphan repository in somebody's account. Refusing the transition turns a silent duplicate into a visible mismatch.

Equally important is what it still allows. repository_unknown to publishing crosses stages and stays legal, because that is the server resolving the uncertainty and moving on. publication_unknown to succeeded likewise. Only the backward-within-stage move is blocked, which is the one that implies redoing side effects.

An unknown state is not a pending state. Pending can become in-progress again. Unknown cannot, because the thing you do not know about may already have happened.

validRepositoryTransition carries the same idea into identity: once a repository is reported, it must keep the same id, so the server cannot swap which repository this publication refers to partway through.

408 is separated from the rest of 4xx

if (response.status === 408 || response.status >= 500) {
  throw new PublicationRequestOutcomeUnknownError(response.status, problem);
}

Most 4xx codes are a decision the server made and will make again, which is what makes failing fast correct. 408 is the exception: it says the server stopped waiting, which is silent on whether it did any work first. Grouping it with 5xx rather than with the definite rejections is the accurate reading.

Worth noting the symmetry with firstdraft#279, where I argued 401 and 403 are the other 4xx codes that are not really decisions about the request. The same distinction is being drawn here for a different code, which suggests it is a considered position rather than a one-off.

The default case is unreachable, which I checked rather than assumed

publicationStage falls through to return 3, so an unrecognized status would rank as terminal and let any transition into it pass. That is fine only because the status is allowlisted first:

const PUBLICATION_STATUSES = new Set([
  "compiling", "provisioning_repository", "repository_unknown",
  "publishing", "publication_unknown", "succeeded",
  "repository_conflict", "failed", "cancelled",
]);

with a rejection at the validation boundary before any transition logic runs. So a novel status from a server is refused rather than silently treated as terminal.

Ambiguity still resolves by reading, not repeating

The reconciliation path continues to perform exactly one GET after an ambiguous PUT and never replays the mutation, which is the property that matters for a singleton whose creation has an external side effect. The added response on both error types means a lifecycle mismatch now reports the pinned and the rejected projection together, so whoever debugs it can see which field diverged rather than being told only that something did.

No findings

@raghubetina

Copy link
Copy Markdown
Contributor Author

Lesson: "unknown" is not "pending"

Most state machines people write have states like pending, processing, done, failed. Add a remote call and you eventually need a fifth kind, and it behaves differently from all of them.

compiling
provisioning_repository        <- in progress
repository_unknown             <- we asked, and never found out
publishing
publication_unknown
succeeded / failed / cancelled

The *_unknown states are the interesting ones, and the rule this PR adds is worth internalising:

if (
  (from === "repository_unknown" && to === "provisioning_repository") ||
  (from === "publication_unknown" && to === "publishing")
) {
  return false;
}

You can move from unknown to a definite outcome. You cannot move from unknown back to in-progress.

Why the difference matters

pending means nothing has happened yet. Retrying from pending is free.

unknown means something may already have happened and you cannot see it. Retrying from unknown is a coin flip on whether you do it twice.

Here, repository_unknown means a GitHub repository may or may not have been created. If the state slides back to provisioning_repository, that is the system announcing it will try creating one again. Half the time that produces a second repository in a real person's account, with no reference to it anywhere, and nobody notices until they wonder why there are two.

So the transition is refused. A mismatch surfaces instead of a duplicate.

The Rails shape

You have written this state machine, probably with an aasm or a status column:

state :pending
state :charging
state :charged
state :failed

Then a payment gateway times out, and there is nowhere to put "we sent it and never heard back." The usual outcome is that it goes back to pending for the retry worker, and the customer gets charged twice.

The fix is a state, plus a rule about it:

state :charge_unknown

event :reconcile do
  transitions from: :charge_unknown, to: :charged
  transitions from: :charge_unknown, to: :failed
  # deliberately no transition back to :charging
end

The absence of that last transition is the whole design. It forces the recovery path to be "go ask the gateway what happened," which is the only correct move, rather than "try again," which is the tempting one.

Getting the ordering right is not enough

A detail worth noticing. This code also ranks states and requires monotonic progress:

publicationStage(to) >= publicationStage(from)

That alone does not solve it, because repository_unknown and provisioning_repository sit at the same rank. The monotonic rule permits the slide; the explicit rule forbids it.

A general invariant plus a specific exception is often the honest design. Trying to express "unknown cannot go backward" purely through rank ordering would mean giving the unknown states their own rank, which then wrongly forbids repository_unknown from advancing to publishing, a transition that is entirely legitimate when the server resolves its own uncertainty.

Two rules, each simple, beat one clever rule that gets an edge case wrong.

The check to run on your own code

Find every state that means "we do not know." If you do not have one, find the remote call that needs it.

Then ask: can this state transition to something that will redo the side effect? If yes, and the side effect is not idempotent, that path will eventually charge someone twice, send two emails, or create two repositories.

The answer is usually to delete that transition and replace it with one that reads remote state instead of writing it.

@raghubetina
raghubetina merged commit 7944bf3 into main Aug 1, 2026
4 checks passed
@raghubetina
raghubetina deleted the codex/github-publication-contract branch August 1, 2026 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant