feat: --from <ref> cuts a new branch from a base you name - #649
Open
JSmithRobotics wants to merge 1 commit into
Open
JSmithRobotics wants to merge 1 commit into
JSmithRobotics wants to merge 1 commit into
Conversation
Reviewer's GuideAdds a per-launch Sequence diagram for cutting a branch from a named refsequenceDiagram
participant User
participant CLI
participant Launch
participant CloneManager
participant Remote
participant Git
User->>CLI: dl owner/repo@branch --from base
CLI->>Launch: from_ref(Some(base))
Launch->>CloneManager: prepare_cold_from(..., branch, base, ...)
CloneManager->>Remote: fetch_ref(branch)
Remote-->>CloneManager: branch missing
CloneManager->>Remote: fetch_ref(base)
Remote-->>CloneManager: Updated
CloneManager->>Git: ensure_branch_exists(branch, base)
Git-->>CloneManager: branch created
CloneManager-->>Launch: PreparedWorkspace
State diagram for --from branch-cut validationstateDiagram-v2
[*] --> CheckTargetBranch
CheckTargetBranch --> BranchAlreadyExists: target branch exists
CheckTargetBranch --> FetchBase: target branch is new
FetchBase --> BaseNotResolved: fetch fails or ref is missing
FetchBase --> CutBranch: base fetched successfully
CutBranch --> WorkspacePrepared
BranchAlreadyExists --> [*]
BaseNotResolved --> [*]
WorkspacePrepared --> [*]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="rust/devlaunch-core/src/flows/workspace_clone.rs" line_range="820-830" />
<code_context>
+ });
+ }
+
+ let base = match self.repo_manager.fetch_ref(owner, repo, from_ref, notices) {
+ Ok(FetchOutcome::Updated) => BranchBase::Fresh,
+ Ok(FetchOutcome::RefMissingOnRemote) => {
+ return Err(EnsureBranchError::BaseNotResolved {
+ base: from_ref.to_owned(),
+ });
+ }
+ Ok(FetchOutcome::Failed { reason }) => {
+ notices.say(CacheNotice::RefNotFetched {
</code_context>
<issue_to_address>
**issue (broader_impact):** A failed fetch of the requested base returns `BaseNotResolved` before `ensure_branch_exists` is called, so `--from` refuses to cut the branch instead of reporting the fetch failure and proceeding from the base already present in the cache. This contradicts the stated behavior and the test coverage described for a failed base fetch.
**Triggers:** When the remote fetch for `from_ref` fails but the requested base already exists locally in the bare cache.
**Suggested fix:** Preserve the cached base on `FetchOutcome::Failed` after emitting `RefNotFetched`, and continue to `ensure_branch_exists`; reserve `BaseNotResolved` for `RefMissingOnRemote` and unsafe/unresolvable refs.
```suggestion
Ok(FetchOutcome::Failed { reason }) => {
notices.say(CacheNotice::RefNotFetched {
owner: owner.to_owned(),
repo: repo.to_owned(),
branch: from_ref.to_owned(),
reason: NotRefreshed::FetchFailed { reason },
});
BranchBase::Stale {
base: from_ref.to_owned(),
reason: NotRefreshed::FetchFailed { reason },
}
}
```
</issue_to_address>
Comment on lines
+820
to
+830
| Ok(FetchOutcome::Failed { reason }) => { | ||
| notices.say(CacheNotice::RefNotFetched { | ||
| owner: owner.to_owned(), | ||
| repo: repo.to_owned(), | ||
| branch: from_ref.to_owned(), | ||
| reason: NotRefreshed::FetchFailed { reason }, | ||
| }); | ||
| return Err(EnsureBranchError::BaseNotResolved { | ||
| base: from_ref.to_owned(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
issue (broader_impact): A failed fetch of the requested base returns BaseNotResolved before ensure_branch_exists is called, so --from refuses to cut the branch instead of reporting the fetch failure and proceeding from the base already present in the cache. This contradicts the stated behavior and the test coverage described for a failed base fetch.
Triggers: When the remote fetch for from_ref fails but the requested base already exists locally in the bare cache.
Suggested fix: Preserve the cached base on FetchOutcome::Failed after emitting RefNotFetched, and continue to ensure_branch_exists; reserve BaseNotResolved for RefMissingOnRemote and unsafe/unresolvable refs.
Suggested change
| Ok(FetchOutcome::Failed { reason }) => { | |
| notices.say(CacheNotice::RefNotFetched { | |
| owner: owner.to_owned(), | |
| repo: repo.to_owned(), | |
| branch: from_ref.to_owned(), | |
| reason: NotRefreshed::FetchFailed { reason }, | |
| }); | |
| return Err(EnsureBranchError::BaseNotResolved { | |
| base: from_ref.to_owned(), | |
| }); | |
| } | |
| Ok(FetchOutcome::Failed { reason }) => { | |
| notices.say(CacheNotice::RefNotFetched { | |
| owner: owner.to_owned(), | |
| repo: repo.to_owned(), | |
| branch: from_ref.to_owned(), | |
| reason: NotRefreshed::FetchFailed { reason }, | |
| }); | |
| BranchBase::Stale { | |
| base: from_ref.to_owned(), | |
| reason: NotRefreshed::FetchFailed { reason }, | |
| } | |
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
`dl owner/repo@fix/123` creates the branch if it is not there, and what it cut from was settled with no way to ask for anything else: the default branch's remote ref, freshly fetched. `--from` replaces the answer `WorkspaceCloneManager::ensure_branch` gives and nothing else. Carried forward onto upstream's v0.49.0 rework of that method (312005c, 1a8dc37, 8da1a86) rather than replayed from the old fork's diff: upstream's current `ensure_branch` already matches the shape the old patch assumed, so the seam it hooks into (a new `ensure_branch_from` arm, taken before the no-flag fetch/default-branch dance) needed no rediscovery. `prepare_cold_from` threads `from` through the lock the same way `prepare_cold` always has, and `prepare_cold` itself becomes a `None` call of it so the module's forty-odd existing test call sites stay untouched. An unresolvable base is a refusal naming the base, and creates no branch: falling back to the default would be the same "invented base" failure the no-flag path already forbids, arriving through a new door. A branch that already exists is a refusal that names the branch, not the base -- existence is decided before the base is resolved, so ignoring the flag there would look like it worked while the workspace opens on whatever the branch already is. Like `--claude-profile` and unlike `--devcontainer`: a base describes an event that happened once, and a workspace that re-applied one on every later launch would be claiming to re-cut a branch it did not. `Launch::from_ref` rides one call and is never read back off a record. `aid` gained no `--from` of its own, but `DL_VALUE_OPTIONS` had to learn that dl's `--from` takes a value, or `aid --from develop owner/repo fix it` reads "develop" as the prompt's first word. `completions/dl.bash` carries the same fact in three tables, each pinned by `completion_tables`. `render_select` picks up the same `#[allow(clippy::too_many_arguments)]` its sibling `render_workspace` already carries, for the trio of independent launch modifiers. README's `--from` paragraph is restored (it was pulled earlier for describing a flag the binary did not yet have), and the public-api snapshots carry the four new additive rows (`Launch::from_ref` twice, `EnsureBranchError`'s two new variants), inserted by anchoring on content rather than copying a full regeneration -- a local nightly renders unrelated toolchain noise (`core::io::error::Error` vs `std::io::error::Error`, blanket-impl reordering) that has nothing to do with this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdSFnBdxie6TosHVmjLY28
JSmithRobotics
force-pushed
the
feat/from-ref
branch
from
October 2, 2026 10:32
aaeecdf to
23701e3
Compare
Collaborator
Author
|
Not changing this one — the behaviour is intended, and the doc directly above
So the review's premise — that this contradicts the stated behaviour and the test coverage — does not hold. The asymmetry with the no-
The refusal names the ref, so the recovery is to fetch and re-run, or to pass a base that is already current. |
This branch has not been deployed
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
1 participant
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.
What this adds
dl owner/repo@branchcuts a new branch from the repository's default branch.--from <ref>says which base to cut from instead:The base is fetched fresh before the branch is cut, so
--fromnames a ref as it is on the remote now, not as a stale local clone last saw it.Behaviour worth stating
--fromdoes not silently rebase or move it; the existing branch wins and is named as such.--fromon a later attach would be meaningless at best.from_is_absent_from_what_the_workspace_storespins that.Testing
Nine unit tests in
workspace_clone.rscover: cutting from a named base, an unresolvable base being refused without creating a branch, a base fetch that fails being reported while the cut still proceeds from what is there,--fromagainst an already-existing branch naming the branch rather than the base, and the flag's absence from stored workspace state.--fromis added to the README's flag table, toVALUE_FLAGS(pinned against clap's own parser bythe_value_flags_are_the_ones_clap_takes_values_for), and to the bash completion's option and value-taking lists.cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings, and the README/citation/prose doc guards all pass.Summary by Sourcery
Allow users to cut new workspace branches from a named, freshly fetched remote ref with explicit validation and one-time launch semantics.
New Features:
--from <ref>launch option that creates new branches from a freshly fetched named remote ref instead of the repository default branch.Bug Fixes:
--fromis used with an existing branch.--fromfrom being persisted in workspace state or applied to commands that do not cut branches.Enhancements:
Documentation:
--from <ref>usage, applicability, and one-time launch semantics in the README.Tests:
--from.