Skip to content

bin/worktree: idempotent provision subcommand + .agents/provision hook#2732

Closed
leshy wants to merge 3 commits into
mainfrom
agent/worktree-provision
Closed

bin/worktree: idempotent provision subcommand + .agents/provision hook#2732
leshy wants to merge 3 commits into
mainfrom
agent/worktree-provision

Conversation

@leshy

@leshy leshy commented Jul 6, 2026

Copy link
Copy Markdown
Member

Factors the provisioning steps out of bin/worktree new into an idempotent bin/worktree provision [dir] (new now calls it), and adds a 2-line executable .agents/provision shim exec-ing it.

This is the repo side of the agent-harness worktree provisioning convention: any tool that cuts a plain git worktree add tree (agent harnesses, CI) can run .agents/provision to get a working uv venv + deps + direnv without knowing repo internals.

Verified by provisioning a bare git worktree add scratch tree: pytest, mypy and import dimos all resolve from the freshly created .venv. Branch was itself dogfooded via bin/worktree new.

Factor 'new''s provisioning steps (.envrc symlink + uv venv + uv sync
--all-groups + direnv allow) into 'bin/worktree provision [dir]', which
provisions an EXISTING worktree in place and is idempotent; 'new' now calls
it. Add executable .agents/provision at the repo root — a 2-line shim over
'bin/worktree provision' — so an agent harness can auto-set-up a freshly-cut
worktree after a bare 'git worktree add'.

Verified: provisioned a plain 'git worktree add' scratch tree; pytest 8.3.5,
mypy 1.19.0, and 'import dimos' all resolve from the new .venv.
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a reusable worktree provisioning path. The main changes are:

  • New .agents/provision shim that delegates to bin/worktree provision.
  • New idempotent bin/worktree provision [dir] subcommand.
  • bin/worktree new now provisions by calling the shared helper.
  • Provisioning now creates or reuses the uv environment and runs dependency sync.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
.agents/provision Adds an executable provisioning shim that resolves basename invocation before delegating to the worktree helper.
bin/worktree Adds the shared provision subcommand and routes new worktree creation through it.

Reviews (3): Last reviewed commit: "fix: resolve .agents/provision when invo..." | Re-trigger Greptile

Comment thread .agents/provision Outdated
@@ -0,0 +1,3 @@
#!/usr/bin/env bash
# Agent worktree provisioning hook: set this tree up (uv venv/sync, .envrc, direnv).
exec "$(dirname "$0")/../bin/worktree" provision "$@"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Relative Hook Target

When an agent invokes this hook by basename from PATH, $0 is just provision, so dirname "$0" becomes . and the script tries to exec ../bin/worktree relative to the caller's current directory. A harness running from a scratch directory can fail before provisioning starts, even though the hook file was found correctly.

Suggested change
exec "$(dirname "$0")/../bin/worktree" provision "$@"
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
exec "$SCRIPT_DIR/../bin/worktree" provision "$@"

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread bin/worktree Outdated
# allow. Idempotent, so it's safe to re-run and `new` calls it after adding
# the tree; you can also point it at a bare `git worktree add` dir to set it
# up after the fact. Defaults to the current directory.
local DIR="${1:-.}" resolved=""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unscoped Default Provisioning

Running bin/worktree provision with no directory now provisions whatever directory the caller is in. If the command is launched from the main checkout instead of a bare worktree, it can run uv venv and uv sync --all-groups against the main tree, which was not reachable through the old new-only provisioning path.

Review findings on #2732:
- .agents/provision resolved bin/worktree via a relative dirname "$0",
  which breaks when invoked from PATH or another cwd; resolve absolutely.
- `worktree provision` with no argument provisioned the caller's cwd,
  so running it from the main checkout would uv-venv/sync the main tree;
  default to the root of the tree the script lives in instead (explicit
  dir argument unchanged). REPO_ROOT is likewise anchored on the script
  location so the shim works from any cwd.
- uv venv --allow-existing keeps re-provisioning idempotent with uv
  versions that refuse an existing .venv.
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
2649 1 2648 72
View the full list of 1 ❄️ flaky test(s)
dimos.e2e_tests.test_dimsim_walk_forward::test_walk_forward

Flake rate in main: 23.23% (Passed 76 times, Failed 23 times)

Stack Traces | 205s run time
lcm_spy = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x7200d44a26f0>
start_blueprint = <function start_blueprint.<locals>.set_name_and_start at 0x7200d153d260>
human_input = <function human_input.<locals>.send_human_input at 0x7200d153cb80>
dim_sim = <dimos.e2e_tests.dim_sim_client.DimSimClient object at 0x7200d32a8260>

    @pytest.mark.self_hosted_large
    def test_walk_forward(lcm_spy, start_blueprint, human_input, dim_sim) -> None:
        start_blueprint(
            "run",
            "--disable",
            "spatial-memory",
            "--disable",
            "security-module",
            "unitree-go2-agentic",
            simulator="dimsim",
        )
        lcm_spy.save_topic(".../McpClient/on_system_modules/res")
        lcm_spy.wait_for_saved_topic(".../McpClient/on_system_modules/res", timeout=1200.0)
    
        origin_x, origin_y = 1, 2
        dim_sim.set_agent_position(origin_x, origin_y)
    
        human_input("move forward 3 meter")
    
>       lcm_spy.wait_until_odom_position(origin_x + 3, origin_y, threshold=0.4, timeout=120)

dim_sim    = <dimos.e2e_tests.dim_sim_client.DimSimClient object at 0x7200d32a8260>
human_input = <function human_input.<locals>.send_human_input at 0x7200d153cb80>
lcm_spy    = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x7200d44a26f0>
origin_x   = 1
origin_y   = 2
start_blueprint = <function start_blueprint.<locals>.set_name_and_start at 0x7200d153d260>

dimos/e2e_tests/test_dimsim_walk_forward.py:37: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
dimos/e2e_tests/lcm_spy.py:167: in wait_until_odom_position
    self.wait_for_message_result(
        predicate  = <function LcmSpy.wait_until_odom_position.<locals>.predicate at 0x7200d153c180>
        self       = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x7200d44a26f0>
        threshold  = 0.4
        timeout    = 120
        x          = 4
        y          = 2
dimos/e2e_tests/lcm_spy.py:153: in wait_for_message_result
    wait_until(
        event      = <threading.Event at 0x7200d32a8860: unset>
        fail_message = 'Failed to get to position x=4, y=2'
        listener   = <function LcmSpy.wait_for_message_result.<locals>.listener at 0x7200d153d8a0>
        predicate  = <function LcmSpy.wait_until_odom_position.<locals>.predicate at 0x7200d153c180>
        self       = <dimos.e2e_tests.lcm_spy.LcmSpy object at 0x7200d44a26f0>
        timeout    = 120
        topic      = '/odom#geometry_msgs.PoseStamped'
        type       = <class 'dimos.msgs.geometry_msgs.PoseStamped.PoseStamped'>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

predicate = <bound method Event.is_set of <threading.Event at 0x7200d32a8860: unset>>

    def wait_until(
        predicate: Callable[[], bool],
        *,
        timeout: float,
        interval: float = 0.1,
        message: str | None = None,
    ) -> None:
        """Poll ``predicate`` until it returns truthy or ``timeout`` elapses."""
        deadline = time.monotonic() + timeout
        while time.monotonic() < deadline:
            if predicate():
                return
            time.sleep(interval)
>       raise TimeoutError(message or f"Timed out after {timeout}s waiting for condition")
E       TimeoutError: Failed to get to position x=4, y=2

deadline   = 3454841.370040225
interval   = 0.1
message    = 'Failed to get to position x=4, y=2'
predicate  = <bound method Event.is_set of <threading.Event at 0x7200d32a8860: unset>>
timeout    = 120

.../utils/testing/waiting.py:35: TimeoutError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@leshy

leshy commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Both findings fixed in 998d3ff: the .agents/provision shim now resolves its own directory absolutely (SCRIPT_DIR=$(cd $(dirname $0) && pwd)) so it works when invoked from PATH or any cwd, and no-arg worktree provision now defaults to the root of the tree the script lives in instead of the caller's cwd (explicit-dir form unchanged; REPO_ROOT is likewise anchored on the script location). Also added uv venv --allow-existing so re-provisioning stays idempotent with newer uv. Verified from /tmp on an already-provisioned tree: runs in ~7s and targets the script's own worktree.

Comment thread .agents/provision Outdated
Comment on lines +3 to +4
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
exec "$SCRIPT_DIR/../bin/worktree" provision "$@"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Resolve PATH invocation

When the hook is invoked by basename from PATH, $0 is just provision. In that case dirname "$0" becomes ., so SCRIPT_DIR becomes the caller's current directory and the shim tries to exec $PWD/../bin/worktree. A harness can find .agents/provision on PATH but still fail before provisioning starts when its cwd is not the worktree's .agents directory. Resolve $0 through PATH before deriving SCRIPT_DIR.

Suggested change
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
exec "$SCRIPT_DIR/../bin/worktree" provision "$@"
SELF=$0
case "$SELF" in
*/*) ;;
*) SELF=$(command -v -- "$SELF") ;;
esac
SCRIPT_DIR="$(cd "$(dirname "$SELF")" && pwd)"
exec "$SCRIPT_DIR/../bin/worktree" provision "$@"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5b3eaa8: the shim now resolves a slash-less $0 with command -v before computing SCRIPT_DIR, as suggested. Verified from /tmp that both absolute-path invocation and bare-basename-via-PATH invocation provision the script's own tree (/home/lesh/coding/dimos-worktree-provision), not the caller's cwd.

When the shim is run by basename via PATH, $0 has no slash and
dirname yields "." (the caller's cwd again); look the name up with
command -v first so SCRIPT_DIR is the script's real directory.
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