Skip to content

Unify Compile and publication journey - #18

Merged
raghubetina merged 4 commits into
mainfrom
codex/stream1-compile-journey
Aug 4, 2026
Merged

Unify Compile and publication journey#18
raghubetina merged 4 commits into
mainfrom
codex/stream1-compile-journey

Conversation

@raghubetina

@raghubetina raghubetina commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Stack

CLI stack 2 of 3, based on #16. The focused provenance correction follows in #19.

Service dependency: firstdraft/firstdraft#301 exposes immutable compilation.head_source_sha256 for historical download provenance. The audit job also requires the independent lockfile repair in #17 to land on main.

The service response and CLI package are a coordinated release pair because both sides validate the exact response shape. The CLI is unpublished and there are no users, so this stack intentionally adds no transitional parser for mixed versions.

Summary

  • make zero-flag plan compile the single product terminal action
  • have Compile submit the exact current whole-file Plan, wait for the analysis generation created by that accepted push, and proceed only from valid analysis
  • invoke the existing singleton internal GitHub Publication lifecycle and print one validated Project, Compilation, and Publication JSON projection on success
  • remove the public plan publish command and the old local-start plan compile --output mode without compatibility aliases
  • add retained compilation status with one-shot and bounded-wait modes
  • add retained compilation download with absent-path preflight, exact Head/artifact provenance, transport and manifest validation, and atomic materialization
  • preserve phase-specific recovery when either the Plan push or Publication mutation has an ambiguous outcome

The final Publication mutation is pinned to both the accepted ETag and exact Plan bytes so a concurrent writer cannot advance the candidate between analysis and creation.

Verification

  • PATH="/Users/sandbox2/.asdf/shims:$PATH" npm run check
    • typecheck
    • ESLint
    • Prettier
    • 147 tests
    • package allowlist
    • freshly packed executable smoke

Current limitation

A Project has one retained Publication in this release. Re-running Compile can safely replay that singleton, but cannot repoint it to a later Head. No CLI package was published and no live service or GitHub account was mutated.

Make plan compile the single terminal command for validated GitHub
publication from one explicitly authorized action.

Keep compilation status and artifact download as read-only recovery
tools with retained-Head provenance checks.
@raghubetina

Copy link
Copy Markdown
Contributor Author

The Node 24 quality failure is the same newly published transitive audit advisory isolated in #17; this stacked diff does not change the lockfile. All three Node 22 platform jobs are green. After #17 lands on main and #16 is updated, this stack should be rebased and CI rerun.

@raghubetina

Copy link
Copy Markdown
Contributor Author

Stack note update: this PR was originally described as 2 of 2. It is now the second feature chunk, followed by #19 for retained Compilation status, provenance, and download. The product Compile behavior here remains the prerequisite for that follow-on.

@raghubetina

Copy link
Copy Markdown
Contributor Author

Review

The journey unification is clean and the concurrency pinning is well designed. One finding that resolves an open question I left on firstdraft#301: the service and CLI changes are a hard flag day in both directions, not just one.

Verification

Check Result
npm run check 147 tests / 147 pass, typecheck, ESLint, Prettier, allowlist, packed smoke
plan publish removed Unknown command. with a pointer to help

Matches the description.

Neither deployment order is safe

On firstdraft#301 I flagged that the released CLI rejects the expanded compilation response, and left open whether the new CLI would tolerate a service that does not yet send head_source_sha256. It does not.

Both sides use exact set equality. Released CLI, 13 keys. This branch, 14:

const COMPILATION_KEYS = [
  "id",
  "analysis_run_id",
  "graph_version",
  "head_source_sha256",
  ...
];

gated at src/commands/compilation.js:461 through the same helper:

function hasExactKeySet(value, keys) {
  return (
    isRecord(value) && arraysEqual(Object.keys(value).sort(), [...keys].sort())
  );
}

All four combinations:

old service response new service response
released CLI (13 keys) accepts rejects
this branch (14 keys) rejects accepts

Only the matched pairs work. So:

  • deploying firstdraft#301 before releasing this CLI breaks every existing user
  • releasing this CLI before deploying firstdraft#301 breaks every user who upgrades early

There is no ordering that avoids a broken window, which makes this a flag day rather than a sequencing question. The description does name the service dependency, which is the right instinct, but "requires #301" understates it: the two must become visible to users simultaneously, or one release has to tolerate both shapes.

The cheapest fix is one transitional release where head_source_sha256 is optional on the client, accepting 13 or 14 keys, then tightening once the service has shipped. That costs one extra release and removes the window entirely.

Worth weighing against how many users exist right now. At 0.1.0-alpha.2 with no published package, the answer may be zero, in which case a flag day is genuinely fine and the thing to do is write that down rather than leave it implied.

The ETag design is the best part

The description says the final Publication mutation is pinned to both the accepted ETag and exact Plan bytes. Those turn out to be one mechanism:

const HEAD_ETAG_PATTERN = /^"sha256:([0-9a-f]{64})"$/;

The ETag is a digest of the Plan bytes, so:

headers: {
  "If-Match": etag,
},

gives optimistic concurrency and byte identity in a single conditional request. A concurrent writer who advanced the Head between analysis and creation changes the bytes, which changes the digest, which changes the ETag, and the If-Match fails. No separate "are the bytes still what I analyzed" round trip, and no window between the two checks.

That is a genuinely tidy use of a mechanism most codebases only use for caching. Worth noting in the design docs if it is not there already, because a reader who knows ETags only as a cache header will not see why this closes the race.

The same idea appears on the download path at compilation.js:442:

response.headers.get("etag") !== `"sha256:${metadata.sha256}"` ||

The artifact's ETag is checked against the digest the manifest declared, so a transport that served the wrong bytes is caught before materialization rather than after.

Recovery handling

Preserving phase-specific recovery when either the push or the Publication mutation is ambiguous is the right shape, and it matches how the service models the same states. PublicationRequestOutcomeUnknownError is raised on network and protocol errors at plan-publish.js:384, keeping "we do not know" distinct from "it was rejected."

One thing worth confirming against the service side: the ambiguous-Publication case on the CLI needs to agree with the service's publication_unknown state about what a subsequent Compile does. The service routes unknown publications to read-only reconciliation and never replays the mutation. If a CLI retry can reach the mutating path while the service is in that state, the two disagree about who is responsible for not replaying. The stated singleton replay suggests this is handled; naming the interaction in the description would make it checkable.

The stated limitation is honest

A Project has one retained Publication in this release. Re-running Compile can safely replay that singleton, but cannot repoint it to a later Head.

That is a real constraint stated plainly, including what it can and cannot do, which is more useful than a general "publication is limited." A user who compiles, edits their Plan, and compiles again will meet it, so it belongs in the README rather than only in the PR description if it is not there yet.

Removals

plan publish and the old plan compile --output local-start mode are gone with no aliases, consistent with how #16 removed plan subject-id. Fair at this version. Same caveat I raised on #16 applies: if any packaged Skill instruction still tells an agent to run plan publish, it will fail at the point the agent tries. I will check that against skills#18 when I reach it.

@raghubetina

Copy link
Copy Markdown
Contributor Author

The header you thought was for caching

You have probably seen ETag and filed it under "browser caching, not my problem." This PR uses it to close a race condition, and the trick is worth knowing because Rails gives you the same tool.

The race

Compiling a Foundation Plan takes three steps:

  1. Push the Plan to First Draft.
  2. Wait for analysis, and only proceed if it says the Plan is valid.
  3. Ask First Draft to compile and publish it.

Between step 2 and step 3 there is a gap. Analysis said "this Plan is valid," and then you make a separate request asking for it to be compiled.

Now suppose something else pushes a new Plan in that gap. Another terminal, a teammate, an agent doing work in parallel. Step 3 arrives and compiles a Plan that nothing ever validated. Analysis approved version 4; version 5 got compiled.

This is a time-of-check to time-of-use bug, and it is one of the most common shapes of concurrency bug there is. You verified something, then acted on it, and the thing changed in between. You will meet it with permission checks, balance checks, inventory checks, and status checks.

The fix that does not work

The instinct is to check again:

const current = await readPlanStatus();
if (current.etag !== validatedEtag) throw new Error("changed");
await requestCompilation();

Look closely and you have moved the gap rather than closed it. Now it sits between the re-check and requestCompilation. Smaller window, same bug. You cannot close it from the client, because two separate requests can never be atomic from the outside.

The fix that does work

The check has to happen inside the operation, on the server, as part of the same request. HTTP has a header for exactly this:

response = await sendRequest(fetchFunction, endpoint, {
  method: "PUT",
  headers: {
    Accept: "application/json, application/problem+json",
    "If-Match": etag,
  },
  ...
});

If-Match tells the server: perform this write only if the resource still matches this ETag. If it does not, reject with 412 Precondition Failed and change nothing.

Now there is no gap. The comparison and the mutation are one atomic operation on the server, which is the only place they can be atomic.

This is optimistic concurrency control. You do not lock anything. You proceed on the assumption nothing changed, and you carry enough information for the server to reject you if you were wrong. Compare pessimistic locking, where you take a lock up front and everybody else waits: optimistic is cheaper when conflicts are rare, and conflicts here are rare.

If you have used lock_version in Rails, this is the same idea over HTTP:

class Plan < ApplicationRecord
  # a lock_version column makes Rails raise
  # ActiveRecord::StaleObjectError on a conflicting update
end

Same bargain. Carry a version, let the write fail if it moved.

The part that makes this elegant

Here is where this design is better than a plain version counter:

const HEAD_ETAG_PATTERN = /^"sha256:([0-9a-f]{64})"$/;

The ETag is not a counter or a timestamp. It is a SHA-256 of the Plan's bytes.

That has a consequence worth thinking about. With a version counter, If-Match: 4 means "the Plan is still on revision 4." With a content digest, If-Match: "sha256:abc..." means "the Plan bytes are still exactly these bytes."

The second is a stronger statement, and it is stronger in a useful direction. Consider somebody pushing a change and then pushing it back:

  • Counter: revision 4, then 5, then 6 restores the original content. Your If-Match: 4 fails, even though the bytes it described are back.
  • Digest: the digest returns to its original value when the content does. Your conditional request succeeds, correctly, because the thing you validated is what is there.

You get identity rather than sequence. The description mentions being pinned "to both the accepted ETag and exact Plan bytes," and once you see that the ETag is the byte digest, those are one thing said twice.

The same idea reappears on the download path:

response.headers.get("etag") !== `"sha256:${metadata.sha256}"` ||

The artifact's ETag is checked against the digest its manifest declared. A proxy that served a stale or wrong body is caught before anything is written to disk.

Weak versus strong ETags, while we are here. An ETag prefixed with W/ is weak, meaning "semantically equivalent" rather than byte-identical, and If-Match will not accept one. These are strong, unprefixed, which is what you need when byte identity is the point.

Doing this in Rails

Rails gives you both halves:

# generating the ETag
def show
  @plan = Plan.find(params[:id])
  fresh_when etag: Digest::SHA256.hexdigest(@plan.source)
end

# enforcing the condition
def update
  @plan = Plan.find(params[:id])
  if request.headers["If-Match"] != %("sha256:#{digest_of(@plan)}")
    return head :precondition_failed
  end
  ...
end

fresh_when and stale? are usually taught for caching, which is why most Rails developers never notice that the same header solves lost updates. It is the same mechanism doing a different job.

The other thing in this PR, and it is a warning

The same file has a key check:

function hasExactKeySet(value, keys) {
  return (
    isRecord(value) && arraysEqual(Object.keys(value).sort(), [...keys].sort())
  );
}

The response must have exactly these keys. Not fewer, not more.

That client is being paired with a service change that adds one field. I checked all four combinations, and only the matched pairs work:

old service new service
old CLI accepts rejects
new CLI rejects accepts

So there is no order in which you can ship these two without a window where somebody is broken. Deploy the service first and existing users break. Release the client first and early upgraders break.

That is called a flag day, and it is the thing strict validation on both ends buys you. The escape is one transitional release where the client accepts both shapes:

  1. Client ships tolerating 13 or 14 keys.
  2. Everyone upgrades.
  3. Service starts sending the field.
  4. Client tightens to require it.

Four releases to add one field. Worth it when the strictness is load bearing, and worth knowing you have signed up for it.

The general lesson pairs with the one above. Strict checks inside a single request, like If-Match, are pure win: they close races. Strict checks across releases, like exact key sets, buy detection and cost coordination. Same instinct, very different bills.

Describe how Compile binds current local bytes to the accepted Head before its conditional Publication request. This makes the concurrency boundary reproducible from the public workflow.
Record why normalization alone cannot preserve these letters before the ASCII identifier filter. This keeps the explicit map from looking arbitrary.
@raghubetina

Copy link
Copy Markdown
Contributor Author

Addressed the documentation follow-up in 1490991: the README now states the complete Compile binding—extract the accepted source SHA-256 from the saved ETag, hash the current local Plan bytes, then send the complete ETag in If-Match. The full 147-test check, package allowlist, and packed smoke pass.

The PR description now also records the intentional coordinated service/CLI rollout. With no published compatible package or users, I am keeping the mixed-version compatibility shim rejected rather than adding transitional cruft.

Preserve the reviewed Compile journey while inheriting the landed generators and advisory repair required by every hosted check.
@raghubetina
raghubetina changed the base branch from codex/stream1-generators-init to main August 4, 2026 22:01
@raghubetina
raghubetina merged commit 2f4bfe6 into main Aug 4, 2026
8 checks passed
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