Skip to content

feat: add top-level -C <path> flag (git -C semantics)#523

Merged
avihut merged 5 commits into
masterfrom
daft-519/feat/run-command-in-dir
May 18, 2026
Merged

feat: add top-level -C <path> flag (git -C semantics)#523
avihut merged 5 commits into
masterfrom
daft-519/feat/run-command-in-dir

Conversation

@avihut
Copy link
Copy Markdown
Owner

@avihut avihut commented May 17, 2026

Note: Replaces #522 — that PR was accidentally merged then reverted (master force-pushed back to pre-merge state). This is the same work on the same branch; opening fresh here because GitHub doesn't allow reopening a merged PR. Original comment thread: #522

Summary

Adds a top-level daft -C <path> <subcommand> global flag that chdirs before
any path-dependent state is resolved (repo discovery, layout, hooks,
daft.yml, multicall dispatch). Semantics match git -C / make -C /
pnpm -C. Works for both the daft multicall arm and every git-worktree-*
symlinked entry.

Motivating use case (per the issue): coding agents operating across multiple
daft worktrees in one session can't reliably cd, so each invocation has to
be self-contained. -C <path> turns every call into "do X in path Y".

Fixes #519.

Two corrections to the issue (raised in this comment)

  1. Multi--C composes, doesn't last-win. The issue claimed "last -C wins
    matches git" — that's empirically wrong about git, which composes
    (verified: git -C /tmp/x -C a -C b ... lands in /tmp/x/a/b).
    Implementation matches git: compose. Reasons in the issue comment.
  2. Stripped argv has to be globally visible, not just a local in main().
    Subcommands re-read std::env::args() directly via crate::get_clap_args()
    and a dozen other callers. Implemented as a OnceLock<Vec<String>> in a
    new src/cli/ module that owns the strip-and-install lifecycle; all argv
    readers now route through cli::argv().

Architecture (per ARCHITECTURE.md)

The new src/cli/ module is the seed for the future daft-cli crate, applied
as a small functional-core / imperative-shell split:

  • src/cli/argv.rs — pure parse_top_level_cwd(), 10 unit tests, no I/O.
  • src/cli/mod.rsinstall_and_apply() (chdir + OnceLock install), tiny
    imperative shell.

No ports/adapters ceremony for a CLI-layer concern — explicitly out of scope
per the doc's "vertical slice at the CLI command layer" rule. Just lifts argv
reading from "scattered env::args() calls" into one accessor.

Wrapper bug surfaced during integration testing

Adding the bash integration test caught a real wrapper-level bug: the
bash/zsh and fish daft() wrappers dispatched on case "\$1", so
daft -C /path verb args matched -C (not the verb) and fell through to the
no-DAFT_CD_FILE default arm. The shell would never follow the binary into
the new worktree — defeating the cd-redirect for the primary use case.

Fixed by stripping leading -C <path> pairs into a local before dispatching,
reattaching as args to the wrapped binary. Same pattern in both bash/zsh and
fish wrappers. The existing layout|repo cd-redirect branch also benefits.

Commits (4, logically separated for review)

  1. feat(cli): add top-level -C <path> flag with git semantics — core
    cli::install_and_apply + cli::argv(), wired into main.rs, all argv
    readers migrated.
  2. test(cli): cover -C end-to-end across YAML, integration, wrapper
    7 YAML scenarios in tests/manual/scenarios/global-flags/ (basic,
    missing, relative, compose, empty-noop, cd-redirect, hooks-from-new-cwd),
    2 bash integration tests in test_shell_init.sh, plus the wrapper fix.
  3. docs(cli): document -C across completions, man, help, skill, docs site — bash/zsh/fish/fig completions, man-page declaration in xtask
    (man-gen only; still parsed pre-clap), new Global options section in
    daft --help / git daft --help, new docs/cli/daft.md reference page,
    new docs/worktrees/from-anywhere.md how-to, SKILL.md "Global Flags"
    section for agents.
  4. fix(cli): -C trailing-no-arg exits 2 (was 1) — post-review fix.
    daft -C (no path) was exiting via anyhow Err → 1; now matches the
    directory-not-found branch with exit 2 (clap usage-error convention).
    Regression covered.

Acceptance criteria (from the issue)

  • daft -C <path> <subcommand> runs as if invoked from <path>
  • Works for daft and all git-worktree-* / shortcut-alias symlinks
  • Compose like git (corrected from the issue's "last wins")
  • Relative paths resolve against original cwd (first -C) or prior
    applied cwd (subsequent -Cs) — git's actual semantic
  • Missing/inaccessible path: exit 2 with terse daft: -C: '<path>': not a directory. Trailing -C (no arg): exit 2 with option requires an argument
  • Shell-integration cd-redirect works through -C
  • Hooks fire from post--C cwd; daft.yml resolves from new repo root
  • XDG state / config dirs unaffected
  • All four shell completions register -C with path completion
  • Man pages regenerated; daft --help documents -C as global flag
  • YAML scenarios under tests/manual/scenarios/global-flags/
  • Bash integration test for the symlink-entry + shell-wrapper case

Test plan

  • cargo test --lib — 1813/1813
  • cargo clippy --all-targets -D warnings — clean
  • mise run fmt — clean
  • mise run man:verify — up-to-date
  • mise run test:integration (full matrix: default+gitoxide × bash+yaml)
    — 2211/2211 across 579 scenarios
  • mise run test:manual -- --ci global-flags:* — 8 scenarios pass
  • tests/integration/test_completions.sh — 27/27
  • tests/integration/test_shell_init.sh — 19/21 (see Known issues below)
  • Spot-checked: shell wrapper followed daft into new worktree for both
    -C and non--C invocations.

Known issues, not from this PR

test_shell_init_bash_aliases and test_shell_init_fish_aliases fail
because they expect a gwcob alias that was renamed to gwcb upstream. The
failures pre-exist this PR; do not chase them as part of review.

🤖 Generated with Claude Code

@avihut avihut added this to the Public Launch milestone May 17, 2026
@avihut avihut added the feat New feature label May 17, 2026
@avihut avihut self-assigned this May 17, 2026
avihut and others added 4 commits May 18, 2026 01:14
Implements the foundation for `daft -C <path> <subcommand>` — a global flag
that chdirs before any path-dependent state is resolved (repo discovery,
layout, hooks, daft.yml, multicall dispatch). Semantics match git's `-C`:

- Compose, not last-wins: `daft -C /a -C b ...` → /a/b
- Empty `-C ""` is a no-op (matches `git -C ""`)
- Stops scanning at first non-option token, so subcommand-local `-C` flags
  (e.g. `daft exec -C ... script.sh`) are preserved
- Missing/non-directory path → terse error, exit 2 (clap usage convention)

Architecture (per ARCHITECTURE.md): seeds the future `daft-cli` crate as a
small functional-core / imperative-shell split:

- `src/cli/argv.rs` — pure `parse_top_level_cwd()`, 10 unit tests
- `src/cli/mod.rs` — `install_and_apply()` (chdir + OnceLock install) and
  `argv()` accessor

All callers that previously read `std::env::args()` directly are migrated to
`cli::argv()` so the strip is universally visible. Without this, subcommands
re-reading raw argv via clap would reject the now-consumed `-C` pair as
unknown flags. The install runs at the very top of `main()` — before
`skip_startup_tasks_for(argv)` so the fork-bomb gate keeps seeing the real
subcommand name in argv[1].

Tests, completions, docs, skill in follow-up commits per the plan.

Refs #519

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
YAML scenarios (tests/manual/scenarios/global-flags/):
- c-basic: -C chdirs before subcommand dispatch
- c-missing: non-existent path → exit 2 + terse error
- c-relative: first -C resolves against invocation cwd
- c-compose: multiple -C compose (git semantic), not last-win
- c-empty-noop: `-C ""` is a no-op (matches `git -C ""`)
- c-cd-redirect: binary writes DAFT_CD_FILE relative to post-`-C` cwd
- c-hooks-from-new-cwd: worktree-post-create hook in the -C target fires
  when invoking from another repo

Bash integration tests (tests/integration/test_shell_init.sh):
- c_flag_cd_redirect_through_wrapper: full shell-wrapper round-trip
- c_flag_symlink_entry: `git-worktree-list -C` via the symlink entry

Wrapper fix (src/commands/shell_init.rs):

Adding the integration test surfaced a wrapper-level bug: when invoked as
`daft -C /path verb args`, the `case "$1"` in the bash/zsh and fish daft()
wrappers matched `-C`, not the verb. The default arm runs `command daft "$@"`
WITHOUT setting DAFT_CD_FILE — so the shell never followed the binary into a
newly-created worktree, defeating the cd-redirect for the agent use case the
feature was built for.

Fix: strip leading `-C <path>` pairs into a local before dispatching, and
re-attach them as arguments to the wrapped binary so cli::install_and_apply
applies the chdir. Applied symmetrically to bash/zsh and fish; the existing
`layout|repo` cd-redirect path also benefits (e.g., `daft -C /a repo remove`
now keeps its cd-out behavior).

Refs #519

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surface the -C feature in every channel users discover it through:

- **Shell completions** (bash/zsh/fish/fig): top-level `-C <path>` flag with
  directory-value completion. Bash/zsh additionally normalize `${words[*]}`
  by stripping leading `-C <path>` pairs so subsequent subcommand-specific
  completion logic keeps working when `-C` is the first arg. Fish's positional
  short-letter matching makes the strip-and-restore unnecessary there.
- **Man pages** (`man/daft.1`): top-level `-C` arg declared on the clap
  Command in `xtask::build_top_level_command()`. Strictly for man-gen and
  --help rendering — actual parsing still happens in cli::install_and_apply
  pre-clap.
- **`daft --help` / `git daft --help`**: new "Global options" section in
  both renderers.
- **Docs site**: new `docs/cli/daft.md` reference page with full semantics
  (compose, empty-noop, error behavior, interactions) and new how-to
  `docs/worktrees/from-anywhere.md` framing the agent-driven use case.
  Sidebar registered in `docs/.vitepress/config.ts`.
- **Agent skill** (`SKILL.md`): "Global Flags" section calling out -C as the
  recommended pattern for agents operating across multiple worktrees in one
  session. This is the primary motivating audience for the feature.

The fig spec gained a `template` field on `FigOptionArg` so `-C` can use Fig's
built-in `folders` completion source; existing instantiations updated
mechanically.

Refs #519

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_tests

Two cleanups surfaced by post-implementation review:

**Exit code consistency.** `daft -C` (trailing flag with no path) was
propagating MissingPathAfterC through anyhow Err → main's `?` → exit code 1,
while `daft -C /missing list` exits 2 via the apply_chdir branch. Both are
usage errors and should match clap's exit-2 convention. Switch to direct
process::exit(2) in install_and_apply for the parse-error branch.

Extended c-missing.yml with a regression step for the trailing-`-C` case so
this can't drift again.

**Dead code.** install_for_tests() in cli/mod.rs was left behind when I
simplified the test module — never called. Removed; cfg(test) pub fn doesn't
trip default clippy dead-code lints, which is why it slipped through.

Refs #519

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@avihut
Copy link
Copy Markdown
Owner Author

avihut commented May 17, 2026

/claude review

@claude
Copy link
Copy Markdown

claude Bot commented May 17, 2026

Claude finished @avihut's task in 5m 9s —— View job


PR Review: feat: add top-level -C <path> flag (git -C semantics)

Todo List

  • Read CLAUDE.md guidelines
  • Read core implementation (src/cli/argv.rs, src/cli/mod.rs)
  • Read main wiring (src/main.rs, src/lib.rs)
  • Read shell wrapper changes (src/commands/shell_init.rs)
  • Review test coverage
  • Review documentation changes
  • Review completions changes
  • Formulate and post final review

Summary

Solid, well-scoped implementation. The functional-core / imperative-shell split is clean, the OnceLock global-argv pattern is appropriate, and the test suite is thorough. One stale test grep pattern to fix, a minor UX gap in the shell wrapper, and a few observations.


Issues

Medium — stale grep in test_daft_wrapper_function_exists

tests/integration/test_shell_init.sh, line ~378

if grep -q 'command daft "\$@"' <<< "$output"; then
    log_success "daft() wrapper passes through other commands"
else
    log_error "daft() wrapper missing passthrough"
    return 1
fi

The generated BASH_ZSH_WRAPPER constant changed. The *) passthrough arm is now:

        *)
            command daft "${__daft_pre[@]}" "$@" ;;

The grep pattern command daft "\$@" (which resolves to command daft "$@") is not a substring of command daft "${__daft_pre[@]}" "$@", so this check will fail. The same check appears in test_daft_wrapper_function_exists, not test_daft_wrapper_passthrough (which tests behavior, not grep-in-code — that one is fine).

The PR description claims 19/21 pass with the two known alias failures, but test_daft_wrapper_function_exists should be a third failure. Worth verifying and fixing the grep to use:

grep -q 'command daft.*"\$@"' <<< "$output"

Fix this →


Minor — bash wrapper silently returns 2 for daft -C (no path arg)

src/commands/shell_init.rs, bash daft() strip loop

-C)
    __daft_pre+=("$1" "$2"); shift 2 || return 2 ;;

When $# is 1 (just -C, no following arg), shift 2 fails and return 2 fires — correct exit code. But the binary never executes, so the user sees no error message at all: just a silent non-zero exit. The binary path (eprintln!("daft: -C: option requires an argument")) is bypassed.

The fish wrapper has the same gap via the test (count $argv) -ge 3 guard path.

Mitigation: add an explicit echo "daft: -C: option requires an argument" >&2 before return 2. Rare case, but the error message is what teaches the user what went wrong.


Observations (no action required)

TOCTOU in apply_chdir (src/cli/mod.rs:55–65)

fn apply_chdir(path: &Path) {
    if !path.is_dir() {          // check
        ...exit(2);
    }
    std::env::set_current_dir(path) // use

is_dir() and set_current_dir() are two separate syscalls, creating a race window. Entirely acceptable for a CLI tool; set_current_dir's Err path handles the failure correctly. Just worth knowing.

Parser stops at first non--C token — correct, but undiscoverable via -- separator

daft -- -C /path verb would pass -- to the subcommand (stopping the top-level scan at --), which is correct git semantics. The parser handles this implicitly (non--C token stops scan). No action needed, just noting the edge case is handled correctly.

main.rs ordering comment (line 14–21) is load-bearing

The comment explaining why install_and_apply must precede skip_startup_tasks_for (the gate reads argv[1] and would fail open if -C were still there) is exactly right. Worth preserving in future refactors.

OnceLock double-install → panic, not error

install_and_apply returns Err if called twice, which propagates through main's ? as exit 1. But argv() panics if called before install. The panic is documented as a programming error and is the right choice — this is not a recoverable runtime state.


Positive highlights

  • Functional core / imperative shell: argv.rs (pure parser, 10 unit tests, zero I/O) + mod.rs (chdir + OnceLock install) is exactly the right split.
  • Compose semantics match git: applying each chdir_path as a separate set_current_dir call inherently composes relative paths, matching git -C /a -C b/a/b.
  • Wrapper -C strip is correct for the common case: the while/case strip loop in both bash/zsh and fish correctly re-attaches __daft_pre to every binary invocation, including the layout|repo cd-redirect arm.
  • skip_startup_tasks_for still works correctly: the stripped argv is installed before the gate, so daft -C /path shell-init bash won't spawn background tasks (the gate sees shell-init at argv[1] of the stripped argv).
  • Test scenario coverage is excellent: c-basic, c-compose, c-relative, c-missing, c-empty-noop, c-cd-redirect, and c-hooks-from-new-cwd together cover the acceptance criteria exhaustively. The compose test (cwd /, -C $WORK_DIR -C repo-b/main) is particularly well-designed to make last-wins vs. compose observable.

@avihut avihut force-pushed the daft-519/feat/run-command-in-dir branch from 27180f0 to b8462a0 Compare May 17, 2026 22:16
…al grep

Two PR review follow-ups (#523):

**bash/zsh wrapper: silent exit on `daft -C` (no path).** Previously
`shift 2 || return 2` short-circuited silently when `$#` was 1, so the user
saw exit code 2 with no message — the binary's
`eprintln!("daft: -C: option requires an argument")` never ran. Now the
wrapper prints the same message to stderr before returning. Added
`test_c_flag_no_arg_through_wrapper_errors_cleanly` to lock in the behavior.

Fish wrapper unaffected: its strip-loop falls through to `command daft -C`
when argv is exhausted, so the binary's error path does fire.

**Drop tautological grep in `test_daft_wrapper_function_exists`.** The grep
`'command daft "\$@"'` was originally a smoke check for the wrapper's
passthrough arm. After PR #523 the wrapper emits
`command daft "${__daft_pre[@]}" "$@"` and the literal pattern no longer
matches the code — but it *does* match an explanatory comment in the wrapper
that includes the literal `*) command daft "$@"`, so the test passes by
accident. Removed; `test_daft_wrapper_passthrough` already exercises the
behavior end-to-end by sourcing the wrapper and invoking daft.

Refs #519, addresses /claude-review feedback on #523.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@avihut avihut merged commit 28a8ed1 into master May 18, 2026
13 checks passed
@avihut avihut deleted the daft-519/feat/run-command-in-dir branch May 18, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add top-level -C <path> flag (git -C semantics)

1 participant