Skip to content

Remove the recorded pre-run push outcome while preserving the push - #723

Merged
brynary merged 2 commits into
mainfrom
remove-pre-run-push-outcome
Aug 4, 2026
Merged

Remove the recorded pre-run push outcome while preserving the push#723
brynary merged 2 commits into
mainfrom
remove-pre-run-push-outcome

Conversation

@swerner

@swerner swerner commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

What this removes

PreRunPushOutcome and GitContext.push_outcome were write-only freight: the manifest builder performed a useful best-effort push before a run and then serialized a report of what happened, but no production path ever read that report — every .push_outcome read in the workspace was a test. The type expanded into a public five-variant enum, five OpenAPI union arms, six generated TypeScript models, and constructor/fixture noise across the workspace.

This PR deletes the recorded outcome everywhere: canonical Rust (fabro-types), the manifest builder, OpenAPI (GitContext.push_outcome plus the PreRunPushOutcome union and its five arm schemas), the fabro-api type replacement and re-export, the generated TypeScript client models, and all fixtures/tests that constructed or asserted it. There is deliberately no replacement status type, flag, or message.

What this preserves

  • The push itself. build_git_context still runs the best-effort push on the same path, now via a side-effect-only helper (push_manifest_branch_best_effort) with identical decision rules: no origin → skip; configured repository differs from the normalized origin → skip; branch already synced → skip; git probe errors still fall back to attempting the push; a needed push still uses remote origin, the detected branch, and GIT_TERMINAL_PROMPT=0.
  • Non-fatal failure. A failed push still cannot fail manifest creation, and the discarded error (which may contain raw Git stderr) is still not logged or serialized.
  • Useful git provenance. GitContext keeps origin_url, branch, optional sha, and dirty in canonical Rust, OpenAPI, events, manifests, and generated clients.

Test approach

Push behavior is now proven through observable repository state instead of the deleted enum: a local branch ahead of a temporary bare origin is pushed during manifest build (remote ref equals local HEAD afterward); a mismatched configured repository is not pushed (remote ref stays absent); a failing remote helper is invoked with prompting disabled while manifest creation still succeeds.

Compatibility

  • Historical manifests and run.created events carrying a nested push_outcome remain readable through ordinary serde unknown-field tolerance, pinned by one regression test at the event boundary. No migration, event rewrite, or compatibility enum.
  • This is a directional wire reduction: new servers/readers accept old JSON and ignore the removed field, but newly generated clients omit a field older servers required whenever GitContext was present, so new-client-to-old-server compatibility is intentionally not promised for this pre-1.0 contract.

Deliberately out of scope

  • Input scalar coercion (a parallel branch owns that move; the only expected overlap is the fabro-types crate-root export list)
  • The definition/intent request-contract migration
  • Local target/workspace functionality
  • Any redesign of push selection, credentials, retries, or Git error taxonomy

🤖 Generated with Claude Code

swerner and others added 2 commits August 3, 2026 19:27
The manifest builder's best-effort pre-run push converted every result
into a PreRunPushOutcome that was serialized into GitContext, expanded
into five OpenAPI union arms, and generated into API clients — but no
production path ever read it; every field read was a test.

Delete the concept while preserving the behavior:

- Drop the PreRunPushOutcome enum and GitContext.push_outcome from
  fabro-types; GitContext keeps origin_url, branch, optional sha, and
  dirty, which remain real execution inputs and provenance.
- Rename the manifest outcome builder to push_manifest_branch_best_effort,
  a side-effect-only helper with the same decision rules: skip without an
  origin, skip on configured-repository mismatch, skip when the branch is
  already synced, otherwise push noninteractively and discard the result
  without failing manifest creation or logging raw Git stderr.
- Prove the push through repository state instead of the deleted enum: a
  branch ahead of a local bare origin is pushed during manifest build, a
  mismatched configured repository is not, and a failing remote helper
  still cannot fail manifest creation.
- Remove push_outcome from GitContext in OpenAPI, delete the five-arm
  union schemas, and drop the fabro-api type replacement and re-export.
- Keep one regression proving historical run.created events with a nested
  push_outcome still deserialize through ordinary unknown-field tolerance
  and reserialize to the reduced shape. No migration or event rewrite.

Old JSON carrying the removed field stays readable. Newly generated
clients omit a field older servers required, so new-client-to-old-server
compatibility is intentionally not promised for this pre-1.0 contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regenerates the Axios client from the reduced OpenAPI spec and removes
the six stale pre-run-push-outcome model files the generator leaves
behind, along with their barrel and generator-manifest entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@brynary
brynary merged commit 6ce418a into main Aug 4, 2026
17 checks passed
@brynary
brynary deleted the remove-pre-run-push-outcome branch August 4, 2026 15:45
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.

2 participants