feat(run): autodiscover project agent config#3681
Conversation
|
and .hcl please |
There was a problem hiding this comment.
Autodiscovery matches #2435 (including the .hcl request), keeps explicit arguments and --remote untouched, and ships focused tests plus doc updates. One inconsistency blocks approval: discovery runs after the sandbox decision in runRunCommand, so a discovered config declaring runtime.sandbox: true silently runs unsandboxed. Details in the inline comments.
| # | Severity | Finding |
|---|---|---|
| 1 | blocking | runtime.sandbox: true in a discovered config is ignored; fix by normalizing args early, like --agent-picker does |
| 2 | should | Discovery is silent; print which project config was picked |
| 3 | nit | Stat-error branch needs a comment (and ideally a test) |
| 4 | nit | Three doc/help mentions still describe the old behavior |
| 5 | discussion | Trust boundary: a repo-provided config runs MCP command toolsets and session_start hooks with no consent gate; a direnv-style consent prompt could be a follow-up |
Verified on PR head: go test -run TestResolveRunAgentFileName ./cmd/root pass, gofmt clean, no conflicts with main.
| return nil | ||
| } | ||
|
|
||
| func (f *runExecFlags) resolveRunAgentFileName(args []string) string { |
There was a problem hiding this comment.
Discovery at this layer runs after runRunCommand has already made the sandbox decision from args[0]:
resolveSandboxDefault(ctx, agentRef, ...)receivesagentRef == ""on a no-arg run, soruntime.sandbox: truein a discovereddocker-agent.yamlis silently ignored. The same file passed explicitly runs sandboxed.- On a
--sandboxno-arg run,runInSandboxreceives an emptyagentRef: the kit build is skipped andsandbox.EnvForAgentforwards nothing. The inner run only works because it re-discovers the file inside the workspace, in a degraded mode compared to an explicit path.
The --agent-picker block already solves this shape of problem by normalizing args before dispatch. Discovery can reuse that pattern in runRunCommand, right after the picker block and before resolveSandboxDefault:
if len(args) == 0 && f.remoteAddress == "" {
if name, ok := discoverProjectDefaultAgentFile(); ok {
args = prependAgentRef(name, args)
}
}resolveRunAgentFileName then reduces to the previous args[0] read, and explicit, discovered, sandboxed, and remote runs all agree on which config is in play.
| if len(args) > 0 { | ||
| agentFileName = args[0] | ||
| } | ||
| agentFileName := f.resolveRunAgentFileName(args) |
There was a problem hiding this comment.
Discovery is silent: nothing in the output distinguishes a run that picked ./docker-agent.yaml from one using the built-in default agent. Recording mode and worktrees both announce themselves via out.Println; a similar one-liner here keeps surprises down, and matters most for --exec where no TUI sidebar hints at the loaded agent:
out.Println("Using project config: " + agentFileName)Printed only when the name came from discovery, not from an explicit argument.
| if err == nil && !info.IsDir() { | ||
| return name, true | ||
| } | ||
| if err != nil && !errors.Is(err, os.ErrNotExist) { |
There was a problem hiding this comment.
This branch reports a candidate as discovered even though os.Stat failed (e.g. permission error), so the downstream load surfaces a real error instead of silently running the built-in default next to an unreadable docker-agent.yaml. Failing loudly seems right, but two things are missing:
- a comment: without one the code reads like a bug, and the branch also means an unreadable
docker-agent.yamlshadows a readabledocker-agent.ymllater in the list, which is a deliberate choice worth stating - optionally a test locking the behavior; a self-referential symlink named
docker-agent.yamltriggers a non-ErrNotExiststat error portably on unix (skip on Windows)
| > **No config needed** | ||
| > | ||
| > Running `docker agent run` without a config file uses a built-in default agent. Perfect for quick experimentation. | ||
| > Running `docker agent run` without a config argument uses `docker-agent.yaml`, `docker-agent.yml`, or `docker-agent.hcl` from the current directory when present. Otherwise, it uses a built-in default agent that is perfect for quick experimentation. |
There was a problem hiding this comment.
Three more spots still describe the pre-discovery behavior and now contradict this paragraph:
| Location | Current text |
|---|---|
docs/features/cli/index.md:630 |
Agent References table, row "Default": "(no argument) uses built-in default agent" |
docs/index.md:277 |
"Zero Config" section: "# Run the built-in default agent" |
cmd/root/run.go:124 |
help example: docker-agent run # built-in default agent |
|
Thanks for the review. I pushed
For the trust-boundary point around repo-provided configs running command toolsets/hooks, I left that as follow-up scope since it was marked as discussion and would need a broader consent model than this autodiscovery slice. |
|
looks good now ! thanks for the contribution ! |
|
Can you sign your commits ? |
Signed-off-by: AmirHossein HajiMohammadi <[email protected]>
Signed-off-by: AmirHossein HajiMohammadi <[email protected]>
Signed-off-by: AmirHossein HajiMohammadi <[email protected]>
636a73f to
fb65180
Compare
|
Done, thanks for the reminder. I rewrote the PR commits with SSH signatures and added the signing key to GitHub; the commits now show valid signatures. |
Refs #2435
Summary
docker-agent.yaml/docker-agent.yml/docker-agent.hclfor no-argument localdocker agent runruntime.sandbox: trueis honored the same as explicit config paths--remoteruns unchangedWhy
Repositories can now check in a conventional project-level agent config and let developers run
docker agent runwithout remembering a file path. This keeps the first slice focused on autodiscovery and leaves nativedocker agent newscaffolding for a follow-up.Testing
gofmtgit diff --checkgo test -p=1 ./cmd/root -run TestDiscoverRunAgentArgs -count=1ingolang:1.26.5go test -p=1 ./cmd/root -count=1ingolang:1.26.5./scripts/build.shingolang:1.26.5go mod tidy --diffingolang:1.26.5go run ./lint .ingolang:1.26.5golangci-lint run --concurrency=2with v2.12.2 ingolang:1.26.5