Repository navigation
fix: harden refspec lifecycle, brief parser, status steering, branch-mode docs - #23
Merged
Merged
Conversation
…mode docs Review follow-ups for #16-#22: - reconcile: never touch foreign fetch refspecs. The exact-refspec match also captured a user-configured wildcard (+refs/heads/*:...) converting the pool to full fetch, parsed its branch as '*', found no such remote branch, and silently removed it. Git refnames forbid '*', so any glob entry is foreign config — now skipped in both phases. - switch: roll back the just-registered fetch refspec when the remote branch turns out not to exist. The leftover violated the invariant "every exact refspec points at a real remote branch" and broke every bare fetch in the pool until the next reconcile touchpoint. - remove_fetch_refspec: prefer atomic `git config --unset --fixed-value` (git >= 2.26) over unset-all + re-add rebuild, which loses every fetch refspec if interrupted mid-rebuild. Older git keeps the rebuild path (floor is git 2.20). - brief extract: recover from an unclosed HTML block after a 50-line budget instead of letting it swallow the rest of the README; typed fences — a ~~~ line inside a ``` fence is content, not a toggle. - status: print the raw-mode conversion steering on stderr, closing the gap with context/done per the spec-warnings steering registry. - docs: scoped is the default branch mode everywhere — SKILL.md, CONSTRAINTS.md (both sections), USAGE.md, spec-commands, recipes, and comparison still said "default = raw" / "scoped opt-in", contradicting the workflow section since #18. Tests: 220/220 (wildcard preservation, failed-switch rollback, unclosed HTML recovery, typed fences, status steering). Lint clean. Signed-off-by: zheng-weihao <zheng-weihao@outlook.com>
zheng-weihao
force-pushed
the
review-fixes
branch
from
July 28, 2026 07:48
1bff257 to
edefd19
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review follow-ups for #16–#22. Six findings, one branch; each fix below names the verified problem.
Refspec lifecycle (one area, three fixes)
Reconcile no longer deletes foreign fetch refspecs. The exact-refspec match also captured a user-configured wildcard
+refs/heads/*:refs/remotes/origin/*(the documented way to convert a pool to full fetch), parsed its branch as*, found no such remote branch, and removed it — silently reverting the pool to single-branch semantics. Git refnames forbid*, so any glob entry is config orbit never wrote; both reconcile phases now skip it. Regression test: wildcard survivesorbit syncwhile a stale exact entry is removed.Failed
orbit switchrolls back the refspec it just registered.orbit_ensure_remote_branchadded the refspec before the verifying fetch; a typo'd/deleted branch then failed with the refspec left behind — violating the invariant "every exact refspec points at a real remote branch" and breaking every baregit fetchin the pool until the next reconcile touchpoint. Now rolled back before failing. Regression test: failed switch leaves pool config clean and bare fetch working.orbit_remove_fetch_refspecis atomic on git ≥ 2.26. The unset-all + re-add rebuild loses every fetch refspec if interrupted between the two steps. Now prefersgit config --unset --fixed-value; older git (floor is 2.20) keeps the rebuild fallback.Brief parser (#20 robustness)
<p align="center">without</p>, common in hand-written READMEs) no longer swallows the rest of the file: after a 50-line budget the stack is dropped and normal scanning resumes. Fences are typed: a~~~line inside a```block is content, not a toggle. Two regression tests.Agent steering
orbit statusprints the raw-mode conversion steering on stderr (orbit switch -c <name>), closing the gap with context/done per the spec-warnings steering registry; the stdout table keeps only therawmark. Regression test included.Branch-mode docs made consistent: scoped is the default.
SKILL.md,CONSTRAINTS.md(two sections),USAGE.md(sections reordered),spec-commands.md,recipes.md, andcomparison.mdstill said "default = raw" / "scoped opt-in", contradicting the workflow section since feat: recommend scoped mode over raw mode for branch management #18 — agents loading the skill received two conflicting instructions. (The opencode plugin's bundled skill copy is generated fromskills/orbit/SKILL.mdbyprepublishOnlyand gitignored in the working tree — single source already enforced.)Verification
make lintclean.<orbit-context>hint comment are already declared in spec-commands.md §context / spec-hooks.md; smaller nits deferred.Signed-off-by: zheng-weihao zheng-weihao@outlook.com