mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-09 18:08:31 -10:00
d065467cd3
* feat(zerogit): auto-create a conventional branch before push/pr on default branch
zero changes push/pr refused to push straight to the default branch but
never offered an alternative, unlike other agent tooling that names and
checks out a feature branch automatically. Add CreateBranch, IsDefaultBranch,
CurrentGitUser, and branch-name slugify/build helpers to zerogit, and wire
push/pr to auto-create a "<user>/<slug>" branch (slug from an LLM when a
provider is configured, otherwise a deterministic fallback from the diff)
whenever the current branch is the default one and --yes/--dry-run weren't
passed.
* fix(zerogit): normalize LLM branch slugs, bound remote lookup, handle existing branches
Address CodeRabbit's review on the auto-branch-naming PR:
- generateAutoBranchSlug now takes the first non-empty, quote-trimmed
line of the model's response before slugifying, instead of folding
any preamble or wrapping quotes into the branch name.
- isDefaultBranch's ls-remote lookup is now bounded by a 5s timeout so
a slow or unreachable remote can't stall push/pr; falls back to the
local main/master heuristic as before.
- CreateBranch checks whether the target branch already exists locally
and checks it out instead of failing checkout -b on the collision.
Also add regression coverage: CurrentGitUser's OS-username fallback
tier, CreateBranch's existing-branch and generic checkout-failure
paths, a CLI-level test for changes pr (previously untested) mirroring
the existing changes push coverage, and a messy-LLM-response case for
the slug normalization fix.
* fix(zerogit,cli): make auto-branching collision-safe, remote-aware, fail-closed, and opt-in for LLM naming
- CreateBranch never checks out an existing branch on a name collision: it
picks a unique suffixed name off the current HEAD (name-2 .. name-9) and
fails visibly when the namespace is exhausted, so a stale branch with
unrelated history can no longer be published while the new commit stays
behind on the default branch.
- ensureFeatureBranch resolves the remote once (explicit --remote, then the
original branch's configured upstream, then origin) via IsDefaultBranch,
which now reports the resolved remote, and push/pr thread that remote into
Push, so a freshly created branch without tracking configuration no longer
falls back to origin in fork setups.
- The default-branch check fails closed: main and master count as default
with no network, an unreachable remote falls back to the local
refs/remotes/<remote>/HEAD record, and when neither answers the push is
refused with guidance instead of silently dropping the confirmation guard
for repositories whose default is trunk or develop.
- LLM branch naming is now opt-in via --auto on push/pr, matching commit
--auto: a configured provider alone no longer causes the change diff to be
uploaded during ordinary git-only pushes.
- After the ordinary commit-then-push sequence the tree is clean, so the
fallback name comes from the HEAD commit subject instead of the constant
user/changes.
* style(zerogit): align fake-runner result comments to gofmt output
* test(zerogit): cover Push's fail-closed path when the default branch is unknown
* fix(zerogit): keep a stale cached remote HEAD from clearing the push guard
When the live ls-remote default-branch lookup fails, isDefaultBranch fell
back to the local refs/remotes/<remote>/HEAD cache and returned
branch == cachedName. That cache is only a hint: if the server renamed its
default (main -> trunk) the record still names main, so checking whether
pushing trunk is safe returned false and the newly protected branch was
pushed without --yes.
Trust the cache only to block a push (a positive match proves the branch is
the recorded default); on a mismatch fall through to the existing
fail-closed unknown-default error instead of treating the branch as
unprotected.
* fix(cli): honor --diff-bytes when inspecting the diff for auto-branch naming
changes push/pr accept and document --diff-bytes, but ensureFeatureBranch
inspected the working tree with only Cwd set. With --auto the resulting
unbounded diff was embedded in the branch-name request to the provider, so a
user who capped the diff to limit how much proprietary source is uploaded
for naming still sent the complete diff.
Thread options.maxDiffBytes through ensureFeatureBranch into InspectOptions,
matching how the commit path already bounds the diff.
* fix(cli): refuse an auto-branch push when HEAD has nothing to publish
The auto-branch path decided to branch solely from working-tree status and
never checked that HEAD was ahead of the default branch. On a clean,
up-to-date default branch it would create and push user/<subject> at the
exact default tip; with only uncommitted edits it named a branch from those
edits but pushed the unchanged HEAD, leaving the edits local, and changes pr
then abandoned the empty branch before the host rejected the comparison.
Add zerogit.CommitsAhead and consult it before branching: when HEAD is
provably not ahead of <remote>/<default>, report "no changes to publish"
instead of creating the branch. An indeterminate count (for example a
remote-tracking ref that was never fetched) is treated as "cannot tell" so a
legitimate first push is not blocked.
* fix(cli): recover the branch slug from preamble or fenced LLM replies
The slug helper returned the first non-empty line verbatim, so a reply like
"Here is a suggested branch name:\nadd-login-page" produced
user/here-is-a-suggested-branch-name, and a reply wrapped in a code fence
slugified the ``` line to nothing and silently fell back to the local name.
Drop Markdown code-fence lines and prefer a line that already reads as a
kebab-case slug, falling back to the first quoted/plain line only when no
slug-shaped line exists. Cover preamble and fenced responses with tests.
* fix(zerogit): harden auto-branch remote handling
Three review findings:
- isDefaultBranch passed the remote to ls-remote before the HEAD
positional with no --, so a remote value shaped like an option
(--upload-pack=/bin/echo) was parsed as one. Terminate options with --
and cover a dash-prefixed remote.
- An unborn remote (freshly created, zero refs) answered ls-remote with
empty output, fell through to the fail-closed unknown-default error,
and made the very first feature-branch push a --yes dead end that
git remote set-head --auto cannot repair. ls-remote succeeding with no
refs now counts as proof there is no protected default; main/master
stay guarded by the name heuristic and every failure path stays
fail-closed.
- CreateBranch probed only local refs/heads for collisions, so a branch
existing only on the target remote (an old merged-PR branch) was
silently fast-forwarded by the later push -u. The target remote's
heads are now probed once (bounded, with --) and count as taken;
an unreachable remote fails visibly before anything is created.
Co-Authored-By: Claude Fable 5 <[email protected]>
* fix(zerogit): close branch-naming and push safety gaps from review
Require a confirmed unborn HEAD, not just an empty ls-remote --symref,
before granting the first-push exception, since a dangling remote
HEAD looked the same. Resolve branch naming and ahead-count checks
against the remote branch instead of a working-tree snapshot, which
could describe uncommitted edits a commit-only push would never
publish. Guard concurrent branch creation with a force-with-lease
push. Scope the ls-remote timeout to the default-branch preflight
only, so it no longer caps Push's own unrelated check.
* fix(cli): require a clean tree before auto-branch push/pr
Refuse ensureFeatureBranch when the working tree is dirty or the
ahead count against the remote default cannot be determined, so a
default-branch push cannot leave uncommitted edits behind or publish
an empty comparison under a guessed name.
* fix(zerogit): drop fixed five-second remote lookup timeout
IsDefaultBranch and CreateBranch applied a hard 5s bound on ls-remote,
so ordinary feature-branch pushes and the collision probe failed on slow
but reachable remotes (SSH/VPN) before Push could run. Honor the
caller's context instead; callers that need a bound pass a deadline.
* fix(cli): bypass the ahead-count and diff-base checks on a confirmed-unborn remote
jatmn flagged that the confirmed-unborn-remote exception this PR added to
IsDefaultBranch never actually helps the main/master case it was written
for: when the local branch is named main, isDefaultBranch returns true from
the name heuristic before it ever consults the remote, so
ensureFeatureBranch still unconditionally calls commitsAhead against
<remote>/<branch>. On a genuinely empty remote that ref cannot exist, so
`zero changes push`/`pr` from a local default branch against a brand-new
empty remote still dead-ended with "cannot determine whether HEAD is
ahead of origin/main" - the exact bug this PR set out to fix.
Tracing the flow further, the same problem also hit the diff-base Inspect
call right after: it uses the same nonexistent <remote>/<branch> ref, so
bypassing only commitsAhead would have left the dead end one line later.
ensureFeatureBranch now probes the new zerogit.IsUnbornRemote (ls-remote
--heads on the remote) whenever commitsAhead fails, and only bypasses both
checks when that probe positively confirms the remote has no refs at all.
An unreachable remote, or one that answers with refs, still fails closed
exactly as before - the fail-closed "unknown default branch" behavior is
unchanged and covered by TestEnsureFeatureBranchFailsWhenAheadCountUnknown
and the new TestEnsureFeatureBranchFailsWhenUnbornCheckErrors.
TestEnsureFeatureBranchCreatesBranchOnConfirmedUnbornRemote is the
regression test: confirmed failing (with the exact dead-end error) against
the pre-fix logic, passing after.
* fix(zerogit): resolve main/master against the live remote default first
isDefaultBranch short-circuited on the literal branch names main/master
before ever consulting the remote. A repository whose remote default is
genuinely something else (e.g. trunk) could still have a local main branch
tracking that remote, and the shortcut wrongly treated it as protected
instead of trusting the live symref result. The conventional-name check
now only applies as a last-resort fallback, after both the live ls-remote
symref lookup and the local remotes/<remote>/HEAD cache fail to answer.
* fix(cli): preserve push lease on retry and check the live default ref
Two related gaps in ensureFeatureBranch's auto-branch preflight:
- When an auto-branch push loses a force-with-lease race against a
concurrent creator, the generated branch is left checked out locally
with no successful push behind it. A retry took the "already off the
default branch" early return and dropped RequireNewRemoteBranch,
letting the next push silently fast-forward whatever the concurrent
creator published. The lease now stays required unless the branch
already has a configured upstream (proof a push already landed).
- The ahead-of-default check compared HEAD against the local
remote-tracking ref, which is only refreshed at clone or the last
fetch. A stale ref can under-report how far ahead HEAD is, or say
zero when the remote has actually moved past it, so ensureFeatureBranch
now refreshes that tracking ref before trusting it, and fails closed
if the refresh itself cannot be confirmed.
Adds regression coverage for both: a retry after a lease collision keeps
requiring the lease (and an already-published branch does not), and a
stale tracking ref is refreshed before the ahead check runs (with a
fail-closed case when that refresh cannot be confirmed).
* fix(cli): track generated branches, reject PR on unborn remote, classify main on unborn
* fix(zerogit,cli): preserve explicit nonexistence lease on force, propagate marker write errors, check unborn remote before branch switch
* fix(zerogit): preserve non-existence lease on target remote and roll back failed branch marking
Compare target remote against configured upstream remote to retain non-existence lease on target, roll back created branch if marking fails, and allow --yes to bypass PR default-branch preflight.
Refs #671
* fix(cli): address review findings for auto feature branch creation
* fix(cli): refine LLM preamble candidate slug extraction and enforce unborn-remote guard
* fix(cli): restore default branch after auto-branch and resolve --yes upstream
Do not leave auto-branch commits on the protected local default branch,
and resolve the real upstream for the unborn-remote guard under --yes
instead of always probing origin.
* test(cli): assert auto-branch passes capped diff into the provider
Thread --auto and a mock provider through DiffBytes coverage so the
cap is checked on the real LLM request path, not only Inspect.
* fix(cli): block auto-branch on unborn remotes and fix lease/restore
Refuse auto-creating a feature branch when the target remote has no refs yet.
Publishing only user/slug leaves remote HEAD dangling, so the next changes pr
fails closed and a later main push has no PR diff. Require push --yes first.
Keep the nonexistence lease unless branch@{upstream} is exactly the target
remote/generated-branch ref from a successful push -u. Inherited origin/main
from branch.autoSetupMerge=inherit must not drop the lease.
Restore the original default branch against its own upstream tip, not the
push destination, so --remote upstream does not rewrite a fork main that
tracks origin/main.
* fix(cli): recover push -u upstream and filter LLM slug preambles
Push -u can publish a remote branch then fail to write local upstream
config. Treat that as incomplete, recover with set-upstream-to, and probe
the remote before reasserting the nonexistence lease on retry. Also skip
one-word LLM acknowledgements before accepting a slug-shaped line.
Refs #671
* fix(cli): decide the branch-collision lease with one live remote check
Replace the local-state inference chain (upstream config, a
zeroAutoBranch marker, per-remote provenance tracking) that kept
producing new collision-safety edge cases across many review rounds
with a single live remoteHasBranch check against the destination
remote: missing there gets the nonexistence lease, already there gets
a plain push, and git's own fast-forward check is the safety net if
that existing branch isn't actually ours.
Removes MarkGeneratedBranch/IsGeneratedBranch, now dead, along with
the tests that pinned the specific local-state heuristics rather than
the collision-safety property itself.
* fix(cli,zerogit): address CodeRabbit findings on auto-branch naming
Recover trailing-punctuation LLM slugs, notice deterministic fallback,
report dual restore/rollback failures, and skip Push upstream repair on
dry-run. Align comments with fail-closed and DryRun behavior.
Refs #671
* fix(tools): set explicit stdin reader and use resolved binary for format-on-write
Use the binary path resolved by exec.LookPath and set formatter.Stdin
to an empty reader so format-on-write does not hang on inherited stdin
handles on Windows.
Refs #671
* fix(cli,zerogit,tools): address CodeRabbit review findings on PR 671
Remove redundant command validation, eliminate duplicate remote lookup in PR
workflow, harden ref options and branch switching, isolate test Git
environment, and add formatter lookup test.
Refs #671
* fix(cli,zerogit): address new CodeRabbit review findings on PR 671
Harden CreateBranch name validation, remove unreachable workflow error check,
complete doc comment, isolate test Git environment, and clean up unused
appDeps field.
Refs #671
* fix(cli,zerogit,tools): address third round of CodeRabbit review findings on PR 671
Use featureBranchOptions struct for ensureFeatureBranch, bound preflight
contexts, add upstream repair mismatch subtest, and centralize test Git
config isolation.
Refs #671
* fix(cli,zerogit): finish ensureFeatureBranch doc contract and close review gaps
The ensureFeatureBranch doc comment ended mid-sentence and sat on the featureBranchOptions type, so go doc attached function prose to the struct. Move it above the function and state which deps are required non-nil (inspectChanges, commitsAhead, headCommitSubject, currentGitUser, createBranch) versus optional and nil-guarded (refreshTrackingRef, isUnbornRemote, branchUpstreamRef, resetBranchRef, deleteBranch). Also extract the branch slug filename with path.Base because git paths are always slash-separated, gate two tests on the git version their fixtures need (autoSetupMerge=inherit requires 2.35, init --bare -b requires 2.28), and cover the literal "user" CurrentGitUser fallback via a test seam.
Refs #671
Co-Authored-By: cairn-code <[email protected]>
* Protect auto-branch publication and default ref restoration
Co-Authored-By: cairn-code <[email protected]>
* fix(cli): wire expected-old restore through the dependency seam
The P1 fix added expectedOld to resetBranchRef but left the appDeps field and the workflows.go import behind, so the CLI package did not compile. Update the field type to the 5-argument signature and restore the filepath import used by worktree release resolution.
Co-Authored-By: cairn-code <[email protected]>
* fix(cli,zerogit): wire CAS tip and close review test gaps
Addresses the CodeRabbit round on 93e3448e: fillAppDeps now defaults currentBranchTip so the compare-and-swap old value actually reaches ResetBranchRef in production; TestEnsureFeatureBranchRestoresDefaultBranchAfterCreate asserts the CAS value; the real-git auto-branch test asserts local main is restored to its upstream tip; docs explain the deliberate context.Background push, CurrentBranchTip semantics, the RemoteHasBranch tip-equality ownership check, and the Git 2.24 requirement for CommitsAhead; CreateBranch gains name-validation subtests.
Co-Authored-By: cairn-code <[email protected]>
* fix(zerogit): make RemoteHasBranch report existence only, not tip equality
RemoteHasBranch's tip-equality check made ensureFeatureBranch reassert the
nonexistence lease (--force-with-lease=<branch>:) on every push after the
first: a second local commit puts HEAD ahead of the just-published remote
tip, so the check reported false for a branch that plainly exists, and Git
rejected the follow-up push with a stale-info error instead of a normal
fast-forward.
The concurrent-creator race the tip check was meant to guard was already
handled one layer up: ensureFeatureBranch drops to a plain push when the
branch exists and relies on Git's own fast-forward check as the safety net
(see TestRunChangesPushDropsLeaseWhenBranchExistsOnRemote), so a race
survivor with unrelated history is rejected there regardless of tip
equality here.
Add TestRemoteHasBranchReportsTrueForDivergedTip, which fails under the old
tip-equality check and passes now.
Refs #671
* fix(cli,zerogit): preserve the generated branch on a compare-and-swap conflict
After auto-branching, ensureFeatureBranch restores the default branch to its
own upstream tip using the compare-and-swap form of update-ref. When that
refused because the default branch had moved concurrently, the failure was
indistinguishable from an ordinary update-ref write error, so the rollback
path ran and deleted the freshly created feature branch. That branch
exclusively owns the user's commits at that point, so a lost race destroyed
work in order to "recover" from it.
Add ErrCompareAndSwapConflict, returned by ResetBranchRef only when an
expected-old value was supplied (the sole way git refuses is a tip mismatch).
ensureFeatureBranch now recognizes it and returns without rolling back,
reporting that the branch was preserved so the user can reconcile the default
branch by hand.
Extend TestResetBranchRefRefusesConcurrentDefaultAdvance, which already drove
a real concurrent advance, to require the sentinel. Add
TestEnsureFeatureBranchPreservesBranchOnCompareAndSwapConflict, which fails
with "must not delete the generated branch" when the new branch is removed.
Refs #671
* Address review feedback for auto-branch workflow.
Refresh the exact configured upstream ref before branching and restoring, guard direct URL and path remote destinations from tracking-ref assertion, and remove undeclared --end-of-options from CommitsAhead.
Refs #671
* fix(zerogit,cli): resolve reviewer feedback on branch restoration and remote handling
Update CommitsAhead and ResetBranchRef to safely terminate option parsing with --, inspect all linked worktrees before ref updates, resolve advertised branches on direct path and URL remotes, and query git configuration directly for tracking remote and merge branch without splitting on slashes.
Refs #671
* fix(zerogit): invoke rev-list without positional delimiter to treat revs correctly
Remove premature option terminator from rev-list command invocation while validating base ref against option-like prefixes.
Refs #671
* fix(cli,zerogit): resolve restore tip commit and enforce push remote unborn policy
Resolve restore tip commits via ResolveRemoteBranchTip for named and direct URL/path upstreams to eliminate bare branch fallback, enforce unborn destination policy on push remotes before branch creation, and extract resolveBranchPushPlan to isolate decision logic from repository mutation.
Refs #671
* fix(cli,zerogit): address review feedback on destination remote and upstream matching
Forward resolved pushRemote to branchPushPlan, update HasUpstream to support multi-segment remote upstream references, clarify IsNamedRemote comment, and extend CLI argument parsing tests.
Refs #671
* docs(cli): complete ensureFeatureBranch documentation and wire branchUpstreamRemoteAndMerge
Refs #671
---------
Co-authored-by: Claude Fable 5 <[email protected]>
Co-authored-by: euxaristia <[email protected]>
Co-authored-by: cairn-code <[email protected]>