From ce2e98c1a8cc88c541f8e159536d9ad6d3a29001 Mon Sep 17 00:00:00 2001 From: "chencong.fu@chaitin.com" Date: Fri, 17 Jul 2026 21:12:48 +0800 Subject: [PATCH 1/2] feat(compose): unify resource source configuration --- README.md | 13 +- README.zh-CN.md | 7 +- .../cli_daemon_lifecycle_test.go | 4 +- cmd/agent-compose/cli_project_test.go | 2 +- .../cli_project_workflow_test.go | 17 +- cmd/agent-compose/cli_scheduler_test.go | 8 +- .../e2e_docker_scheduler_test.go | 5 +- docs/design/agent-compose_design.md | 3 +- docs/pages/agent-compose-yaml-manual.md | 104 +++-- docs/pages/zh-CN/agent-compose-yaml-manual.md | 104 +++-- examples/agent-compose/README.md | 2 +- examples/agent-compose/README.zh-CN.md | 2 +- .../docker-scheduler-script-url/README.md | 4 +- .../README.zh-CN.md | 4 +- .../agent-compose.yml | 3 +- .../adapters/agent_runner_test.go | 2 +- ...orkspace_failure_retry_integration_test.go | 4 +- pkg/agentcompose/api/project.go | 40 +- pkg/agentcompose/api/project_test.go | 19 +- .../app/controller_helpers_coverage_test.go | 2 +- pkg/compose/coverage_shape_workflows_test.go | 4 +- pkg/compose/helper_coverage_test.go | 4 +- pkg/compose/normalize.go | 384 ++++++++++-------- pkg/compose/normalize_test.go | 127 ++++-- pkg/compose/output.go | 42 +- pkg/compose/output_test.go | 10 +- pkg/compose/script_source.go | 68 +++- pkg/compose/script_source_test.go | 59 ++- pkg/compose/spec.go | 126 +++--- pkg/compose/spec_test.go | 55 ++- pkg/compose/workspace_commit_test.go | 71 ---- pkg/compose/workspace_ref_test.go | 50 +++ pkg/model/agent_model.go | 36 +- pkg/projects/controller_coverage_test.go | 2 +- pkg/projects/legacy_default_project.go | 2 +- ...legacy_default_project_integration_test.go | 2 +- pkg/projects/legacy_default_project_test.go | 2 +- pkg/projects/legacy_workspace_binding.go | 4 +- pkg/projects/legacy_workspace_binding_test.go | 6 +- pkg/projects/legacy_workspace_migration.go | 17 +- .../legacy_workspace_migration_test.go | 33 +- pkg/projects/records.go | 5 +- pkg/projects/records_test.go | 8 +- pkg/runs/coverage_shape_workflows_test.go | 27 +- pkg/runs/preparation.go | 32 +- pkg/runs/preparation_legacy_workspace_test.go | 6 +- pkg/runs/preparation_workspace_compat_test.go | 23 +- ...oject_workspace_resume_integration_test.go | 2 +- pkg/runs/workspace.go | 27 +- pkg/skills/resolve.go | 173 ++------ pkg/skills/resolve_test.go | 134 +++--- pkg/sources/git.go | 223 ++++++++++ pkg/sources/git_test.go | 141 +++++++ pkg/sources/source.go | 96 +++++ .../configstore/schema_coverage_test.go | 4 +- pkg/workspaces/git_workspace.go | 138 +------ pkg/workspaces/provisioner_git_state_test.go | 5 +- pkg/workspaces/workspace_coverage_test.go | 32 +- proto/agentcompose/v2/agentcompose.pb.go | 80 +++- proto/agentcompose/v2/agentcompose.proto | 11 +- ...ocker_workspace_resume_host_daemon_test.go | 2 +- 61 files changed, 1566 insertions(+), 1056 deletions(-) delete mode 100644 pkg/compose/workspace_commit_test.go create mode 100644 pkg/compose/workspace_ref_test.go create mode 100644 pkg/sources/git.go create mode 100644 pkg/sources/git_test.go create mode 100644 pkg/sources/source.go diff --git a/README.md b/README.md index 50ef2a1b6..6e2471464 100644 --- a/README.md +++ b/README.md @@ -133,13 +133,13 @@ More runnable examples (cron, timeout, scheduler scripts) live in ## The compose file -**Top-level fields:** `name`, `variables`, `agents`, `mcp_servers`, `volumes`, `network`. +**Top-level fields:** `name`, `env_file`, `variables`, `workspaces`, `agents`, `mcp_servers`, `volumes`, `network`. **Common agent fields:** `provider`, `model`, `system_prompt`, `image`, `driver`, `env` (scalars or `{ value, secret }`), `workspace`, `scheduler`, `mcp_servers`, `skills`, and `volumes`. -Provision an agent's workspace from a local path (`provider: local`) or a Git +Provision an agent's workspace from a local path (`provider: file`) or a Git repository (`provider: git`): ```yaml @@ -148,12 +148,13 @@ agents: workspace: provider: git url: https://github.com/example/repo.git - branch: main + ref: main + target: . ``` -Scheduler scripts may be inline JavaScript or an explicit `{ url: ... }` -source using a local path, `file://`, `http://`, or `https://`. `config` and -`up` fetch URL sources locally and send an inline snapshot to the daemon. Use +Scheduler scripts may be inline JavaScript or a flat source mapping using +`provider: file`, `provider: http`, or `provider: git`. `config` and `up` +fetch mapped sources locally and send an inline snapshot to the daemon. Use either `scheduler.script` or `scheduler.triggers` in one scheduler. Add scheduled or event-driven runs. Use either `scheduler.triggers` **or** an diff --git a/README.zh-CN.md b/README.zh-CN.md index 73eef71d4..b174527ff 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -98,12 +98,12 @@ agent-compose down # 停止 sandbox、禁用 sche ## Compose 配置 -**顶层字段:** `name`、`variables`、`agents`、`mcp_servers`、`volumes`、`network`。 +**顶层字段:** `name`、`env_file`、`variables`、`workspaces`、`agents`、`mcp_servers`、`volumes`、`network`。 **agent 常用字段:** `provider`、`model`、`system_prompt`、`image`、`driver`、 `env`(scalar 或 `{ value, secret }`)、`workspace`、`scheduler`、`mcp_servers`、`skills`、`volumes`。 -为 agent 从本地路径(`provider: local`)或 Git 仓库(`provider: git`)配置 workspace: +为 agent 从本地路径(`provider: file`)或 Git 仓库(`provider: git`)配置 workspace: ```yaml agents: @@ -111,7 +111,8 @@ agents: workspace: provider: git url: https://github.com/example/repo.git - branch: main + ref: main + target: . ``` 添加定时或事件驱动的 run。`scheduler.triggers` 与内联 `scheduler.script` 在同一 scheduler 中二选一: diff --git a/cmd/agent-compose/cli_daemon_lifecycle_test.go b/cmd/agent-compose/cli_daemon_lifecycle_test.go index 5dac6f5b7..6abdae69d 100644 --- a/cmd/agent-compose/cli_daemon_lifecycle_test.go +++ b/cmd/agent-compose/cli_daemon_lifecycle_test.go @@ -142,7 +142,7 @@ func TestConfigCommandPrintsNormalizedYAMLWithoutStartingDaemon(t *testing.T) { writeComposeFile(t, dir, ` workspaces: default: - provider: local + provider: file path: . variables: API_KEY: @@ -190,7 +190,7 @@ variables: secret: true workspaces: default: - provider: local + provider: file path: . agents: reviewer: diff --git a/cmd/agent-compose/cli_project_test.go b/cmd/agent-compose/cli_project_test.go index c80267390..d5a8abb84 100644 --- a/cmd/agent-compose/cli_project_test.go +++ b/cmd/agent-compose/cli_project_test.go @@ -279,7 +279,7 @@ agents: name: cli-up-demo workspaces: default: - provider: local + provider: file path: . agents: reviewer: diff --git a/cmd/agent-compose/cli_project_workflow_test.go b/cmd/agent-compose/cli_project_workflow_test.go index e462eedb5..a92b438db 100644 --- a/cmd/agent-compose/cli_project_workflow_test.go +++ b/cmd/agent-compose/cli_project_workflow_test.go @@ -59,6 +59,7 @@ agents: reviewer: scheduler: script: + provider: http url: %s/scheduler.js `, sourceServer.URL)) _, stderr, _, exitCode := executeCLICommand("up", "--file", composePath, "--host", daemon.URL) @@ -95,7 +96,7 @@ func testCLIWorkspaceRegistryConfigAndApply(t *testing.T) { name: workspace-default workspaces: repo-root: - provider: local + provider: file path: . agents: reviewer: @@ -126,7 +127,7 @@ agents: if err := json.Unmarshal([]byte(stdout), &decoded); err != nil { t.Fatalf("config json decode failed: %v\n%s", err, stdout) } - if len(decoded.Workspaces) != 1 || decoded.Workspaces[0].Key != "repo-root" || decoded.Workspaces[0].Provider != "local" { + if len(decoded.Workspaces) != 1 || decoded.Workspaces[0].Key != "repo-root" || decoded.Workspaces[0].Provider != "file" { t.Fatalf("decoded workspaces = %#v", decoded.Workspaces) } if len(decoded.Agents) != 1 || decoded.Agents[0].Workspace != nil { @@ -148,12 +149,12 @@ agents: name: workspace-reference workspaces: repo-root: - provider: local + provider: file path: . docs-repo: provider: git url: https://example.test/docs.git - path: docs + target: docs agents: reviewer: provider: codex @@ -186,7 +187,7 @@ agents: if len(decoded.Workspaces) != 2 || decoded.Workspaces[0].Key != "docs-repo" || decoded.Workspaces[1].Key != "repo-root" { t.Fatalf("decoded workspaces = %#v", decoded.Workspaces) } - if len(decoded.Agents) != 1 || decoded.Agents[0].Workspace.Provider != "local" || decoded.Agents[0].Workspace.Path != "." || decoded.Agents[0].Workspace.Name != "" { + if len(decoded.Agents) != 1 || decoded.Agents[0].Workspace.Provider != "file" || decoded.Agents[0].Workspace.Path != "." || decoded.Agents[0].Workspace.Name != "" { t.Fatalf("decoded agents = %#v", decoded.Agents) } @@ -205,7 +206,7 @@ agents: name: workspace-ambiguous workspaces: repo-root: - provider: local + provider: file path: . docs-repo: provider: git @@ -257,7 +258,7 @@ agents: name: workspace-missing-ref workspaces: repo-root: - provider: local + provider: file path: . agents: reviewer: @@ -464,7 +465,7 @@ func writeComposeFileNamed(t *testing.T, dir string, name string, content string } trimmed := strings.TrimSpace(content) if strings.HasPrefix(name, "agent-compose.") && !strings.Contains(trimmed, "\nworkspaces:") && !strings.HasPrefix(trimmed, "workspaces:") { - content = "workspaces:\n default:\n provider: local\n path: .\n" + content + content = "workspaces:\n default:\n provider: file\n path: .\n" + content } path := filepath.Join(dir, name) if err := os.WriteFile(path, []byte(content), 0o600); err != nil { diff --git a/cmd/agent-compose/cli_scheduler_test.go b/cmd/agent-compose/cli_scheduler_test.go index 6905086ee..690819f88 100644 --- a/cmd/agent-compose/cli_scheduler_test.go +++ b/cmd/agent-compose/cli_scheduler_test.go @@ -43,9 +43,9 @@ func TestConfigCommandExpandsSchedulerScriptURLs(t *testing.T) { if err := os.WriteFile(path, []byte(script), 0o600); err != nil { t.Fatal(err) } - return "./scripts/scheduler.js" + return "provider: file\n path: ./scripts/scheduler.js" }}, - {name: "HTTP", location: func(string) string { return httpServer.URL + "/scheduler.js" }}, + {name: "HTTP", location: func(string) string { return "provider: http\n url: " + httpServer.URL + "/scheduler.js" }}, } { t.Run(tc.name, func(t *testing.T) { dir := t.TempDir() @@ -55,7 +55,7 @@ agents: reviewer: scheduler: script: - url: %s + %s `, tc.location(dir))) stdout, stderr, runCount, err := executeCommand("config", "--file", composePath) if err != nil || stderr != "" || runCount != 0 { @@ -90,6 +90,7 @@ agents: reviewer: scheduler: script: + provider: http url: %s/scheduler.js `, sourceServer.URL)) _, expected, err := loadResolvedNormalizedCompose(context.Background(), cliOptions{ComposeFile: composePath}) @@ -129,6 +130,7 @@ agents: reviewer: scheduler: script: + provider: http url: http://127.0.0.1:1/unreachable.js `) _, stderr, _, exitCode := executeCLICommand("down", "--file", composePath, "--host", daemon.URL, "--json") diff --git a/cmd/agent-compose/e2e_docker_scheduler_test.go b/cmd/agent-compose/e2e_docker_scheduler_test.go index 799f1ed5b..294e63b08 100644 --- a/cmd/agent-compose/e2e_docker_scheduler_test.go +++ b/cmd/agent-compose/e2e_docker_scheduler_test.go @@ -93,7 +93,7 @@ scheduler.cron("hello-every-minute", "*/1 * * * *", async function() { name: e2e-docker-script workspaces: default: - provider: local + provider: file path: . agents: hello: @@ -103,7 +103,8 @@ agents: docker: {} scheduler: script: - url: ./scripts/hello-scheduler.js + provider: file + path: ./scripts/hello-scheduler.js `, guestImage)) stdout, stderr, _, exitCode := executeCLICommand("up", "--file", composePath) diff --git a/docs/design/agent-compose_design.md b/docs/design/agent-compose_design.md index ddfd9737a..ebcce0e98 100644 --- a/docs/design/agent-compose_design.md +++ b/docs/design/agent-compose_design.md @@ -155,7 +155,8 @@ workspaces: default: provider: git url: https://github.com/org/repo.git - branch: main + ref: main + target: . agents: reviewer: diff --git a/docs/pages/agent-compose-yaml-manual.md b/docs/pages/agent-compose-yaml-manual.md index 3d754cae0..114844d1b 100644 --- a/docs/pages/agent-compose-yaml-manual.md +++ b/docs/pages/agent-compose-yaml-manual.md @@ -32,13 +32,13 @@ variables: workspaces: source: - provider: local + provider: file path: . upstream: provider: git url: https://github.com/example/project.git - branch: main - path: . + ref: main + target: . mcp_servers: local-tools: @@ -107,7 +107,7 @@ agents: skills: - ./skills/review - name: release-check - source: git + provider: git url: https://github.com/example/agent-skills.git path: skills/release-check ref: main @@ -240,7 +240,7 @@ The top-level key must be plural: ```yaml workspaces: source: - provider: local + provider: file path: . ``` @@ -249,7 +249,7 @@ The old singular top-level form is invalid: ```yaml # Invalid: strict parsing rejects top-level workspace. workspace: - provider: local + provider: file path: . ``` @@ -258,23 +258,27 @@ Each `workspaces.` accepts: | Field | Type | Applicability | Purpose | | --- | --- | --- | --- | | `name` | string | Compatibility field | The map key is the effective project workspace name. Normally omit this redundant field. | -| `provider` | string | Required | `local` or `git`. | -| `url` | string | Required for `git` | Git clone URL. It is forbidden for `local`. | -| `branch` | string | Optional for `git` | Branch to check out. It is forbidden for `local`. | -| `path` | string | Provider-specific | For `local`, a required path relative to the compose directory that cannot escape the project root. For `git`, a clone target/subpath that defaults to `.` and cannot escape the workspace root. | +| `provider` | string | Required | `file` or `git`. | +| `url` | string | Required for `git` | Git clone URL. It is forbidden for `file`. | +| `ref` | string | Optional for `git` | Git branch, tag, or commit. | +| `path` | string | Required for `file` | Source path relative to the compose directory; it cannot escape the project root. Git workspaces do not support a repository subpath. | +| `target` | string | Optional | Destination below the sandbox workspace root. Defaults to `.`. | +| `username` | string | Optional for `git` | Git HTTP username. | +| `password` | string | Optional for `git` | Git password as an exact environment reference such as `${NAME}`. | +| `token` | string | Optional for `git` | Git token as an exact environment reference such as `${NAME}`. | A local workspace is copied into an isolated snapshot for each project run. Agent changes to that snapshot do not modify the source directory. ```yaml workspaces: source: - provider: local + provider: file path: ./src release-branch: provider: git url: https://github.com/example/service.git - branch: release - path: . + ref: release + target: . ``` Workspace selection follows these rules: @@ -375,6 +379,8 @@ agents: | Field | Type | Default | Purpose | | --- | --- | --- | --- | | `status` | string | Enabled semantics | `enabled` or `disabled`. An empty value is treated as enabled. A disabled definition remains stored but cannot run normally, and its scheduler is not enabled. | +| `display_name` | string | Empty | Human-readable agent label. | +| `description` | string | Empty | Human-readable explanation of the agent's role. | | `provider` | string | `codex` | Agent provider: `codex`, `claude`, `gemini`, or `opencode`. Compatibility aliases are normalized at persistence boundaries. | | `model` | string | Provider/daemon default | Model name. Supports `${NAME}` interpolation. | | `system_prompt` | string | Empty | Additional system instructions; YAML block scalars are recommended for multiline text. | @@ -562,21 +568,14 @@ Empty and duplicate entries are removed. When the capability gateway is configur ### `skills` -A resolved skill directory must contain a valid `SKILL.md`. Sources can be `file`, `git`, or `zip`; final skill names must be unique within an agent. - -Local directory shorthand: - -```yaml -skills: - - ./skills/review -``` +A resolved skill directory must contain a valid `SKILL.md`. Providers can be `file`, `http`, or `git`; final skill names must be unique within an agent. ZIP is a content format, not a provider. -Equivalent full form: +Local directory: ```yaml skills: - name: review - source: file + provider: file path: ./skills/review ``` @@ -585,7 +584,7 @@ Git source: ```yaml skills: - name: review - source: git + provider: git url: https://github.com/example/skills.git path: review ref: v1.0.0 @@ -593,37 +592,33 @@ skills: token: ${GIT_TOKEN} ``` -GitHub shorthand: - -```yaml -skills: - - source: git - url: github:example/skills//review@v1.0.0 -``` - -ZIP source: +Remote ZIP: ```yaml skills: - name: review - source: zip + provider: http url: https://downloads.example.com/review.zip + format: zip path: review ``` | Field | Type | Purpose | | --- | --- | --- | | `name` | string | Skill name. If omitted, it is inferred from path/URL and must end as a stable identifier. | -| `source` | string | `file`, `git`, or `zip`. Most local paths, `.git` URLs, and `.zip` locations can be inferred. Plain HTTP URLs are ambiguous, so set it explicitly. | -| `url` | string | Git remote or ZIP URL/local archive location. | -| `path` | string | Local directory for `file`; artifact subdirectory for Git or remote ZIP. Relative local paths use the compose directory. | -| `ref` | string | Git branch, tag, or ref. | -| `username` | string | Git HTTP username; interpolation is supported. | -| `password` | string | Git password. Only an exact environment reference such as `${NAME}` is allowed. | -| `token` | string | Git token. Only an exact environment reference such as `${NAME}` is allowed. | +| `provider` | string | Required source provider: `file`, `http`, or `git`. | +| `url` | string | Required for `http` and `git`. | +| `path` | string | Local path for `file`; content subdirectory for Git or ZIP. Relative file paths use the compose directory. | +| `ref` | string | Git branch, tag, or commit. | +| `format` | string | Optional content format. Currently only `zip` is supported; HTTP skills require it. | +| `username` | string | HTTP/Git username; interpolation is supported. | +| `password` | string | HTTP/Git password. Only an exact environment reference such as `${NAME}` is allowed. | +| `token` | string | HTTP/Git token. Only an exact environment reference such as `${NAME}` is allowed. | `password` and `token` cannot contain plaintext. They are resolved against the daemon environment during skill resolution, avoiding expanded credentials in the project specification. Remote ZIP downloads are restricted to HTTP(S) and are subject to size, archive, and network-address safety checks. +Git refs are resolved at each business lifecycle: skills during an agent run, workspaces during sandbox provisioning, and scheduler sources during `config`/`up` before the script snapshot is stored. A moving branch can therefore resolve to different commits across those operations. Use a commit SHA in `ref` when all consumers must use the exact same revision. + ### `volumes` The short form is `source:target[:ro|rw]`: @@ -672,7 +667,7 @@ Define an inline local workspace: ```yaml workspace: - provider: local + provider: file path: ./src ``` @@ -682,11 +677,11 @@ Define an inline Git workspace: workspace: provider: git url: https://github.com/example/project.git - branch: main - path: . + ref: main + target: . ``` -If `name` is combined with any of `provider`, `url`, `branch`, or `path`, the object is treated as an inline workspace rather than an inherited project workspace with overrides. To reuse a project entry, set only `name`. +If `name` is combined with any source field or `target`, the object is treated as an inline workspace rather than an inherited project workspace with overrides. To reuse a project entry, set only `name`. ### `scheduler` @@ -697,7 +692,7 @@ A scheduler uses either declarative `triggers` or JavaScript `script`; the two f | `enabled` | bool | `true` | Enables this agent's scheduler. `status: disabled` also prevents it from being enabled. | | `sandbox_policy` | string | `new` | Scheduler default sandbox policy: `new` or `sticky`. | | `triggers` | list | Empty | Declarative triggers. | -| `script` | string/object | Empty | Inline JavaScript or an external `{url: ...}` source. Cannot coexist with `triggers`. | +| `script` | string/object | Empty | Inline JavaScript or a flat `file`/`http`/`git` source mapping. Cannot coexist with `triggers`. | `new` creates a new sandbox for scheduler calls. `sticky` allows the scheduler to bind and reuse a sandbox. A trigger-level `sandbox_policy` controls the generated agent call for that trigger. @@ -755,16 +750,17 @@ The scheduler runtime validates the script and derives registered triggers from scheduler: enabled: true script: - url: ./scheduler.js + provider: file + path: ./scheduler.js ``` -`url` accepts: +External script mappings use the same source keys as skills and workspaces: -- A relative or absolute local path. Relative paths use the compose directory. -- A `file:///absolute/path/scheduler.js` URL. -- An `http://` or `https://` URL. +- File: `provider: file` with `path`. Relative paths use the compose directory. +- HTTP: `provider: http` with `url` and optional authentication. +- Git: `provider: git` with `url`, optional `ref`, and the required repository-internal `path`. -When a project is applied, the CLI reads the script and stores a content snapshot in the project specification; the daemon does not fetch the source again later. Fetching uses a 10-second timeout, a 1 MiB limit, no more than five HTTP redirects, and UTF-8 validation. URL userinfo and HTTPS-to-HTTP redirect downgrades are rejected. +When a project is applied, the CLI reads the script and stores a content snapshot in the project specification; the daemon does not fetch the source again later. HTTP fetching uses a 10-second timeout, a 1 MiB limit, no more than five redirects, and UTF-8 validation. URL userinfo and HTTPS-to-HTTP redirect downgrades are rejected. ### `jupyter` @@ -802,7 +798,7 @@ Invalid: ```yaml workspace: - provider: local + provider: file path: . ``` @@ -811,7 +807,7 @@ Valid: ```yaml workspaces: source: - provider: local + provider: file path: . agents: diff --git a/docs/pages/zh-CN/agent-compose-yaml-manual.md b/docs/pages/zh-CN/agent-compose-yaml-manual.md index 052442f1c..eba68cf7e 100644 --- a/docs/pages/zh-CN/agent-compose-yaml-manual.md +++ b/docs/pages/zh-CN/agent-compose-yaml-manual.md @@ -32,13 +32,13 @@ variables: workspaces: source: - provider: local + provider: file path: . upstream: provider: git url: https://github.com/example/project.git - branch: main - path: . + ref: main + target: . mcp_servers: local-tools: @@ -106,7 +106,7 @@ agents: skills: - ./skills/review - name: release-check - source: git + provider: git url: https://github.com/example/agent-skills.git path: skills/release-check ref: main @@ -239,7 +239,7 @@ variables: ```yaml workspaces: source: - provider: local + provider: file path: . ``` @@ -248,7 +248,7 @@ workspaces: ```yaml # 错误:顶层 workspace 会被严格解析器拒绝 workspace: - provider: local + provider: file path: . ``` @@ -257,23 +257,27 @@ workspace: | 字段 | 类型 | 适用范围 | 作用 | | --- | --- | --- | --- | | `name` | string | 兼容字段 | 顶层条目的实际名称由 map key 决定;通常不要重复填写。 | -| `provider` | string | 必填 | `local` 或 `git`。 | -| `url` | string | `git` 必填 | Git clone URL;`local` 不允许设置。 | -| `branch` | string | `git` 可选 | 要检出的分支;`local` 不允许设置。 | -| `path` | string | 必填/可选 | `local` 时是配置文件目录下的相对源目录且不可逃逸;`git` 时是仓库内 clone target/subpath,默认 `.`。 | +| `provider` | string | 必填 | `file` 或 `git`。 | +| `url` | string | `git` 必填 | Git clone URL;`file` 不允许设置。 | +| `ref` | string | `git` 可选 | Git branch、tag 或 commit。 | +| `path` | string | `file` 必填 | 相对于 compose 文件目录的来源路径,不可逃逸项目根目录;Git Workspace 不支持仓库内子目录。 | +| `target` | string | 可选 | sandbox workspace 根目录下的目标目录,默认 `.`。 | +| `username` | string | `git` 可选 | Git HTTP 用户名。 | +| `password` | string | `git` 可选 | Git 密码,只允许完整环境引用 `${NAME}`。 | +| `token` | string | `git` 可选 | Git token,只允许完整环境引用 `${NAME}`。 | 本地 Workspace 在每次项目 run 创建时被复制为隔离快照,Agent 对快照的修改不会写回源目录。 ```yaml workspaces: source: - provider: local + provider: file path: ./src release-branch: provider: git url: https://github.com/example/service.git - branch: release - path: . + ref: release + target: . ``` Workspace 选择规则: @@ -376,6 +380,8 @@ agents: | 字段 | 类型 | 默认值 | 作用 | | --- | --- | --- | --- | | `status` | string | enabled 语义 | `enabled` 或 `disabled`。空值等同启用;禁用后定义保留但不可按正常流程运行,Scheduler 也不会启用。 | +| `display_name` | string | 空 | Agent 的可读显示名称。 | +| `description` | string | 空 | Agent 职责的可读说明。 | | `provider` | string | `codex` | Agent CLI/provider:`codex`、`claude`、`gemini` 或 `opencode`。兼容别名会在持久化边界归一化。 | | `model` | string | provider/daemon 默认 | 模型名;支持 `${NAME}` 插值。 | | `system_prompt` | string | 空 | 附加的系统提示,适合使用 YAML `|` 多行标量。 | @@ -567,21 +573,14 @@ capset_ids: ### `skills` -Skill 目录必须包含有效的 `SKILL.md`。支持 `file`、`git` 和 `zip` 来源;同一 Agent 的最终 Skill 名不可重复。 - -本地目录短写: - -```yaml -skills: - - ./skills/review -``` +Skill 目录必须包含有效的 `SKILL.md`。provider 支持 `file`、`http` 和 `git`;同一 Agent 的最终 Skill 名不可重复。ZIP 是内容格式,不是 provider。 -等价完整写法: +本地目录: ```yaml skills: - name: review - source: file + provider: file path: ./skills/review ``` @@ -590,7 +589,7 @@ Git 来源: ```yaml skills: - name: review - source: git + provider: git url: https://github.com/example/skills.git path: review ref: v1.0.0 @@ -598,37 +597,33 @@ skills: token: ${GIT_TOKEN} ``` -GitHub shorthand: - -```yaml -skills: - - source: git - url: github:example/skills//review@v1.0.0 -``` - -ZIP 来源: +远程 ZIP: ```yaml skills: - name: review - source: zip + provider: http url: https://downloads.example.com/review.zip + format: zip path: review ``` | 字段 | 类型 | 作用 | | --- | --- | --- | | `name` | string | Skill 名;省略时从 path/URL 推导,最终必须符合稳定标识符格式。 | -| `source` | string | `file`、`git` 或 `zip`;多数本地路径、`.git`、`.zip` 可自动推断。普通 HTTP URL 无法唯一推断,建议显式填写。 | -| `url` | string | Git remote 或 ZIP URL/本地归档位置。 | -| `path` | string | `file` 的本地目录;Git/远程 ZIP 的制品内子目录。相对本地路径以 compose 文件目录为基准。 | -| `ref` | string | Git branch、tag 或 ref。 | -| `username` | string | Git HTTP 用户名,可插值。 | -| `password` | string | Git 密码,只允许完整环境引用 `${NAME}`。 | -| `token` | string | Git token,只允许完整环境引用 `${NAME}`。 | +| `provider` | string | 必填,支持 `file`、`http` 或 `git`。 | +| `url` | string | `http` 和 `git` 必填。 | +| `path` | string | `file` 的本地路径;Git 或 ZIP 内容内的子目录。相对 file 路径以 compose 文件目录为基准。 | +| `ref` | string | Git branch、tag 或 commit。 | +| `format` | string | 可选内容格式,目前只支持 `zip`;HTTP Skill 必须设置。 | +| `username` | string | HTTP/Git 用户名,可插值。 | +| `password` | string | HTTP/Git 密码,只允许完整环境引用 `${NAME}`。 | +| `token` | string | HTTP/Git token,只允许完整环境引用 `${NAME}`。 | `password` 和 `token` 不允许明文。它们在 Skill 解析阶段通过 daemon 环境解析,避免把凭证展开后写入项目规范。远程 ZIP 下载限制为 HTTP(S),并执行大小、压缩包和网络地址安全检查。 +Git ref 会在各自业务生命周期中解析:Skill 在 Agent run 时解析,Workspace 在 sandbox provisioning 时解析,Scheduler 来源在 `config`/`up` 时解析并保存脚本快照。因此 moving branch 在三处可能得到不同 commit;需要严格一致时,应在 `ref` 中直接填写 commit SHA。 + ### `volumes` 短写格式是 `source:target[:ro|rw]`: @@ -677,7 +672,7 @@ workspace: ```yaml workspace: - provider: local + provider: file path: ./src ``` @@ -687,11 +682,11 @@ workspace: workspace: provider: git url: https://github.com/example/project.git - branch: main - path: . + ref: main + target: . ``` -若 `name` 与 `provider`/`url`/`branch`/`path` 同时出现,该对象按内联 Workspace 处理,而不是从顶层继承后局部覆盖。需要复用时只写 `name`。 +若 `name` 与任一来源字段或 `target` 同时出现,该对象按内联 Workspace 处理,而不是从顶层继承后局部覆盖。需要复用时只写 `name`。 ### `scheduler` @@ -702,7 +697,7 @@ Scheduler 可以使用声明式 `triggers`,也可以使用 JavaScript `script` | `enabled` | bool | `true` | 是否启用该 Agent 的 Scheduler。Agent `status: disabled` 也会使其无效。 | | `sandbox_policy` | string | `new` | Scheduler 默认 sandbox 策略:`new` 或 `sticky`。 | | `triggers` | list | 空 | 声明式触发器。 | -| `script` | string/object | 空 | 内联 JavaScript,或 `{url: ...}` 外部脚本来源。不能和 `triggers` 同时使用。 | +| `script` | string/object | 空 | 内联 JavaScript,或扁平的 `file`/`http`/`git` 来源配置。不能和 `triggers` 同时使用。 | `new` 为每次调用创建新 sandbox;`sticky` 允许 Scheduler 绑定并复用 sandbox。单个 Trigger 的 `sandbox_policy` 可覆盖执行 Agent 时的策略。 @@ -760,16 +755,17 @@ scheduler: scheduler: enabled: true script: - url: ./scheduler.js + provider: file + path: ./scheduler.js ``` -`url` 支持: +外部脚本 mapping 使用与 Skill、Workspace 相同的来源字段: -- 相对或绝对本地路径;相对路径以 compose 文件目录为基准。 -- `file:///absolute/path/scheduler.js`。 -- `http://` 或 `https://` URL。 +- File:`provider: file` 配合 `path`,相对路径以 compose 文件目录为基准。 +- HTTP:`provider: http` 配合 `url`,可选认证字段。 +- Git:`provider: git` 配合 `url`、可选 `ref` 和必填的仓库内 `path`。 -应用项目时 CLI 会读取脚本并将内容快照保存到项目规范,而不是让 daemon 以后重新读取来源。读取限制包括 10 秒超时、最大 1 MiB、最多 5 次 HTTP redirect、UTF-8 校验;HTTPS 不允许降级 redirect 到 HTTP,URL userinfo 不允许使用。 +应用项目时 CLI 会读取脚本并将内容快照保存到项目规范,而不是让 daemon 以后重新读取来源。HTTP 读取限制包括 10 秒超时、最大 1 MiB、最多 5 次 redirect、UTF-8 校验;HTTPS 不允许降级 redirect 到 HTTP,URL userinfo 不允许使用。 ### `jupyter` @@ -807,7 +803,7 @@ network: ```yaml workspace: - provider: local + provider: file path: . ``` @@ -816,7 +812,7 @@ workspace: ```yaml workspaces: source: - provider: local + provider: file path: . agents: diff --git a/examples/agent-compose/README.md b/examples/agent-compose/README.md index e29006c1f..913048fb7 100644 --- a/examples/agent-compose/README.md +++ b/examples/agent-compose/README.md @@ -9,7 +9,7 @@ simplest to most complete. | --- | --- | --- | | [docker-minimal](docker-minimal/) | Smallest Docker-backed project: one agent, no scheduler. | No, for `config`/`up`/`ps` | | [docker-scheduler-cron](docker-scheduler-cron/) | Managed cron scheduler control plane. | No, for `config`/`up`/`ps`/`down` | -| [docker-scheduler-script-url](docker-scheduler-script-url/) | A scheduler script loaded from a relative file URL source. | No, for `config`/`up`/`ps`/`down` | +| [docker-scheduler-script-url](docker-scheduler-script-url/) | A scheduler script loaded from a relative `file` source. | No, for `config`/`up`/`ps`/`down` | | [docker-scheduler-timeout](docker-scheduler-timeout/) | End-to-end scheduled run that fires, executes the agent, and persists logs. | Yes, for the scheduled run | ## Common prerequisites diff --git a/examples/agent-compose/README.zh-CN.md b/examples/agent-compose/README.zh-CN.md index c908f5f33..4d2cd13fc 100644 --- a/examples/agent-compose/README.zh-CN.md +++ b/examples/agent-compose/README.zh-CN.md @@ -8,7 +8,7 @@ | --- | --- | --- | | [docker-minimal](docker-minimal/) | 最小的 Docker project:一个 agent,不启用 scheduler。 | `config`/`up`/`ps` 不需要 | | [docker-scheduler-cron](docker-scheduler-cron/) | managed cron scheduler 的控制面流程。 | `config`/`up`/`ps`/`down` 不需要 | -| [docker-scheduler-script-url](docker-scheduler-script-url/) | 从相对文件 URL 来源加载 scheduler 脚本。 | `config`/`up`/`ps`/`down` 不需要 | +| [docker-scheduler-script-url](docker-scheduler-script-url/) | 从相对 `file` 来源加载 scheduler 脚本。 | `config`/`up`/`ps`/`down` 不需要 | | [docker-scheduler-timeout](docker-scheduler-timeout/) | 端到端的定时运行:触发、执行 agent 并持久化日志。 | 定时运行需要 | ## 通用前置条件 diff --git a/examples/agent-compose/docker-scheduler-script-url/README.md b/examples/agent-compose/docker-scheduler-script-url/README.md index faac9b615..d25577d97 100644 --- a/examples/agent-compose/docker-scheduler-script-url/README.md +++ b/examples/agent-compose/docker-scheduler-script-url/README.md @@ -1,9 +1,9 @@ -# Scheduler script URL example +# Scheduler script file-source example Languages: English | [中文](README.zh-CN.md) This example keeps QJS in `scheduler.js` and references it from -`agent-compose.yml` with `scheduler.script.url`. +`agent-compose.yml` with `scheduler.script.provider: file` and `path`. ```bash agent-compose config diff --git a/examples/agent-compose/docker-scheduler-script-url/README.zh-CN.md b/examples/agent-compose/docker-scheduler-script-url/README.zh-CN.md index 8cbaaeeae..0f3f229fa 100644 --- a/examples/agent-compose/docker-scheduler-script-url/README.zh-CN.md +++ b/examples/agent-compose/docker-scheduler-script-url/README.zh-CN.md @@ -1,9 +1,9 @@ -# Scheduler 脚本 URL 示例 +# Scheduler 脚本 file 来源示例 语言:[English](README.md) | 中文 本示例把 QJS 保存在 `scheduler.js`,并在 `agent-compose.yml` 中通过 -`scheduler.script.url` 引用。 +`scheduler.script.provider: file` 和 `path` 引用。 ```bash agent-compose config diff --git a/examples/agent-compose/docker-scheduler-script-url/agent-compose.yml b/examples/agent-compose/docker-scheduler-script-url/agent-compose.yml index 28d4ec9c5..ab8d4e65f 100644 --- a/examples/agent-compose/docker-scheduler-script-url/agent-compose.yml +++ b/examples/agent-compose/docker-scheduler-script-url/agent-compose.yml @@ -9,4 +9,5 @@ agents: scheduler: enabled: true script: - url: ./scheduler.js + provider: file + path: ./scheduler.js diff --git a/pkg/agentcompose/adapters/agent_runner_test.go b/pkg/agentcompose/adapters/agent_runner_test.go index ca77baed1..3dd69d959 100644 --- a/pkg/agentcompose/adapters/agent_runner_test.go +++ b/pkg/agentcompose/adapters/agent_runner_test.go @@ -102,7 +102,7 @@ func TestAgentRunnerExecuteAgentRunWritesSystemPromptAndParsesResult(t *testing. runner := NewAgentRunner(config, store, nil, fakeAgentDefinitionStore{agent: domain.AgentDefinition{ ID: "agent-1", SystemPrompt: "Reply only in Chinese", - Skills: []domain.AgentSkill{{Name: "pdf", Source: "file", Path: skillSource}}, + Skills: []domain.AgentSkill{{Name: "pdf", Provider: "file", Path: skillSource}}, }}, fakeRuntimeProvider{runtime: runtime}) result, parsed, err := runner.ExecuteAgentRun(ctx, session, "codex", "agent-1", "", "", "hello", "", nil) diff --git a/pkg/agentcompose/adapters/session_rpc_bridge_workspace_failure_retry_integration_test.go b/pkg/agentcompose/adapters/session_rpc_bridge_workspace_failure_retry_integration_test.go index 594a69159..8eaa96557 100644 --- a/pkg/agentcompose/adapters/session_rpc_bridge_workspace_failure_retry_integration_test.go +++ b/pkg/agentcompose/adapters/session_rpc_bridge_workspace_failure_retry_integration_test.go @@ -16,6 +16,7 @@ import ( testutil "agent-compose/pkg/internal/testutil" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" "agent-compose/pkg/storage/sessionstore" "agent-compose/pkg/workspaces" ) @@ -104,8 +105,7 @@ func TestIntegrationSandboxRPCBridgeGitWorkspaceProvisioningFailureRetry(t *test const workspaceID = "git-provisioning-failure-retry" sourceRepo := filepath.Join(t.TempDir(), "source.git") configJSON, err := json.Marshal(workspaces.GitWorkspaceConfig{ - URL: "file://" + filepath.ToSlash(sourceRepo), - Branch: "main", + Source: sources.Source{Provider: sources.ProviderGit, URL: "file://" + filepath.ToSlash(sourceRepo), Ref: "main"}, }) if err != nil { t.Fatalf("marshal Git workspace config: %v", err) diff --git a/pkg/agentcompose/api/project.go b/pkg/agentcompose/api/project.go index fa1531f9a..43372b012 100644 --- a/pkg/agentcompose/api/project.go +++ b/pkg/agentcompose/api/project.go @@ -341,10 +341,11 @@ func SkillSpecsToProto(skills []compose.NormalizedSkillSpec) []*agentcomposev2.S for _, skill := range skills { items = append(items, &agentcomposev2.SkillSpec{ Name: skill.Name, - Source: skill.Source, + Provider: skill.Provider, Url: skill.URL, Path: skill.Path, Ref: skill.Ref, + Format: skill.Format, Username: skill.Username, Password: skill.Password, Token: skill.Token, @@ -435,9 +436,13 @@ func WorkspaceSpecToProto(workspace *compose.WorkspaceSpec) *agentcomposev2.Work Name: workspace.Name, Provider: workspace.Provider, Url: workspace.URL, - Branch: workspace.Branch, - Commit: workspace.Commit, + Ref: workspace.Ref, Path: workspace.Path, + Format: workspace.Format, + Target: workspace.Target, + Username: workspace.Username, + Password: workspace.Password, + Token: workspace.Token, } } @@ -760,8 +765,8 @@ func SkillYAMLList(path string, skills []*agentcomposev2.SkillSpec) ([]map[strin if name != "" { raw["name"] = name } - if strings.TrimSpace(skill.GetSource()) != "" { - raw["source"] = skill.GetSource() + if strings.TrimSpace(skill.GetProvider()) != "" { + raw["provider"] = skill.GetProvider() } if strings.TrimSpace(skill.GetUrl()) != "" { raw["url"] = skill.GetUrl() @@ -772,6 +777,9 @@ func SkillYAMLList(path string, skills []*agentcomposev2.SkillSpec) ([]map[strin if strings.TrimSpace(skill.GetRef()) != "" { raw["ref"] = skill.GetRef() } + if strings.TrimSpace(skill.GetFormat()) != "" { + raw["format"] = skill.GetFormat() + } if strings.TrimSpace(skill.GetUsername()) != "" { raw["username"] = skill.GetUsername() } @@ -983,15 +991,27 @@ func WorkspaceYAMLShape(workspace *agentcomposev2.WorkspaceSpec) map[string]any if strings.TrimSpace(workspace.GetUrl()) != "" { raw["url"] = workspace.GetUrl() } - if strings.TrimSpace(workspace.GetBranch()) != "" { - raw["branch"] = workspace.GetBranch() - } - if strings.TrimSpace(workspace.GetCommit()) != "" { - raw["commit"] = workspace.GetCommit() + if strings.TrimSpace(workspace.GetRef()) != "" { + raw["ref"] = workspace.GetRef() } if strings.TrimSpace(workspace.GetPath()) != "" { raw["path"] = workspace.GetPath() } + if strings.TrimSpace(workspace.GetFormat()) != "" { + raw["format"] = workspace.GetFormat() + } + if strings.TrimSpace(workspace.GetTarget()) != "" { + raw["target"] = workspace.GetTarget() + } + if strings.TrimSpace(workspace.GetUsername()) != "" { + raw["username"] = workspace.GetUsername() + } + if strings.TrimSpace(workspace.GetPassword()) != "" { + raw["password"] = workspace.GetPassword() + } + if strings.TrimSpace(workspace.GetToken()) != "" { + raw["token"] = workspace.GetToken() + } return raw } diff --git a/pkg/agentcompose/api/project_test.go b/pkg/agentcompose/api/project_test.go index 7d89a76cd..2bc15c4e7 100644 --- a/pkg/agentcompose/api/project_test.go +++ b/pkg/agentcompose/api/project_test.go @@ -8,6 +8,7 @@ import ( "agent-compose/pkg/compose" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" agentcomposev2 "agent-compose/proto/agentcompose/v2" "gopkg.in/yaml.v3" ) @@ -42,7 +43,7 @@ func TestResolvedTriggerPreservesDeclaredSpec(t *testing.T) { } func TestProjectSpecToProtoRejectsUnresolvedSchedulerScriptURL(t *testing.T) { - raw, err := compose.Parse([]byte("name: unresolved-url\nagents:\n reviewer:\n scheduler:\n script:\n url: ./scheduler.js\n")) + raw, err := compose.Parse([]byte("name: unresolved-url\nagents:\n reviewer:\n scheduler:\n script:\n provider: file\n path: ./scheduler.js\n")) if err != nil { t.Fatal(err) } @@ -68,10 +69,10 @@ func TestProjectSpecToProtoURLSnapshotMatchesInline(t *testing.T) { if err != nil { t.Fatal(err) } - urlRaw, _ := compose.Parse([]byte("name: proto-snapshot\nagents:\n reviewer:\n scheduler:\n script:\n url: https://example.test/scheduler.js\n")) + urlRaw, _ := compose.Parse([]byte("name: proto-snapshot\nagents:\n reviewer:\n scheduler:\n script:\n provider: http\n url: https://example.test/scheduler.js\n")) fromURL, err := compose.Normalize(urlRaw, compose.NormalizeOptions{ ResolveScriptURLs: true, - ScriptSourceResolver: compose.ScriptSourceResolverFunc(func(context.Context, string) ([]byte, error) { + ScriptSourceResolver: compose.ScriptSourceResolverFunc(func(context.Context, sources.Source) ([]byte, error) { return []byte(script), nil }), }) @@ -198,12 +199,12 @@ func TestIntegrationProjectSpecToProtoIncludesWorkspaceRegistry(t *testing.T) { spec := &compose.NormalizedProjectSpec{ Name: "workspace-registry", Workspaces: map[string]compose.WorkspaceSpec{ - "docs": {Name: "docs", Provider: "git", URL: "https://example.test/docs.git", Branch: "main", Commit: "abc123", Path: "docs"}, - "repo": {Name: "repo", Provider: "local", Path: "."}, + "docs": {Name: "docs", Provider: "git", URL: "https://example.test/docs.git", Ref: "abc123", Target: "docs"}, + "repo": {Name: "repo", Provider: "file", Path: "."}, }, Agents: []compose.NormalizedAgentSpec{{ Name: "reviewer", - Workspace: &compose.WorkspaceSpec{Provider: "local", Path: "."}, + Workspace: &compose.WorkspaceSpec{Provider: "file", Path: "."}, Driver: &compose.NormalizedDriverSpec{Name: compose.DriverDocker, Docker: &compose.DockerDriverSpec{}}, }}, } @@ -212,7 +213,7 @@ func TestIntegrationProjectSpecToProtoIncludesWorkspaceRegistry(t *testing.T) { if response == nil || len(response.GetWorkspaces()) != 2 { t.Fatalf("ProjectSpecToProto workspaces = %#v", response) } - if response.GetWorkspaces()[0].GetName() != "docs" || response.GetWorkspaces()[0].GetWorkspace().GetProvider() != "git" || response.GetWorkspaces()[0].GetWorkspace().GetBranch() != "main" || response.GetWorkspaces()[0].GetWorkspace().GetCommit() != "abc123" { + if response.GetWorkspaces()[0].GetName() != "docs" || response.GetWorkspaces()[0].GetWorkspace().GetProvider() != "git" || response.GetWorkspaces()[0].GetWorkspace().GetRef() != "abc123" { t.Fatalf("first workspace = %#v", response.GetWorkspaces()[0]) } if response.GetWorkspaces()[1].GetName() != "repo" || response.GetWorkspaces()[1].GetWorkspace().GetName() != "" { @@ -225,7 +226,7 @@ func TestIntegrationProjectSpecYAMLShapeIncludesWorkspaceRegistry(t *testing.T) Name: "workspace-shape", Workspaces: []*agentcomposev2.NamedWorkspaceSpec{{ Name: "repo", - Workspace: &agentcomposev2.WorkspaceSpec{Provider: "git", Url: "https://example.test/repo.git", Branch: "main", Commit: "abc123", Path: "."}, + Workspace: &agentcomposev2.WorkspaceSpec{Provider: "git", Url: "https://example.test/repo.git", Ref: "abc123", Target: "."}, }}, Agents: []*agentcomposev2.AgentSpec{{ Name: "reviewer", @@ -240,7 +241,7 @@ func TestIntegrationProjectSpecYAMLShapeIncludesWorkspaceRegistry(t *testing.T) t.Fatalf("workspaces shape = %#v", shape["workspaces"]) } repo, ok := workspaces["repo"].(map[string]any) - if !ok || repo["branch"] != "main" || repo["commit"] != "abc123" { + if !ok || repo["ref"] != "abc123" || repo["target"] != "." { t.Fatalf("repo workspace shape = %#v", workspaces["repo"]) } agents, ok := shape["agents"].(map[string]any) diff --git a/pkg/agentcompose/app/controller_helpers_coverage_test.go b/pkg/agentcompose/app/controller_helpers_coverage_test.go index ce3eb1dd5..a93e16837 100644 --- a/pkg/agentcompose/app/controller_helpers_coverage_test.go +++ b/pkg/agentcompose/app/controller_helpers_coverage_test.go @@ -28,7 +28,7 @@ func TestAppProjectControllerHelperCoverage(t *testing.T) { Name: "app-project", Workspaces: []*agentcomposev2.NamedWorkspaceSpec{{ Name: "default", - Workspace: &agentcomposev2.WorkspaceSpec{Provider: "local", Path: "."}, + Workspace: &agentcomposev2.WorkspaceSpec{Provider: "file", Path: "."}, }}, Agents: []*agentcomposev2.AgentSpec{{ Name: "worker", diff --git a/pkg/compose/coverage_shape_workflows_test.go b/pkg/compose/coverage_shape_workflows_test.go index 8f979a0f6..c49a9daba 100644 --- a/pkg/compose/coverage_shape_workflows_test.go +++ b/pkg/compose/coverage_shape_workflows_test.go @@ -38,8 +38,8 @@ func TestIntegrationComposeParseNormalizeAndOutputWorkflows(t *testing.T) { t.Run("parse minimal", TestParseMinimalSpec) t.Run("parse full", TestParseFullSpec) t.Run("parse scheduler script", TestParseSchedulerScript) - t.Run("parse scheduler script URL", TestParseSchedulerScriptURL) - t.Run("parse invalid scheduler script URL", TestParseRejectsInvalidSchedulerScriptURLObject) + t.Run("parse scheduler script provider", TestParseSchedulerScriptProvider) + t.Run("parse invalid scheduler script provider", TestParseRejectsInvalidSchedulerScriptProviderObject) t.Run("parse unknown field", TestParseUnknownFieldIncludesPath) t.Run("parse invalid yaml", TestParseInvalidYAML) t.Run("parse type error", TestParseTypeErrorIncludesPath) diff --git a/pkg/compose/helper_coverage_test.go b/pkg/compose/helper_coverage_test.go index 8ef010568..1eea82dec 100644 --- a/pkg/compose/helper_coverage_test.go +++ b/pkg/compose/helper_coverage_test.go @@ -17,8 +17,8 @@ func TestComposePrivateHelperCoverage(t *testing.T) { t.Fatalf("cloneStringMap aliased input: %#v", values) } - workspace := cloneWorkspaceSpec(&WorkspaceSpec{Provider: " git ", URL: " https://example.test/repo.git ", Branch: " main ", Commit: " abc123 ", Path: " app "}) - if workspace.Provider != "git" || workspace.URL != "https://example.test/repo.git" || workspace.Branch != "main" || workspace.Commit != "abc123" || workspace.Path != "app" { + workspace := cloneWorkspaceSpec(&WorkspaceSpec{Provider: " git ", URL: " https://example.test/repo.git ", Ref: " abc123 ", Target: " app "}) + if workspace.Provider != "git" || workspace.URL != "https://example.test/repo.git" || workspace.Ref != "abc123" || workspace.Target != "app" { t.Fatalf("cloneWorkspaceSpec = %#v", workspace) } if cloneWorkspaceSpec(nil) != nil { diff --git a/pkg/compose/normalize.go b/pkg/compose/normalize.go index 5f0118e47..83ec1ff3f 100644 --- a/pkg/compose/normalize.go +++ b/pkg/compose/normalize.go @@ -3,6 +3,7 @@ package compose import ( "context" "fmt" + "net/url" "os" "path/filepath" "regexp" @@ -11,6 +12,8 @@ import ( "time" "unicode/utf8" + "agent-compose/pkg/sources" + "github.com/robfig/cron/v3" ) @@ -24,7 +27,6 @@ const ( var stableIdentifierPattern = regexp.MustCompile(`^[a-z][a-z0-9_-]*$`) var volumeSourceNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]*$`) var envReferencePattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)\}`) -var exactEnvReferencePattern = regexp.MustCompile(`^\$\{[A-Za-z_][A-Za-z0-9_]*\}$`) var composeCronParser = cron.NewParser(cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor) type NormalizeOptions struct { @@ -79,10 +81,11 @@ type NormalizedMCPServerSpec struct { type NormalizedSkillSpec struct { Name string `yaml:"name,omitempty" json:"name,omitempty"` - Source string `yaml:"source,omitempty" json:"source,omitempty"` + Provider string `yaml:"provider,omitempty" json:"provider,omitempty"` URL string `yaml:"url,omitempty" json:"url,omitempty"` - Path string `yaml:"path,omitempty" json:"path,omitempty"` Ref string `yaml:"ref,omitempty" json:"ref,omitempty"` + Path string `yaml:"path,omitempty" json:"path,omitempty"` + Format string `yaml:"format,omitempty" json:"format,omitempty"` Username string `yaml:"username,omitempty" json:"username,omitempty"` Password string `yaml:"password,omitempty" json:"password,omitempty"` Token string `yaml:"token,omitempty" json:"token,omitempty"` @@ -129,17 +132,17 @@ type NormalizedSchedulerSpec struct { Script string `yaml:"script,omitempty" json:"script,omitempty"` Triggers []NormalizedTriggerSpec `yaml:"triggers,omitempty" json:"triggers,omitempty"` - scriptURL string + scriptSource *sources.Source } // HasScript reports whether a scheduler has either an inline script snapshot // or an unresolved URL source. func (s *NormalizedSchedulerSpec) HasScript() bool { - return s != nil && (strings.TrimSpace(s.Script) != "" || s.scriptURL != "") + return s != nil && (strings.TrimSpace(s.Script) != "" || s.scriptSource != nil) } -func (s *NormalizedSchedulerSpec) hasUnresolvedScriptURL() bool { - return s != nil && s.scriptURL != "" +func (s *NormalizedSchedulerSpec) hasUnresolvedScriptSource() bool { + return s != nil && s.scriptSource != nil } type NormalizedTriggerSpec struct { @@ -364,7 +367,7 @@ func resolveAgentWorkspace(path string, spec *WorkspaceSpec, globals map[string] } trimmed := cloneWorkspaceSpec(spec) hasName := trimmed.Name != "" - hasInline := trimmed.Provider != "" || trimmed.URL != "" || trimmed.Branch != "" || trimmed.Commit != "" || trimmed.Path != "" + hasInline := workspaceSource(*trimmed).HasContent() || trimmed.Target != "" switch { case hasName && !hasInline: workspace, ok := globals[trimmed.Name] @@ -391,44 +394,69 @@ func normalizeInlineWorkspaceSpec(path string, spec *WorkspaceSpec, defaultName if provider == "" { return nil, &ValidationError{Path: path + ".provider", Message: "workspace provider is required"} } + normalizedSource := workspaceSource(*workspace).Normalized() + applyWorkspaceSource(workspace, normalizedSource) workspace.Provider = provider workspace.Name = defaultName + if err := validateSourceSecrets(path, normalizedSource); err != nil { + return nil, err + } switch provider { - case "local": + case sources.ProviderFile: if strings.TrimSpace(workspace.URL) != "" { - return nil, &ValidationError{Path: path + ".url", Message: "local workspace does not support url"} + return nil, &ValidationError{Path: path + ".url", Message: "file workspace does not support url"} } - if strings.TrimSpace(workspace.Branch) != "" { - return nil, &ValidationError{Path: path + ".branch", Message: "local workspace does not support branch"} + if strings.TrimSpace(workspace.Ref) != "" { + return nil, &ValidationError{Path: path + ".ref", Message: "file workspace does not support ref"} } - if strings.TrimSpace(workspace.Commit) != "" { - return nil, &ValidationError{Path: path + ".commit", Message: "local workspace does not support commit"} + if strings.TrimSpace(workspace.Format) != "" { + return nil, &ValidationError{Path: path + ".format", Message: "file workspace does not support format"} } - if _, err := cleanComposeLocalWorkspacePath(workspace.Path); err != nil { + if normalizedSource.HasAuthentication() { + return nil, &ValidationError{Path: path, Message: "file workspace does not support authentication"} + } + cleanPath, err := cleanComposeLocalWorkspacePath(workspace.Path) + if err != nil { return nil, &ValidationError{Path: path + ".path", Message: err.Error()} } - case "git": + workspace.Path = cleanPath + case sources.ProviderGit: if strings.TrimSpace(workspace.URL) == "" { return nil, &ValidationError{Path: path + ".url", Message: "git workspace url is required"} } if strings.TrimSpace(workspace.Path) != "" { - cleanPath := filepath.Clean(workspace.Path) - if filepath.IsAbs(workspace.Path) || cleanPath == ".." || strings.HasPrefix(cleanPath, ".."+string(filepath.Separator)) { - return nil, &ValidationError{Path: path + ".path", Message: fmt.Sprintf("git workspace path %q must stay within workspace root", workspace.Path)} - } - workspace.Path = cleanPath - } else { - workspace.Path = "." + return nil, &ValidationError{Path: path + ".path", Message: "git workspace does not support a source path"} + } + if strings.TrimSpace(workspace.Format) != "" { + return nil, &ValidationError{Path: path + ".format", Message: "git workspace does not support format"} } workspace.URL = strings.TrimSpace(workspace.URL) - workspace.Branch = strings.TrimSpace(workspace.Branch) - workspace.Commit = strings.TrimSpace(workspace.Commit) default: return nil, &ValidationError{Path: path + ".provider", Message: fmt.Sprintf("unsupported workspace provider %q", workspace.Provider)} } + target, err := cleanWorkspaceTarget(workspace.Target) + if err != nil { + return nil, &ValidationError{Path: path + ".target", Message: err.Error()} + } + workspace.Target = target return workspace, nil } +func cleanWorkspaceTarget(raw string) (string, error) { + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + return ".", nil + } + if filepath.IsAbs(trimmed) { + return "", fmt.Errorf("workspace target %q must be relative", trimmed) + } + clean := filepath.Clean(trimmed) + if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("workspace target %q must stay within workspace root", trimmed) + } + return clean, nil +} + func cleanComposeLocalWorkspacePath(raw string) (string, error) { trimmed := strings.TrimSpace(raw) if trimmed == "" { @@ -572,11 +600,11 @@ func normalizeSkillSpec(path string, value SkillSpec, options NormalizeOptions) if err != nil { return NormalizedSkillSpec{}, err } - source, err := interpolateEnvValue(path+".source", strings.TrimSpace(value.Source), options) + provider, err := interpolateEnvValue(path+".provider", strings.TrimSpace(value.Provider), options) if err != nil { return NormalizedSkillSpec{}, err } - source = strings.ToLower(strings.TrimSpace(source)) + provider = strings.ToLower(strings.TrimSpace(provider)) urlValue, err := interpolateEnvValue(path+".url", strings.TrimSpace(value.URL), options) if err != nil { return NormalizedSkillSpec{}, err @@ -589,6 +617,11 @@ func normalizeSkillSpec(path string, value SkillSpec, options NormalizeOptions) if err != nil { return NormalizedSkillSpec{}, err } + format, err := interpolateEnvValue(path+".format", strings.TrimSpace(value.Format), options) + if err != nil { + return NormalizedSkillSpec{}, err + } + format = strings.ToLower(strings.TrimSpace(format)) username := strings.TrimSpace(value.Username) if username != "" { username, err = interpolateEnvValue(path+".username", username, options) @@ -598,106 +631,117 @@ func normalizeSkillSpec(path string, value SkillSpec, options NormalizeOptions) } password := strings.TrimSpace(value.Password) token := strings.TrimSpace(value.Token) - if err := validateSecretReference(path+".password", password); err != nil { - return NormalizedSkillSpec{}, err - } - if err := validateSecretReference(path+".token", token); err != nil { + commonSource := sources.Source{ + Provider: provider, + URL: urlValue, + Ref: ref, + Path: pathValue, + Format: format, + Username: username, + Password: password, + Token: token, + }.Normalized() + if err := validateSourceSecrets(path, commonSource); err != nil { return NormalizedSkillSpec{}, err } - if source == "" { - source = inferSkillSource(pathValue, urlValue) - } - if source == "" { - return NormalizedSkillSpec{}, &ValidationError{Path: path + ".source", Message: "skill source is required"} - } - if source == "zip" && urlValue != "" { - urlValue = normalizeLocalSkillArchivePath(urlValue, options) - } - if pathValue != "" { - pathValue, err = normalizeSkillPath(path+".path", source, urlValue, pathValue, options) - if err != nil { - return NormalizedSkillSpec{}, err - } + if commonSource.Provider == "" { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".provider", Message: "skill provider is required"} } - switch source { - case "git": - if parsedURL, parsedPath, parsedRef, ok := parseGitHubSkillShorthand(urlValue, pathValue, ref); ok { - urlValue, pathValue, ref = parsedURL, parsedPath, parsedRef - } else if parsedURL, parsedPath, parsedRef, ok := parseGitHubSkillShorthand(pathValue, "", ref); ok { - urlValue, pathValue, ref = parsedURL, parsedPath, parsedRef - } - if urlValue == "" { - urlValue = pathValue - pathValue = "" - } + switch commonSource.Provider { + case sources.ProviderGit: if strings.TrimSpace(urlValue) == "" { return NormalizedSkillSpec{}, &ValidationError{Path: path + ".url", Message: "git skill url is required"} } - case "file": + if commonSource.Format != "" { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".format", Message: "git skill does not support format"} + } + case sources.ProviderFile: + if commonSource.URL != "" { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".url", Message: "file skill does not support url"} + } if strings.TrimSpace(pathValue) == "" { return NormalizedSkillSpec{}, &ValidationError{Path: path + ".path", Message: "file skill path is required"} } - case "zip": - if strings.TrimSpace(urlValue) == "" && strings.TrimSpace(pathValue) == "" { - return NormalizedSkillSpec{}, &ValidationError{Path: path + ".url", Message: "zip skill url or path is required"} + if commonSource.Ref != "" { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".ref", Message: "file skill does not support ref"} + } + if commonSource.Format != "" && commonSource.Format != sources.FormatZIP { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".format", Message: fmt.Sprintf("file skill format %q is not supported", commonSource.Format)} + } + if commonSource.HasAuthentication() { + return NormalizedSkillSpec{}, &ValidationError{Path: path, Message: "file skill does not support authentication"} + } + commonSource.Path = normalizeFileSkillPath(commonSource.Path, options) + case sources.ProviderHTTP: + if commonSource.URL == "" { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".url", Message: "http skill url is required"} + } + if commonSource.Ref != "" { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".ref", Message: "http skill does not support ref"} + } + if commonSource.Format != sources.FormatZIP { + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".format", Message: "http skill format must be zip"} } default: - return NormalizedSkillSpec{}, &ValidationError{Path: path + ".source", Message: fmt.Sprintf("skill source %q is not supported", source)} + return NormalizedSkillSpec{}, &ValidationError{Path: path + ".provider", Message: fmt.Sprintf("skill provider %q is not supported", commonSource.Provider)} } if name == "" { - name = inferSkillName(source, urlValue, pathValue) + name = inferSkillName(commonSource.URL, commonSource.Path) } if err := validateStableIdentifier(path+".name", name, "skill name"); err != nil { return NormalizedSkillSpec{}, err } return NormalizedSkillSpec{ Name: name, - Source: source, - URL: urlValue, - Path: pathValue, - Ref: ref, - Username: username, - Password: password, - Token: token, + Provider: commonSource.Provider, + URL: commonSource.URL, + Ref: commonSource.Ref, + Path: commonSource.Path, + Format: commonSource.Format, + Username: commonSource.Username, + Password: commonSource.Password, + Token: commonSource.Token, }, nil } -func inferSkillSource(pathValue, urlValue string) string { - candidate := strings.ToLower(strings.TrimSpace(urlValue)) - if candidate == "" { - candidate = strings.ToLower(strings.TrimSpace(pathValue)) - } - switch { - case strings.HasPrefix(candidate, "github:"): - return "git" - case strings.HasSuffix(candidate, ".git"): - return "git" - case strings.HasSuffix(candidate, ".zip"): - return "zip" - case strings.HasPrefix(candidate, "http://"), strings.HasPrefix(candidate, "https://"): - return "" - default: - return "file" - } +func workspaceSource(value WorkspaceSpec) sources.Source { + return sources.Source{ + Provider: value.Provider, + URL: value.URL, + Ref: value.Ref, + Path: value.Path, + Format: value.Format, + Username: value.Username, + Password: value.Password, + Token: value.Token, + }.Normalized() } -func validateSecretReference(path, value string) error { - if strings.TrimSpace(value) == "" { - return nil +func applyWorkspaceSource(value *WorkspaceSpec, source sources.Source) { + value.Provider = source.Provider + value.URL = source.URL + value.Ref = source.Ref + value.Path = source.Path + value.Format = source.Format + value.Username = source.Username + value.Password = source.Password + value.Token = source.Token +} + +func validateSourceSecrets(path string, source sources.Source) error { + if err := sources.ValidateSecretReference("password", source.Password); err != nil { + return &ValidationError{Path: path + ".password", Message: err.Error()} } - if !exactEnvReferencePattern.MatchString(strings.TrimSpace(value)) { - return &ValidationError{Path: path, Message: "secret value must be an environment reference like ${NAME}"} + if err := sources.ValidateSecretReference("token", source.Token); err != nil { + return &ValidationError{Path: path + ".token", Message: err.Error()} } return nil } -func normalizeLocalSkillArchivePath(value string, options NormalizeOptions) string { +func normalizeFileSkillPath(value string, options NormalizeOptions) string { if strings.TrimSpace(value) == "" { return "" } - if strings.HasPrefix(strings.ToLower(value), "http://") || strings.HasPrefix(strings.ToLower(value), "https://") { - return value - } if filepath.IsAbs(value) { return filepath.Clean(value) } @@ -708,29 +752,6 @@ func normalizeLocalSkillArchivePath(value string, options NormalizeOptions) stri return filepath.Clean(filepath.Join(base, value)) } -func normalizeSkillPath(path, source, urlValue, value string, options NormalizeOptions) (string, error) { - if strings.TrimSpace(value) == "" { - return "", nil - } - if source != "file" && source != "zip" { - return value, nil - } - if source == "zip" && strings.TrimSpace(urlValue) != "" { - return value, nil - } - if strings.HasPrefix(strings.ToLower(value), "http://") || strings.HasPrefix(strings.ToLower(value), "https://") { - return value, nil - } - if filepath.IsAbs(value) { - return filepath.Clean(value), nil - } - base := composeBaseDir(options) - if base == "" { - return filepath.Clean(value), nil - } - return filepath.Clean(filepath.Join(base, value)), nil -} - func composeBaseDir(options NormalizeOptions) string { if strings.TrimSpace(options.ComposePath) != "" { if abs, err := filepath.Abs(strings.TrimSpace(options.ComposePath)); err == nil { @@ -747,16 +768,11 @@ func composeBaseDir(options NormalizeOptions) string { return "" } -func inferSkillName(source, urlValue, pathValue string) string { +func inferSkillName(urlValue, pathValue string) string { candidate := strings.TrimSpace(pathValue) if candidate == "" { candidate = strings.TrimSpace(urlValue) } - if source == "git" { - if _, parsedPath, _, ok := parseGitHubSkillShorthand(candidate, "", ""); ok && parsedPath != "" { - candidate = parsedPath - } - } candidate = strings.TrimSuffix(candidate, "/") candidate = strings.TrimSuffix(candidate, ".zip") candidate = strings.TrimSuffix(candidate, ".git") @@ -773,30 +789,6 @@ func inferSkillName(source, urlValue, pathValue string) string { return strings.Trim(b.String(), "-_") } -func parseGitHubSkillShorthand(value, fallbackPath, fallbackRef string) (string, string, string, bool) { - value = strings.TrimSpace(value) - if !strings.HasPrefix(value, "github:") { - return "", "", "", false - } - raw := strings.TrimPrefix(value, "github:") - ref := strings.TrimSpace(fallbackRef) - if before, after, ok := strings.Cut(raw, "@"); ok { - raw = before - ref = strings.TrimSpace(after) - } - repo := raw - pathValue := strings.TrimSpace(fallbackPath) - if before, after, ok := strings.Cut(raw, "//"); ok { - repo = before - pathValue = strings.TrimSpace(after) - } - repo = strings.Trim(repo, "/") - if strings.Count(repo, "/") != 1 { - return "", "", "", false - } - return "https://github.com/" + repo + ".git", pathValue, ref, true -} - func normalizeProjectVolumes(values map[string]VolumeSpec) (map[string]NormalizedVolumeSpec, error) { if len(values) == 0 { return nil, nil @@ -1074,21 +1066,18 @@ func normalizeSchedulerSpec(path string, scheduler *SchedulerSpec, options Norma enabled = *scheduler.Enabled } script := strings.TrimSpace(scheduler.Script.Inline) - scriptURL := strings.TrimSpace(scheduler.Script.URL) - if scheduler.Script.Inline != "" && scriptURL != "" { - return nil, &ValidationError{Path: path + ".script", Message: "script must use exactly one of inline content or url"} + scriptSource := scheduler.Script.Source.Normalized() + if scheduler.Script.Inline != "" && scriptSource.HasContent() { + return nil, &ValidationError{Path: path + ".script", Message: "script must use exactly one of inline content or a provider"} } - if scheduler.Script.URL != "" && scriptURL == "" { - return nil, &ValidationError{Path: path + ".script.url", Message: "script URL is required"} - } - if scriptURL != "" { + if scriptSource.HasContent() { var err error - scriptURL, err = normalizeScriptSourceURL(scriptURL, options) + scriptSource, err = normalizeSchedulerScriptSource(path+".script", scriptSource, options) if err != nil { - return nil, &ValidationError{Path: path + ".script.url", Message: err.Error()} + return nil, err } } - if (script != "" || scriptURL != "") && len(scheduler.Triggers) > 0 { + if (script != "" || scriptSource.HasContent()) && len(scheduler.Triggers) > 0 { return nil, &ValidationError{Path: path, Message: "scheduler script and triggers are mutually exclusive"} } sandboxPolicy, err := normalizeSandboxPolicy(path+".sandbox_policy", scheduler.SandboxPolicy, "new") @@ -1102,29 +1091,29 @@ func normalizeSchedulerSpec(path string, scheduler *SchedulerSpec, options Norma Description: strings.TrimSpace(scheduler.Description), Script: script, } - if scriptURL != "" { + if scriptSource.HasContent() { if !options.ResolveScriptURLs { - normalized.scriptURL = scriptURL + normalized.scriptSource = &scriptSource } else { resolver := options.ScriptSourceResolver if resolver == nil { - resolver = NewDefaultScriptSourceResolver() + resolver = NewDefaultScriptSourceResolver(options.Env) } ctx := options.Context if ctx == nil { ctx = context.Background() } - content, err := resolver.Resolve(ctx, scriptURL) + content, err := resolver.Resolve(ctx, scriptSource) if err != nil { - return nil, &ValidationError{Path: path + ".script.url", Message: err.Error()} + return nil, &ValidationError{Path: path + ".script", Message: err.Error()} } if !utf8.Valid(content) { - return nil, &ValidationError{Path: path + ".script.url", Message: "script content must be valid UTF-8"} + return nil, &ValidationError{Path: path + ".script", Message: "script content must be valid UTF-8"} } text := strings.TrimPrefix(string(content), "\ufeff") normalized.Script = strings.TrimSpace(text) if normalized.Script == "" { - return nil, &ValidationError{Path: path + ".script.url", Message: "script content is empty"} + return nil, &ValidationError{Path: path + ".script", Message: "script content is empty"} } } } @@ -1138,6 +1127,64 @@ func normalizeSchedulerSpec(path string, scheduler *SchedulerSpec, options Norma return normalized, nil } +func normalizeSchedulerScriptSource(path string, source sources.Source, options NormalizeOptions) (sources.Source, error) { + if err := validateSourceSecrets(path, source); err != nil { + return sources.Source{}, err + } + if source.Format != "" { + return sources.Source{}, &ValidationError{Path: path + ".format", Message: "scheduler script does not support format"} + } + switch source.Provider { + case sources.ProviderFile: + if source.Path == "" { + return sources.Source{}, &ValidationError{Path: path + ".path", Message: "file script path is required"} + } + if source.URL != "" || source.Ref != "" || source.HasAuthentication() { + return sources.Source{}, &ValidationError{Path: path, Message: "file script only supports path"} + } + location, err := normalizeScriptSourceURL(source.Path, options) + if err != nil { + return sources.Source{}, &ValidationError{Path: path + ".path", Message: err.Error()} + } + parsed, _ := url.Parse(location) + if parsed.Scheme != "" && parsed.Scheme != "file" { + return sources.Source{}, &ValidationError{Path: path + ".path", Message: "file script path must use a local path or file URL"} + } + source.Path = location + case sources.ProviderHTTP: + if source.URL == "" { + return sources.Source{}, &ValidationError{Path: path + ".url", Message: "http script url is required"} + } + if source.Path != "" || source.Ref != "" { + return sources.Source{}, &ValidationError{Path: path, Message: "http script only supports url and authentication"} + } + location, err := normalizeScriptSourceURL(source.URL, options) + if err != nil { + return sources.Source{}, &ValidationError{Path: path + ".url", Message: err.Error()} + } + parsed, _ := url.Parse(location) + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return sources.Source{}, &ValidationError{Path: path + ".url", Message: "http script url must use http or https"} + } + source.URL = location + case sources.ProviderGit: + if source.URL == "" { + return sources.Source{}, &ValidationError{Path: path + ".url", Message: "git script url is required"} + } + if source.Path == "" { + return sources.Source{}, &ValidationError{Path: path + ".path", Message: "git script path is required"} + } + clean := filepath.Clean(filepath.FromSlash(source.Path)) + if filepath.IsAbs(clean) || clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + return sources.Source{}, &ValidationError{Path: path + ".path", Message: "git script path must stay within the repository"} + } + source.Path = filepath.ToSlash(clean) + default: + return sources.Source{}, &ValidationError{Path: path + ".provider", Message: fmt.Sprintf("scheduler script provider %q is not supported", source.Provider)} + } + return source, nil +} + func normalizeTriggerSpec(path string, trigger TriggerSpec) (NormalizedTriggerSpec, error) { kinds := make([]string, 0, 4) if trigger.cronSet { @@ -1327,11 +1374,8 @@ func cloneWorkspaceSpec(value *WorkspaceSpec) *WorkspaceSpec { } cloned := *value cloned.Name = strings.TrimSpace(cloned.Name) - cloned.Provider = strings.TrimSpace(cloned.Provider) - cloned.URL = strings.TrimSpace(cloned.URL) - cloned.Branch = strings.TrimSpace(cloned.Branch) - cloned.Commit = strings.TrimSpace(cloned.Commit) - cloned.Path = strings.TrimSpace(cloned.Path) + applyWorkspaceSource(&cloned, workspaceSource(cloned)) + cloned.Target = strings.TrimSpace(cloned.Target) return &cloned } diff --git a/pkg/compose/normalize_test.go b/pkg/compose/normalize_test.go index a65ec3fd8..d55f2d5f6 100644 --- a/pkg/compose/normalize_test.go +++ b/pkg/compose/normalize_test.go @@ -6,6 +6,8 @@ import ( "path/filepath" "strings" "testing" + + "agent-compose/pkg/sources" ) func TestNormalizeDefaultsProjectNameFromComposeDirectory(t *testing.T) { @@ -17,7 +19,7 @@ func TestNormalizeDefaultsProjectNameFromComposeDirectory(t *testing.T) { if err := os.WriteFile(path, []byte(` workspaces: default: - provider: local + provider: file path: . agents: reviewer: @@ -45,7 +47,7 @@ func TestNormalizeDefaultsProjectNameFromRelativeComposePath(t *testing.T) { if err := os.WriteFile(path, []byte(` workspaces: default: - provider: local + provider: file path: . agents: reviewer: @@ -163,7 +165,7 @@ agents: func TestNormalizeSortsAgentsForStableOutput(t *testing.T) { spec := &ProjectSpec{ Name: "stable", - Workspaces: map[string]WorkspaceSpec{"default": {Provider: "local", Path: "."}}, + Workspaces: map[string]WorkspaceSpec{"default": {Provider: "file", Path: "."}}, Agents: map[string]AgentSpec{ "worker": {Provider: "codex"}, "reviewer": {Provider: "codex"}, @@ -281,20 +283,23 @@ agents: reviewer: provider: codex skills: - - ./skills/local-review + - name: local-review + provider: file + path: ./skills/local-review - name: pdf - source: git + provider: git url: https://github.com/anthropics/skills.git path: skills/pdf token: ${GIT_TOKEN} - name: report - source: zip + provider: http url: https://example.com/report.zip + format: zip path: report - name: local-report - source: zip - url: ./archives/report.zip - path: report + provider: file + path: ./archives/report.zip + format: zip `) normalized, err := Normalize(spec, NormalizeOptions{ComposePath: composePath}) @@ -305,21 +310,21 @@ agents: if len(skills) != 4 { t.Fatalf("skills = %#v, want 4", skills) } - if skills[0].Name != "local-review" || skills[0].Source != "file" || skills[0].Path != filepath.Join(dir, "skills", "local-review") { + if skills[0].Name != "local-review" || skills[0].Provider != "file" || skills[0].Path != filepath.Join(dir, "skills", "local-review") { t.Fatalf("local skill = %#v", skills[0]) } - if skills[1].Name != "pdf" || skills[1].Source != "git" || skills[1].Token != "${GIT_TOKEN}" { + if skills[1].Name != "pdf" || skills[1].Provider != "git" || skills[1].Token != "${GIT_TOKEN}" { t.Fatalf("git skill = %#v", skills[1]) } - if skills[2].Source != "zip" || skills[2].URL != "https://example.com/report.zip" || skills[2].Path != "report" { + if skills[2].Provider != "http" || skills[2].Format != "zip" || skills[2].URL != "https://example.com/report.zip" || skills[2].Path != "report" { t.Fatalf("zip skill = %#v", skills[2]) } - if skills[3].Source != "zip" || skills[3].URL != filepath.Join(dir, "archives", "report.zip") || skills[3].Path != "report" { + if skills[3].Provider != "file" || skills[3].Format != "zip" || skills[3].Path != filepath.Join(dir, "archives", "report.zip") { t.Fatalf("local zip skill = %#v", skills[3]) } } -func TestNormalizeAgentSkillsInterpolatesSourceBeforeLowercase(t *testing.T) { +func TestNormalizeAgentSkillsInterpolatesProviderBeforeLowercase(t *testing.T) { spec := mustParseCompose(t, ` name: skills-project agents: @@ -327,17 +332,35 @@ agents: provider: codex skills: - name: pdf - source: ${SKILL_SOURCE} + provider: ${SKILL_PROVIDER} url: https://github.com/anthropics/skills.git path: skills/pdf `) - normalized, err := Normalize(spec, NormalizeOptions{Env: map[string]string{"SKILL_SOURCE": "GIT"}}) + normalized, err := Normalize(spec, NormalizeOptions{Env: map[string]string{"SKILL_PROVIDER": "GIT"}}) if err != nil { t.Fatalf("Normalize returned error: %v", err) } - if got := normalized.Agents[0].Skills[0].Source; got != "git" { - t.Fatalf("source = %q, want git", got) + if got := normalized.Agents[0].Skills[0].Provider; got != "git" { + t.Fatalf("provider = %q, want git", got) + } +} + +func TestNormalizeAgentSkillsRejectsZipProvider(t *testing.T) { + spec := mustParseCompose(t, ` +name: skills-project +agents: + reviewer: + provider: codex + skills: + - name: report + provider: zip + url: https://example.com/report.zip +`) + + _, err := Normalize(spec, NormalizeOptions{}) + if err == nil || !strings.Contains(err.Error(), `skill provider "zip" is not supported`) { + t.Fatalf("Normalize error = %v", err) } } @@ -349,7 +372,7 @@ agents: provider: codex skills: - name: private - source: git + provider: git url: https://git.example/skills.git token: plaintext `) @@ -371,11 +394,11 @@ agents: provider: codex skills: - name: pdf - source: git + provider: git url: https://github.com/anthropics/skills.git path: skills/pdf - name: pdf - source: file + provider: file path: ./skills/pdf `) @@ -433,7 +456,7 @@ agents: func TestNormalizePreservesValidAgentNames(t *testing.T) { spec := &ProjectSpec{ Name: "valid-agents", - Workspaces: map[string]WorkspaceSpec{"default": {Provider: "local", Path: "."}}, + Workspaces: map[string]WorkspaceSpec{"default": {Provider: "file", Path: "."}}, Agents: map[string]AgentSpec{ "a1": {Provider: "codex"}, "agent_1": {Provider: "codex"}, @@ -470,7 +493,7 @@ func TestNormalizeRejectsInvalidAgentName(t *testing.T) { t.Run(name, func(t *testing.T) { spec := &ProjectSpec{ Name: "invalid-agent", - Workspaces: map[string]WorkspaceSpec{"default": {Provider: "local", Path: "."}}, + Workspaces: map[string]WorkspaceSpec{"default": {Provider: "file", Path: "."}}, Agents: map[string]AgentSpec{ name: {Provider: "codex"}, }, @@ -692,7 +715,7 @@ func TestNormalizeRejectsInvalidJupyterGuestPort(t *testing.T) { t.Run(tt.name, func(t *testing.T) { spec := &ProjectSpec{ Name: "invalid-jupyter", - Workspaces: map[string]WorkspaceSpec{"default": {Provider: "local", Path: "."}}, + Workspaces: map[string]WorkspaceSpec{"default": {Provider: "file", Path: "."}}, Agents: map[string]AgentSpec{ "reviewer": { Provider: "codex", @@ -792,14 +815,15 @@ agents: reviewer: scheduler: script: - url: ./scripts/scheduler.js + provider: file + path: ./scripts/scheduler.js `) var gotLocation string normalized, err := Normalize(spec, NormalizeOptions{ ComposePath: composePath, ResolveScriptURLs: true, - ScriptSourceResolver: ScriptSourceResolverFunc(func(_ context.Context, location string) ([]byte, error) { - gotLocation = location + ScriptSourceResolver: ScriptSourceResolverFunc(func(_ context.Context, source sources.Source) ([]byte, error) { + gotLocation = source.Path return []byte("\xef\xbb\xbf scheduler.timeout('once', 1000, main); \n"), nil }), }) @@ -828,7 +852,7 @@ func TestNormalizeSchedulerScriptURLValidation(t *testing.T) { {name: "remote file authority", url: "file://server/scheduler.js", want: "authority"}, } { t.Run(tc.name, func(t *testing.T) { - spec := mustParseCompose(t, "name: invalid-url\nagents:\n reviewer:\n scheduler:\n script:\n url: "+tc.url+"\n") + spec := mustParseCompose(t, "name: invalid-url\nagents:\n reviewer:\n scheduler:\n script:\n provider: http\n url: "+tc.url+"\n") _, err := Normalize(spec, NormalizeOptions{}) if err == nil || !strings.Contains(err.Error(), "agents.reviewer.scheduler.script.url") || !strings.Contains(err.Error(), tc.want) { t.Fatalf("Normalize error = %v, want URL path and %q", err, tc.want) @@ -837,6 +861,26 @@ func TestNormalizeSchedulerScriptURLValidation(t *testing.T) { } } +func TestNormalizeSchedulerScriptProviderMatchesLocation(t *testing.T) { + for _, test := range []struct { + name string + body string + want string + }{ + {name: "http provider with file URL", body: "provider: http\n url: file:///tmp/scheduler.js", want: "must use http or https"}, + {name: "file provider with HTTP URL", body: "provider: file\n path: https://example.test/scheduler.js", want: "must use a local path or file URL"}, + {name: "file provider with auth", body: "provider: file\n path: ./scheduler.js\n token: ${TOKEN}", want: "only supports path"}, + } { + t.Run(test.name, func(t *testing.T) { + spec := mustParseCompose(t, "name: invalid-provider-location\nagents:\n reviewer:\n scheduler:\n script:\n "+test.body+"\n") + _, err := Normalize(spec, NormalizeOptions{}) + if err == nil || !strings.Contains(err.Error(), test.want) { + t.Fatalf("Normalize error = %v, want %q", err, test.want) + } + }) + } +} + func TestNormalizeRejectsSchedulerScriptURLWithTriggers(t *testing.T) { spec := mustParseCompose(t, ` name: mixed-url-scheduler @@ -844,7 +888,8 @@ agents: reviewer: scheduler: script: - url: ./scheduler.js + provider: file + path: ./scheduler.js triggers: - interval: 1h `) @@ -864,15 +909,15 @@ func TestNormalizeRejectsInvalidResolvedSchedulerScriptContent(t *testing.T) { {name: "empty", content: []byte(" \n\t"), want: "content is empty"}, } { t.Run(tc.name, func(t *testing.T) { - spec := mustParseCompose(t, "name: invalid-content\nagents:\n reviewer:\n scheduler:\n script:\n url: ./scheduler.js\n") + spec := mustParseCompose(t, "name: invalid-content\nagents:\n reviewer:\n scheduler:\n script:\n provider: file\n path: ./scheduler.js\n") _, err := Normalize(spec, NormalizeOptions{ ComposePath: "/project/agent-compose.yml", ResolveScriptURLs: true, - ScriptSourceResolver: ScriptSourceResolverFunc(func(context.Context, string) ([]byte, error) { + ScriptSourceResolver: ScriptSourceResolverFunc(func(context.Context, sources.Source) ([]byte, error) { return tc.content, nil }), }) - if err == nil || !strings.Contains(err.Error(), "scheduler.script.url") || !strings.Contains(err.Error(), tc.want) { + if err == nil || !strings.Contains(err.Error(), "scheduler.script") || !strings.Contains(err.Error(), tc.want) { t.Fatalf("Normalize error = %v", err) } }) @@ -1052,7 +1097,7 @@ func TestNormalizeResolvesAgentWorkspaceReference(t *testing.T) { name: reference-workspace workspaces: repo-root: - provider: local + provider: file path: . agents: reviewer: @@ -1065,7 +1110,7 @@ agents: if err != nil { t.Fatalf("Normalize returned error: %v", err) } - if normalized.Agents[0].Workspace == nil || normalized.Agents[0].Workspace.Provider != "local" || normalized.Agents[0].Workspace.Path != "." || normalized.Agents[0].Workspace.Name != "" { + if normalized.Agents[0].Workspace == nil || normalized.Agents[0].Workspace.Provider != "file" || normalized.Agents[0].Workspace.Path != "." || normalized.Agents[0].Workspace.Name != "" { t.Fatalf("workspace = %#v", normalized.Agents[0].Workspace) } } @@ -1075,7 +1120,7 @@ func TestNormalizeDoesNotUseOnlyGlobalWorkspaceByDefault(t *testing.T) { name: default-workspace workspaces: repo-root: - provider: local + provider: file path: . agents: reviewer: @@ -1116,7 +1161,7 @@ func TestNormalizeRejectsEmptyAgentWorkspaceObject(t *testing.T) { name: empty-workspace workspaces: repo-root: - provider: local + provider: file path: . agents: reviewer: @@ -1159,14 +1204,14 @@ func TestNormalizeAllowsNamedInlineAgentWorkspaceDefinition(t *testing.T) { name: mixed-workspace workspaces: repo-root: - provider: local + provider: file path: . agents: reviewer: provider: codex workspace: name: repo-root - provider: local + provider: file path: . `) @@ -1174,7 +1219,7 @@ agents: if err != nil { t.Fatalf("Normalize returned error: %v", err) } - if normalized.Agents[0].Workspace == nil || normalized.Agents[0].Workspace.Name != "repo-root" || normalized.Agents[0].Workspace.Provider != "local" || normalized.Agents[0].Workspace.Path != "." { + if normalized.Agents[0].Workspace == nil || normalized.Agents[0].Workspace.Name != "repo-root" || normalized.Agents[0].Workspace.Provider != "file" || normalized.Agents[0].Workspace.Path != "." { t.Fatalf("workspace = %#v", normalized.Agents[0].Workspace) } } @@ -1184,7 +1229,7 @@ func TestNormalizeAllowsOmittedAgentWorkspaceWithMultipleGlobals(t *testing.T) { name: ambiguous-workspace workspaces: repo-root: - provider: local + provider: file path: . docs-repo: provider: git @@ -1210,7 +1255,7 @@ func mustParseCompose(t *testing.T, raw string) *ProjectSpec { t.Fatalf("Parse returned error: %v", err) } if len(spec.Workspaces) == 0 { - spec.Workspaces = map[string]WorkspaceSpec{"default": {Provider: "local", Path: "."}} + spec.Workspaces = map[string]WorkspaceSpec{"default": {Provider: "file", Path: "."}} } return spec } diff --git a/pkg/compose/output.go b/pkg/compose/output.go index 1d3bbeb08..b7b904edb 100644 --- a/pkg/compose/output.go +++ b/pkg/compose/output.go @@ -6,6 +6,8 @@ import ( "encoding/json" "slices" + "agent-compose/pkg/sources" + "gopkg.in/yaml.v3" ) @@ -32,9 +34,13 @@ type orderedNamedWorkspace struct { Name string `yaml:"name,omitempty" json:"name,omitempty"` Provider string `yaml:"provider,omitempty" json:"provider,omitempty"` URL string `yaml:"url,omitempty" json:"url,omitempty"` - Branch string `yaml:"branch,omitempty" json:"branch,omitempty"` - Commit string `yaml:"commit,omitempty" json:"commit,omitempty"` + Ref string `yaml:"ref,omitempty" json:"ref,omitempty"` Path string `yaml:"path,omitempty" json:"path,omitempty"` + Format string `yaml:"format,omitempty" json:"format,omitempty"` + Username string `yaml:"username,omitempty" json:"username,omitempty"` + Password string `yaml:"password,omitempty" json:"password,omitempty"` + Token string `yaml:"token,omitempty" json:"token,omitempty"` + Target string `yaml:"target,omitempty" json:"target,omitempty"` } type orderedAgentSpec struct { @@ -115,10 +121,10 @@ func (s *NormalizedProjectSpec) ValidateResolvedScriptURLs() error { return nil } for _, agent := range s.Agents { - if agent.Scheduler.hasUnresolvedScriptURL() { + if agent.Scheduler.hasUnresolvedScriptSource() { return &ValidationError{ - Path: joinPath("agents", agent.Name) + ".scheduler.script.url", - Message: "script URL source is unresolved", + Path: joinPath("agents", agent.Name) + ".scheduler.script", + Message: "script source is unresolved", } } } @@ -219,9 +225,13 @@ func orderedWorkspaces(values map[string]WorkspaceSpec) []orderedNamedWorkspace Name: value.Name, Provider: value.Provider, URL: value.URL, - Branch: value.Branch, - Commit: value.Commit, + Ref: value.Ref, Path: value.Path, + Format: value.Format, + Username: value.Username, + Password: value.Password, + Token: value.Token, + Target: value.Target, }) } return out @@ -237,9 +247,13 @@ func workspaceMapFromOrdered(values []orderedNamedWorkspace) map[string]Workspac Name: value.Name, Provider: value.Provider, URL: value.URL, - Branch: value.Branch, - Commit: value.Commit, + Ref: value.Ref, Path: value.Path, + Format: value.Format, + Username: value.Username, + Password: value.Password, + Token: value.Token, + Target: value.Target, } } return out @@ -430,7 +444,7 @@ func cloneNormalizedSchedulerSpec(value *NormalizedSchedulerSpec) *NormalizedSch DisplayName: value.DisplayName, Description: value.Description, Script: value.Script, - scriptURL: value.scriptURL, + scriptSource: cloneSourcePointer(value.scriptSource), } for _, trigger := range value.Triggers { cloned.Triggers = append(cloned.Triggers, cloneNormalizedTriggerSpec(trigger)) @@ -438,6 +452,14 @@ func cloneNormalizedSchedulerSpec(value *NormalizedSchedulerSpec) *NormalizedSch return cloned } +func cloneSourcePointer(value *sources.Source) *sources.Source { + if value == nil { + return nil + } + cloned := *value + return &cloned +} + func cloneNormalizedTriggerSpec(value NormalizedTriggerSpec) NormalizedTriggerSpec { cloned := value if value.Event != nil { diff --git a/pkg/compose/output_test.go b/pkg/compose/output_test.go index 560dd00aa..341995dda 100644 --- a/pkg/compose/output_test.go +++ b/pkg/compose/output_test.go @@ -5,6 +5,8 @@ import ( "context" "strings" "testing" + + "agent-compose/pkg/sources" ) func TestNormalizeInterpolatesEnvValues(t *testing.T) { @@ -578,11 +580,11 @@ agents: script: scheduler.interval("hourly-review", "1h"); `, nil) resolve := func(location, content string) *NormalizedProjectSpec { - spec := mustParseCompose(t, "name: script-url-hash\nagents:\n reviewer:\n scheduler:\n script:\n url: "+location+"\n") + spec := mustParseCompose(t, "name: script-url-hash\nagents:\n reviewer:\n scheduler:\n script:\n provider: http\n url: "+location+"\n") normalized, err := Normalize(spec, NormalizeOptions{ ComposePath: "/project/agent-compose.yml", ResolveScriptURLs: true, - ScriptSourceResolver: ScriptSourceResolverFunc(func(context.Context, string) ([]byte, error) { + ScriptSourceResolver: ScriptSourceResolverFunc(func(context.Context, sources.Source) ([]byte, error) { return []byte(content), nil }), }) @@ -608,7 +610,7 @@ agents: } func TestUnresolvedSchedulerScriptURLFailsCanonicalOutputAndHash(t *testing.T) { - spec := mustParseCompose(t, "name: unresolved-url\nagents:\n reviewer:\n scheduler:\n script:\n url: ./scheduler.js\n") + spec := mustParseCompose(t, "name: unresolved-url\nagents:\n reviewer:\n scheduler:\n script:\n provider: file\n path: ./scheduler.js\n") normalized, err := Normalize(spec, NormalizeOptions{ComposePath: "/project/agent-compose.yml"}) if err != nil { t.Fatalf("Normalize returned error: %v", err) @@ -619,7 +621,7 @@ func TestUnresolvedSchedulerScriptURLFailsCanonicalOutputAndHash(t *testing.T) { if _, err := normalized.Hash(); err == nil || !strings.Contains(err.Error(), "unresolved") { t.Fatalf("Hash error = %v", err) } - if _, err := normalized.MarshalCanonicalYAML(false); err == nil || !strings.Contains(err.Error(), "scheduler.script.url") { + if _, err := normalized.MarshalCanonicalYAML(false); err == nil || !strings.Contains(err.Error(), "scheduler.script") { t.Fatalf("MarshalCanonicalYAML error = %v", err) } } diff --git a/pkg/compose/script_source.go b/pkg/compose/script_source.go index 63e094cc4..2c0c6eecb 100644 --- a/pkg/compose/script_source.go +++ b/pkg/compose/script_source.go @@ -11,6 +11,8 @@ import ( "path/filepath" "strings" "time" + + "agent-compose/pkg/sources" ) const ( @@ -22,24 +24,25 @@ const ( // ScriptSourceResolver fetches a structurally validated, normalized script // location. Plain paths are absolute; URL locations use file/http/https. type ScriptSourceResolver interface { - Resolve(ctx context.Context, location string) ([]byte, error) + Resolve(ctx context.Context, source sources.Source) ([]byte, error) } // ScriptSourceResolverFunc adapts a function into a ScriptSourceResolver. -type ScriptSourceResolverFunc func(context.Context, string) ([]byte, error) +type ScriptSourceResolverFunc func(context.Context, sources.Source) ([]byte, error) -func (f ScriptSourceResolverFunc) Resolve(ctx context.Context, location string) ([]byte, error) { - return f(ctx, location) +func (f ScriptSourceResolverFunc) Resolve(ctx context.Context, source sources.Source) ([]byte, error) { + return f(ctx, source) } type defaultScriptSourceResolver struct { client *http.Client + env map[string]string } // NewDefaultScriptSourceResolver returns the bounded file and HTTP(S) resolver // used by CLI compose loading. -func NewDefaultScriptSourceResolver() ScriptSourceResolver { - resolver := &defaultScriptSourceResolver{} +func NewDefaultScriptSourceResolver(env map[string]string) ScriptSourceResolver { + resolver := &defaultScriptSourceResolver{env: env} resolver.client = &http.Client{ Timeout: defaultScriptSourceTimeout, CheckRedirect: func(req *http.Request, via []*http.Request) error { @@ -125,7 +128,7 @@ func scriptSourceBaseDir(options NormalizeOptions) string { return "." } -func (r *defaultScriptSourceResolver) Resolve(ctx context.Context, location string) ([]byte, error) { +func (r *defaultScriptSourceResolver) Resolve(ctx context.Context, source sources.Source) ([]byte, error) { if ctx == nil { ctx = context.Background() } @@ -134,25 +137,45 @@ func (r *defaultScriptSourceResolver) Resolve(ctx context.Context, location stri if err := ctx.Err(); err != nil { return nil, err } - parsed, err := url.Parse(location) - if err != nil { - return nil, errors.New("invalid script URL") - } - switch strings.ToLower(parsed.Scheme) { - case "": - return readScriptFileWithContext(ctx, location) - case "file": - path, err := url.PathUnescape(parsed.Path) + source = source.Normalized() + switch source.Provider { + case sources.ProviderFile: + parsed, err := url.Parse(source.Path) if err != nil { - return nil, errors.New("invalid file URL path") + return nil, errors.New("invalid script path") + } + path := source.Path + if parsed.Scheme == "file" { + path, err = url.PathUnescape(parsed.Path) + if err != nil { + return nil, errors.New("invalid file URL path") + } } return readScriptFileWithContext(ctx, path) - case "http", "https": - parsed.Scheme = strings.ToLower(parsed.Scheme) - return r.readHTTP(ctx, parsed) + case sources.ProviderHTTP: + parsed, err := url.Parse(source.URL) + if err != nil { + return nil, errors.New("invalid script URL") + } + return r.readHTTP(ctx, parsed, source) + case sources.ProviderGit: + return r.readGit(ctx, source) default: - return nil, fmt.Errorf("script URL scheme %q is not supported", parsed.Scheme) + return nil, fmt.Errorf("script provider %q is not supported", source.Provider) + } +} + +func (r *defaultScriptSourceResolver) readGit(ctx context.Context, source sources.Source) ([]byte, error) { + tempDir, err := os.MkdirTemp("", "agent-compose-script-git-*") + if err != nil { + return nil, fmt.Errorf("create git script temp dir: %w", err) + } + defer func() { _ = os.RemoveAll(tempDir) }() + checkoutDir := filepath.Join(tempDir, "repository") + if _, err := (sources.GitClient{Env: r.env}).Checkout(ctx, source, checkoutDir); err != nil { + return nil, err } + return readScriptFileWithContext(ctx, filepath.Join(checkoutDir, filepath.FromSlash(source.Path))) } func readScriptFileWithContext(ctx context.Context, path string) ([]byte, error) { @@ -186,11 +209,12 @@ func readScriptFile(path string) ([]byte, error) { return readLimitedScript(file) } -func (r *defaultScriptSourceResolver) readHTTP(ctx context.Context, location *url.URL) ([]byte, error) { +func (r *defaultScriptSourceResolver) readHTTP(ctx context.Context, location *url.URL, source sources.Source) ([]byte, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, location.String(), nil) if err != nil { return nil, fmt.Errorf("create script request for %s", redactedScriptURL(location)) } + sources.ApplyHTTPAuthentication(req, source, r.env) resp, err := r.client.Do(req) if err != nil { return nil, fmt.Errorf("fetch script from %s: %s", redactedScriptURL(location), sanitizeScriptFetchError(err)) diff --git a/pkg/compose/script_source_test.go b/pkg/compose/script_source_test.go index a2040f9a1..31285ade9 100644 --- a/pkg/compose/script_source_test.go +++ b/pkg/compose/script_source_test.go @@ -8,10 +8,13 @@ import ( "net/http/httptest" "net/url" "os" + "os/exec" "path/filepath" "strings" "testing" "time" + + "agent-compose/pkg/sources" ) func TestDefaultScriptSourceResolverReadsFilesAndFileURLs(t *testing.T) { @@ -24,25 +27,62 @@ func TestDefaultScriptSourceResolverReadsFilesAndFileURLs(t *testing.T) { if err := os.Symlink(path, link); err != nil { t.Fatal(err) } - resolver := NewDefaultScriptSourceResolver() + resolver := NewDefaultScriptSourceResolver(nil) for _, location := range []string{path, (&url.URL{Scheme: "file", Path: link}).String()} { - data, err := resolver.Resolve(context.Background(), location) + data, err := resolver.Resolve(context.Background(), sources.Source{Provider: sources.ProviderFile, Path: location}) if err != nil || !strings.Contains(string(data), "scheduler.interval") { t.Fatalf("Resolve(%q) data=%q err=%v", location, data, err) } } - if _, err := resolver.Resolve(context.Background(), dir); err == nil || !strings.Contains(err.Error(), "regular file") { + if _, err := resolver.Resolve(context.Background(), sources.Source{Provider: sources.ProviderFile, Path: dir}); err == nil || !strings.Contains(err.Error(), "regular file") { t.Fatalf("directory Resolve error = %v", err) } } +func TestDefaultScriptSourceResolverReadsGitFile(t *testing.T) { + repository := t.TempDir() + for _, args := range [][]string{ + {"init", "-b", "main"}, + {"config", "user.email", "agent-compose@example.test"}, + {"config", "user.name", "Agent Compose"}, + } { + cmd := exec.Command("git", args...) + cmd.Dir = repository + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, output) + } + } + if err := os.MkdirAll(filepath.Join(repository, "schedulers"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repository, "schedulers", "review.js"), []byte("scheduler.agent('review');\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"add", "."}, {"commit", "-m", "add scheduler"}} { + cmd := exec.Command("git", args...) + cmd.Dir = repository + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, output) + } + } + data, err := NewDefaultScriptSourceResolver(nil).Resolve(context.Background(), sources.Source{ + Provider: sources.ProviderGit, + URL: repository, + Ref: "main", + Path: "schedulers/review.js", + }) + if err != nil || !strings.Contains(string(data), "scheduler.agent") { + t.Fatalf("git script = %q, err=%v", data, err) + } +} + func TestDefaultScriptSourceResolverHTTPFailures(t *testing.T) { t.Run("status and query redaction", func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadGateway) })) defer server.Close() - _, err := NewDefaultScriptSourceResolver().Resolve(context.Background(), server.URL+"/scheduler.js?token=super-secret") + _, err := NewDefaultScriptSourceResolver(nil).Resolve(context.Background(), sources.Source{Provider: sources.ProviderHTTP, URL: server.URL + "/scheduler.js?token=super-secret"}) if err == nil || !strings.Contains(err.Error(), "status 502") || strings.Contains(err.Error(), "super-secret") { t.Fatalf("Resolve error = %v", err) } @@ -55,7 +95,7 @@ func TestDefaultScriptSourceResolverHTTPFailures(t *testing.T) { defer server.Close() ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) defer cancel() - _, err := NewDefaultScriptSourceResolver().Resolve(ctx, server.URL) + _, err := NewDefaultScriptSourceResolver(nil).Resolve(ctx, sources.Source{Provider: sources.ProviderHTTP, URL: server.URL}) if err == nil || !strings.Contains(err.Error(), "deadline exceeded") { t.Fatalf("Resolve timeout error = %v", err) } @@ -68,7 +108,7 @@ func TestDefaultScriptSourceResolverHTTPFailures(t *testing.T) { http.Redirect(w, r, fmt.Sprintf("/%d", n+1), http.StatusFound) })) defer server.Close() - _, err := NewDefaultScriptSourceResolver().Resolve(context.Background(), server.URL+"/0") + _, err := NewDefaultScriptSourceResolver(nil).Resolve(context.Background(), sources.Source{Provider: sources.ProviderHTTP, URL: server.URL + "/0"}) if err == nil || !strings.Contains(err.Error(), "too many redirects") { t.Fatalf("Resolve redirects error = %v", err) } @@ -79,7 +119,7 @@ func TestDefaultScriptSourceResolverHTTPFailures(t *testing.T) { http.Redirect(w, r, "file:///tmp/scheduler.js", http.StatusFound) })) defer server.Close() - _, err := NewDefaultScriptSourceResolver().Resolve(context.Background(), server.URL) + _, err := NewDefaultScriptSourceResolver(nil).Resolve(context.Background(), sources.Source{Provider: sources.ProviderHTTP, URL: server.URL}) if err == nil || !strings.Contains(err.Error(), "not supported") { t.Fatalf("Resolve redirect error = %v", err) } @@ -99,6 +139,7 @@ agents: reviewer: scheduler: script: + provider: http url: %s `, location)) normalized, err := Normalize(spec, NormalizeOptions{ResolveScriptURLs: true}) @@ -118,14 +159,14 @@ func TestDefaultScriptSourceResolverLimitsDecodedHTTPContent(t *testing.T) { _ = writer.Close() })) defer server.Close() - _, err := NewDefaultScriptSourceResolver().Resolve(context.Background(), server.URL) + _, err := NewDefaultScriptSourceResolver(nil).Resolve(context.Background(), sources.Source{Provider: sources.ProviderHTTP, URL: server.URL}) if err == nil || !strings.Contains(err.Error(), "exceeds") { t.Fatalf("Resolve oversized content error = %v", err) } } func TestDefaultScriptSourceResolverRejectsHTTPSDowngrade(t *testing.T) { - resolver := NewDefaultScriptSourceResolver().(*defaultScriptSourceResolver) + resolver := NewDefaultScriptSourceResolver(nil).(*defaultScriptSourceResolver) httpsRequest := httptest.NewRequest(http.MethodGet, "https://example.test/source", nil) httpRequest := httptest.NewRequest(http.MethodGet, "http://example.test/target", nil) err := resolver.client.CheckRedirect(httpRequest, []*http.Request{httpsRequest}) diff --git a/pkg/compose/spec.go b/pkg/compose/spec.go index d6bf1a7b8..8bc9de697 100644 --- a/pkg/compose/spec.go +++ b/pkg/compose/spec.go @@ -5,6 +5,8 @@ import ( "os" "strings" + "agent-compose/pkg/sources" + "gopkg.in/yaml.v3" ) @@ -120,10 +122,11 @@ type VolumeMountSpec struct { type SkillSpec struct { Name string `yaml:"name,omitempty" json:"name,omitempty"` - Source string `yaml:"source,omitempty" json:"source,omitempty"` + Provider string `yaml:"provider,omitempty" json:"provider,omitempty"` URL string `yaml:"url,omitempty" json:"url,omitempty"` - Path string `yaml:"path,omitempty" json:"path,omitempty"` Ref string `yaml:"ref,omitempty" json:"ref,omitempty"` + Path string `yaml:"path,omitempty" json:"path,omitempty"` + Format string `yaml:"format,omitempty" json:"format,omitempty"` Username string `yaml:"username,omitempty" json:"username,omitempty"` Password string `yaml:"password,omitempty" json:"password,omitempty"` Token string `yaml:"token,omitempty" json:"token,omitempty"` @@ -139,16 +142,16 @@ type SchedulerSpec struct { } // ScriptSource is the authoring shape accepted by scheduler.script. Inline is -// populated for the scalar form and URL for the explicit {url: ...} form. -// The two forms are mutually exclusive. +// populated for the scalar form and Source for a flat provider mapping. The +// two forms are mutually exclusive. type ScriptSource struct { Inline string - URL string + Source sources.Source } // IsZero reports whether neither authoring form is set. func (s ScriptSource) IsZero() bool { - return s.Inline == "" && s.URL == "" + return s.Inline == "" && !s.Source.HasContent() } func (s *ScriptSource) UnmarshalYAML(value *yaml.Node) error { @@ -161,13 +164,11 @@ func (s *ScriptSource) UnmarshalYAML(value *yaml.Node) error { *s = ScriptSource{Inline: inline} return nil case yaml.MappingNode: - var source struct { - URL string `yaml:"url"` - } + var source sources.Source if err := value.Decode(&source); err != nil { return err } - *s = ScriptSource{URL: source.URL} + *s = ScriptSource{Source: source} return nil default: return fmt.Errorf("expected scalar or mapping, got %s", nodeKindName(value.Kind)) @@ -175,8 +176,8 @@ func (s *ScriptSource) UnmarshalYAML(value *yaml.Node) error { } func (s ScriptSource) MarshalYAML() (any, error) { - if s.URL != "" { - return map[string]string{"url": s.URL}, nil + if s.Source.HasContent() { + return s.Source, nil } return s.Inline, nil } @@ -204,9 +205,26 @@ type WorkspaceSpec struct { Name string `yaml:"name,omitempty" json:"name,omitempty"` Provider string `yaml:"provider,omitempty" json:"provider,omitempty"` URL string `yaml:"url,omitempty" json:"url,omitempty"` - Branch string `yaml:"branch,omitempty" json:"branch,omitempty"` - Commit string `yaml:"commit,omitempty" json:"commit,omitempty"` + Ref string `yaml:"ref,omitempty" json:"ref,omitempty"` Path string `yaml:"path,omitempty" json:"path,omitempty"` + Format string `yaml:"format,omitempty" json:"format,omitempty"` + Username string `yaml:"username,omitempty" json:"username,omitempty"` + Password string `yaml:"password,omitempty" json:"password,omitempty"` + Token string `yaml:"token,omitempty" json:"token,omitempty"` + Target string `yaml:"target,omitempty" json:"target,omitempty"` +} + +func (s WorkspaceSpec) ContentSource() sources.Source { + return sources.Source{ + Provider: s.Provider, + URL: s.URL, + Ref: s.Ref, + Path: s.Path, + Format: s.Format, + Username: s.Username, + Password: s.Password, + Token: s.Token, + }.Normalized() } type NetworkSpec struct { @@ -314,28 +332,6 @@ func (s *VolumeMountSpec) UnmarshalYAML(value *yaml.Node) error { } } -func (s *SkillSpec) UnmarshalYAML(value *yaml.Node) error { - switch value.Kind { - case yaml.ScalarNode: - var raw string - if err := value.Decode(&raw); err != nil { - return err - } - s.Path = raw - return nil - case yaml.MappingNode: - type skillSpec SkillSpec - var decoded skillSpec - if err := value.Decode(&decoded); err != nil { - return err - } - *s = SkillSpec(decoded) - return nil - default: - return fmt.Errorf("expected scalar or mapping, got %s", nodeKindName(value.Kind)) - } -} - func (s *TriggerSpec) UnmarshalYAML(value *yaml.Node) error { type triggerSpec TriggerSpec var decoded triggerSpec @@ -633,23 +629,9 @@ func validateSkillList(node *yaml.Node, path string) error { } func validateSkill(node *yaml.Node, path string) error { - switch node.Kind { - case yaml.ScalarNode: - return validateScalar(node, path) - case yaml.MappingNode: - return validateMapping(node, path, map[string]nodeValidator{ - "name": validateScalar, - "source": validateScalar, - "url": validateScalar, - "path": validateScalar, - "ref": validateScalar, - "username": validateScalar, - "password": validateScalar, - "token": validateScalar, - }) - default: - return newParseError(node, path, "expected scalar or mapping") - } + return validateMapping(node, path, sourceFieldValidators(map[string]nodeValidator{ + "name": validateScalar, + })) } func validateBuild(node *yaml.Node, path string) error { @@ -704,20 +686,7 @@ func validateScriptSource(node *yaml.Node, path string) error { case yaml.ScalarNode: return validateScalar(node, path) case yaml.MappingNode: - if err := validateMapping(node, path, map[string]nodeValidator{ - "url": validateScalar, - }); err != nil { - return err - } - if len(node.Content) == 0 { - return newParseError(node, path+".url", "script URL is required") - } - for i := 0; i < len(node.Content); i += 2 { - if node.Content[i].Value == "url" && strings.TrimSpace(node.Content[i+1].Value) == "" { - return newParseError(node.Content[i+1], path+".url", "script URL is required") - } - } - return nil + return validateMapping(node, path, sourceFieldValidators(nil)) default: return newParseError(node, path, "expected scalar or mapping") } @@ -761,14 +730,27 @@ func validateEventTrigger(node *yaml.Node, path string) error { } func validateWorkspace(node *yaml.Node, path string) error { - return validateMapping(node, path, map[string]nodeValidator{ - "name": validateScalar, + return validateMapping(node, path, sourceFieldValidators(map[string]nodeValidator{ + "name": validateScalar, + "target": validateScalar, + })) +} + +func sourceFieldValidators(extra map[string]nodeValidator) map[string]nodeValidator { + fields := map[string]nodeValidator{ "provider": validateScalar, "url": validateScalar, - "branch": validateScalar, - "commit": validateScalar, + "ref": validateScalar, "path": validateScalar, - }) + "format": validateScalar, + "username": validateScalar, + "password": validateScalar, + "token": validateScalar, + } + for name, validator := range extra { + fields[name] = validator + } + return fields } func validateDriver(node *yaml.Node, path string) error { diff --git a/pkg/compose/spec_test.go b/pkg/compose/spec_test.go index e3347c21f..019076f08 100644 --- a/pkg/compose/spec_test.go +++ b/pkg/compose/spec_test.go @@ -92,7 +92,7 @@ workspaces: repo: provider: git url: https://github.com/org/repo.git - branch: main + ref: main agents: reviewer: provider: codex @@ -105,7 +105,7 @@ agents: env: REVIEW_MODE: strict workspace: - provider: local + provider: file path: ./repo jupyter: enabled: true @@ -127,7 +127,7 @@ network: if got := spec.Variables["OPENAI_API_KEY"]; got.Value != "${OPENAI_API_KEY}" || !got.Secret { t.Fatalf("OPENAI_API_KEY = %#v", got) } - if spec.Workspaces["repo"].Provider != "git" || spec.Workspaces["repo"].Branch != "main" { + if spec.Workspaces["repo"].Provider != "git" || spec.Workspaces["repo"].Ref != "main" { t.Fatalf("workspaces = %#v", spec.Workspaces) } agent := spec.Agents["reviewer"] @@ -182,33 +182,68 @@ agents: } } -func TestParseSchedulerScriptURL(t *testing.T) { +func TestParseSchedulerScriptProvider(t *testing.T) { spec, err := Parse([]byte(` name: url-project agents: reviewer: scheduler: script: - url: ./scripts/scheduler.js + provider: file + path: ./scripts/scheduler.js `)) if err != nil { t.Fatalf("Parse returned error: %v", err) } source := spec.Agents["reviewer"].Scheduler.Script - if source.Inline != "" || source.URL != "./scripts/scheduler.js" { + if source.Inline != "" || source.Source.Provider != "file" || source.Source.Path != "./scripts/scheduler.js" { t.Fatalf("scheduler script source = %#v", source) } } -func TestParseRejectsInvalidSchedulerScriptURLObject(t *testing.T) { +func TestParseRejectsLegacyResourceSourceSyntax(t *testing.T) { + for _, test := range []struct { + name string + yaml string + want string + }{ + { + name: "skill source discriminator", + yaml: "agents:\n reviewer:\n skills:\n - source: git\n url: https://example.test/skills.git\n", + want: "agents.reviewer.skills[0].source", + }, + { + name: "skill scalar shorthand", + yaml: "agents:\n reviewer:\n skills:\n - ./skills/review\n", + want: "agents.reviewer.skills[0]", + }, + { + name: "workspace branch", + yaml: "workspaces:\n repo:\n provider: git\n url: https://example.test/repo.git\n branch: main\n", + want: "workspaces.repo.branch", + }, + { + name: "workspace commit", + yaml: "workspaces:\n repo:\n provider: git\n url: https://example.test/repo.git\n commit: abc123\n", + want: "workspaces.repo.commit", + }, + } { + t.Run(test.name, func(t *testing.T) { + _, err := Parse([]byte(test.yaml)) + if err == nil || !strings.Contains(err.Error(), test.want) { + t.Fatalf("Parse error = %v, want path %q", err, test.want) + } + }) + } +} + +func TestParseRejectsInvalidSchedulerScriptProviderObject(t *testing.T) { for _, tc := range []struct { name string body string want string }{ - {name: "unknown field", body: "url: ./scheduler.js\n extra: true", want: "scheduler.script.extra"}, - {name: "empty URL", body: `url: ""`, want: "scheduler.script.url"}, - {name: "missing URL", body: `{}`, want: "scheduler.script.url"}, + {name: "unknown field", body: "provider: file\n path: ./scheduler.js\n extra: true", want: "scheduler.script.extra"}, } { t.Run(tc.name, func(t *testing.T) { _, err := Parse([]byte("agents:\n reviewer:\n scheduler:\n script:\n " + tc.body + "\n")) diff --git a/pkg/compose/workspace_commit_test.go b/pkg/compose/workspace_commit_test.go deleted file mode 100644 index 36453b41b..000000000 --- a/pkg/compose/workspace_commit_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package compose - -import ( - "strings" - "testing" -) - -func TestNormalizeGitWorkspacePreservesBranchAndCommit(t *testing.T) { - normalized, err := Normalize(&ProjectSpec{ - Name: "project", - Agents: map[string]AgentSpec{ - "worker": { - Provider: "codex", - Workspace: &WorkspaceSpec{ - Provider: " git ", - URL: " https://example.test/repo.git ", - Branch: " main ", - Commit: " abc123 ", - }, - }, - }, - }, NormalizeOptions{}) - if err != nil { - t.Fatalf("Normalize returned error: %v", err) - } - workspace := normalized.Agents[0].Workspace - if workspace == nil || workspace.Branch != "main" || workspace.Commit != "abc123" { - t.Fatalf("normalized workspace = %#v", workspace) - } - - data, err := normalized.MarshalCanonicalJSON(false) - if err != nil { - t.Fatalf("MarshalCanonicalJSON returned error: %v", err) - } - if !strings.Contains(string(data), `"branch":"main"`) || !strings.Contains(string(data), `"commit":"abc123"`) { - t.Fatalf("canonical JSON lost git revision fields: %s", data) - } -} - -func TestParseGitWorkspaceAcceptsCommit(t *testing.T) { - spec, err := Parse([]byte(` -name: project -agents: - worker: - provider: codex - workspace: - provider: git - url: https://example.test/repo.git - branch: main - commit: abc123 -`)) - if err != nil { - t.Fatalf("Parse returned error: %v", err) - } - workspace := spec.Agents["worker"].Workspace - if workspace == nil || workspace.Branch != "main" || workspace.Commit != "abc123" { - t.Fatalf("parsed workspace = %#v", workspace) - } -} - -func TestNormalizeLocalWorkspaceRejectsCommit(t *testing.T) { - _, err := Normalize(&ProjectSpec{ - Name: "project", - Agents: map[string]AgentSpec{ - "worker": {Provider: "codex", Workspace: &WorkspaceSpec{Provider: "local", Path: ".", Commit: "abc123"}}, - }, - }, NormalizeOptions{}) - if err == nil || !strings.Contains(err.Error(), "local workspace does not support commit") { - t.Fatalf("Normalize error = %v", err) - } -} diff --git a/pkg/compose/workspace_ref_test.go b/pkg/compose/workspace_ref_test.go new file mode 100644 index 000000000..58a17a347 --- /dev/null +++ b/pkg/compose/workspace_ref_test.go @@ -0,0 +1,50 @@ +package compose + +import ( + "strings" + "testing" +) + +func TestNormalizeGitWorkspaceUsesRef(t *testing.T) { + normalized, err := Normalize(&ProjectSpec{ + Name: "project", + Agents: map[string]AgentSpec{ + "worker": { + Provider: "codex", + Workspace: &WorkspaceSpec{ + Provider: " git ", + URL: " https://example.test/repo.git ", + Ref: " main ", + Target: " source ", + }, + }, + }, + }, NormalizeOptions{}) + if err != nil { + t.Fatalf("Normalize returned error: %v", err) + } + workspace := normalized.Agents[0].Workspace + if workspace == nil || workspace.Ref != "main" || workspace.Target != "source" { + t.Fatalf("normalized workspace = %#v", workspace) + } + + data, err := normalized.MarshalCanonicalJSON(false) + if err != nil { + t.Fatalf("MarshalCanonicalJSON returned error: %v", err) + } + if !strings.Contains(string(data), `"ref":"main"`) || !strings.Contains(string(data), `"target":"source"`) { + t.Fatalf("canonical JSON lost git source fields: %s", data) + } +} + +func TestNormalizeFileWorkspaceRejectsRef(t *testing.T) { + _, err := Normalize(&ProjectSpec{ + Name: "project", + Agents: map[string]AgentSpec{ + "worker": {Provider: "codex", Workspace: &WorkspaceSpec{Provider: "file", Path: ".", Ref: "abc123"}}, + }, + }, NormalizeOptions{}) + if err == nil || !strings.Contains(err.Error(), "file workspace does not support ref") { + t.Fatalf("Normalize error = %v", err) + } +} diff --git a/pkg/model/agent_model.go b/pkg/model/agent_model.go index 884d94dd6..c88d4855c 100644 --- a/pkg/model/agent_model.go +++ b/pkg/model/agent_model.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" "time" + + "agent-compose/pkg/sources" ) const ( @@ -42,10 +44,11 @@ type AgentDefinition struct { type AgentSkill struct { Name string `json:"name,omitempty"` - Source string `json:"source,omitempty"` + Provider string `json:"provider,omitempty"` URL string `json:"url,omitempty"` - Path string `json:"path,omitempty"` Ref string `json:"ref,omitempty"` + Path string `json:"path,omitempty"` + Format string `json:"format,omitempty"` Username string `json:"username,omitempty"` Password string `json:"password,omitempty"` Token string `json:"token,omitempty"` @@ -162,13 +165,15 @@ func NormalizeAgentSkills(skills []AgentSkill) []AgentSkill { out := make([]AgentSkill, 0, len(skills)) for _, skill := range skills { skill.Name = strings.TrimSpace(skill.Name) - skill.Source = strings.ToLower(strings.TrimSpace(skill.Source)) - skill.URL = strings.TrimSpace(skill.URL) - skill.Path = strings.TrimSpace(skill.Path) - skill.Ref = strings.TrimSpace(skill.Ref) - skill.Username = strings.TrimSpace(skill.Username) - skill.Password = strings.TrimSpace(skill.Password) - skill.Token = strings.TrimSpace(skill.Token) + source := AgentSkillSource(skill) + skill.Provider = source.Provider + skill.URL = source.URL + skill.Ref = source.Ref + skill.Path = source.Path + skill.Format = source.Format + skill.Username = source.Username + skill.Password = source.Password + skill.Token = source.Token skill.SourceRoot = strings.TrimSpace(skill.SourceRoot) if skill.Name == "" { continue @@ -185,6 +190,19 @@ func NormalizeAgentSkills(skills []AgentSkill) []AgentSkill { return out } +func AgentSkillSource(skill AgentSkill) sources.Source { + return sources.Source{ + Provider: skill.Provider, + URL: skill.URL, + Ref: skill.Ref, + Path: skill.Path, + Format: skill.Format, + Username: skill.Username, + Password: skill.Password, + Token: skill.Token, + }.Normalized() +} + func normalizeCapsetIDs(ids []string) []string { seen := make(map[string]struct{}, len(ids)) out := make([]string, 0, len(ids)) diff --git a/pkg/projects/controller_coverage_test.go b/pkg/projects/controller_coverage_test.go index aa3f4e87f..a656a44ae 100644 --- a/pkg/projects/controller_coverage_test.go +++ b/pkg/projects/controller_coverage_test.go @@ -23,7 +23,7 @@ variables: SHARED: value workspaces: default: - provider: local + provider: file path: . agents: worker: diff --git a/pkg/projects/legacy_default_project.go b/pkg/projects/legacy_default_project.go index 0a084fb34..728303810 100644 --- a/pkg/projects/legacy_default_project.go +++ b/pkg/projects/legacy_default_project.go @@ -471,7 +471,7 @@ func legacyVolumes(items []domain.VolumeMountSpec) []compose.NormalizedVolumeMou func legacySkills(items []domain.AgentSkill) []compose.NormalizedSkillSpec { result := make([]compose.NormalizedSkillSpec, 0, len(items)) for _, item := range items { - result = append(result, compose.NormalizedSkillSpec{Name: item.Name, Source: item.Source, URL: item.URL, Path: item.Path, Ref: item.Ref, Username: item.Username, Password: item.Password, Token: item.Token}) + result = append(result, compose.NormalizedSkillSpec{Name: item.Name, Provider: item.Provider, URL: item.URL, Path: item.Path, Ref: item.Ref, Format: item.Format, Username: item.Username, Password: item.Password, Token: item.Token}) } return result } diff --git a/pkg/projects/legacy_default_project_integration_test.go b/pkg/projects/legacy_default_project_integration_test.go index 71645d6f0..9cdbfa59c 100644 --- a/pkg/projects/legacy_default_project_integration_test.go +++ b/pkg/projects/legacy_default_project_integration_test.go @@ -145,7 +145,7 @@ scheduler.on("news.ready", "on-news", function onNews() {}); } projectedWorkspace := result.RevisionSpec.Workspaces["news-source"] wantWorkspacePath := filepath.ToSlash(filepath.Join("workspaces", workspace.ID, workspaces.FileWorkspaceContentDirName)) - if projectedWorkspace.Provider != "local" || projectedWorkspace.Path != wantWorkspacePath { + if projectedWorkspace.Provider != "file" || projectedWorkspace.Path != wantWorkspacePath { t.Fatalf("projected workspace = %#v, want path %q", projectedWorkspace, wantWorkspacePath) } diff --git a/pkg/projects/legacy_default_project_test.go b/pkg/projects/legacy_default_project_test.go index 67f84e0f8..29d006cb5 100644 --- a/pkg/projects/legacy_default_project_test.go +++ b/pkg/projects/legacy_default_project_test.go @@ -21,7 +21,7 @@ func TestLegacyDefaultNormalizedProjectPreservesAgentConfiguration(t *testing.T) EnvItems: []domain.SandboxEnvVar{{Name: "TOKEN", Value: "secret", Secret: true}}, Volumes: []domain.VolumeMountSpec{{Type: "bind", Source: "/host", Target: "/guest", ReadOnly: true}}, CapsetIDs: []string{"tools"}, - Skills: []domain.AgentSkill{{Name: "review", Source: "local", Path: "skills/review"}}, + Skills: []domain.AgentSkill{{Name: "review", Provider: "file", Path: "skills/review"}}, SystemPrompt: "review carefully", }, {ID: "agent-a", Name: "worker-a-Z", Enabled: true, Provider: "codex", ConfigJSON: "{}"}, diff --git a/pkg/projects/legacy_workspace_binding.go b/pkg/projects/legacy_workspace_binding.go index 88c310f02..8f6a6de92 100644 --- a/pkg/projects/legacy_workspace_binding.go +++ b/pkg/projects/legacy_workspace_binding.go @@ -90,7 +90,7 @@ func legacyFileWorkspaceID(spec *compose.NormalizedProjectSpec, agent compose.No // LegacyFileWorkspacePresetID returns the original v1 file-workspace preset ID // encoded by the canonical migration-only local workspace path. func LegacyFileWorkspacePresetID(workspace *compose.WorkspaceSpec) (string, bool) { - if workspace == nil || strings.ToLower(strings.TrimSpace(workspace.Provider)) != "local" { + if workspace == nil || strings.ToLower(strings.TrimSpace(workspace.Provider)) != "file" { return "", false } path := filepath.ToSlash(filepath.Clean(strings.TrimSpace(workspace.Path))) @@ -117,7 +117,7 @@ func resolvedLegacyWorkspace(spec *compose.NormalizedProjectSpec, reference *com return &resolved } } - if strings.TrimSpace(reference.Provider) != "" || strings.TrimSpace(reference.URL) != "" || strings.TrimSpace(reference.Branch) != "" || strings.TrimSpace(reference.Commit) != "" || strings.TrimSpace(reference.Path) != "" { + if reference.ContentSource().HasContent() || strings.TrimSpace(reference.Target) != "" { return reference } workspace, ok := spec.Workspaces[strings.TrimSpace(reference.Name)] diff --git a/pkg/projects/legacy_workspace_binding_test.go b/pkg/projects/legacy_workspace_binding_test.go index af8fc109e..81f09c320 100644 --- a/pkg/projects/legacy_workspace_binding_test.go +++ b/pkg/projects/legacy_workspace_binding_test.go @@ -10,7 +10,7 @@ import ( func TestLegacyFileWorkspaceIDSupportsNamedAndReappliedInlineSpecs(t *testing.T) { spec := &compose.NormalizedProjectSpec{ Workspaces: map[string]compose.WorkspaceSpec{ - "uploads": {Provider: "local", Path: "workspaces/legacy-file/content"}, + "uploads": {Provider: "file", Path: "workspaces/legacy-file/content"}, }, } @@ -22,14 +22,14 @@ func TestLegacyFileWorkspaceIDSupportsNamedAndReappliedInlineSpecs(t *testing.T) } workspaceID, ok = legacyFileWorkspaceID(spec, compose.NormalizedAgentSpec{ - Workspace: &compose.WorkspaceSpec{Provider: "local", Path: "workspaces/legacy-file/content"}, + Workspace: &compose.WorkspaceSpec{Provider: "file", Path: "workspaces/legacy-file/content"}, }) if !ok || workspaceID != "legacy-file" { t.Fatalf("inline legacy workspace = %q, %v", workspaceID, ok) } if workspaceID, ok := legacyFileWorkspaceID(spec, compose.NormalizedAgentSpec{ - Workspace: &compose.WorkspaceSpec{Provider: "local", Path: "source"}, + Workspace: &compose.WorkspaceSpec{Provider: "file", Path: "source"}, }); ok || workspaceID != "" { t.Fatalf("ordinary local workspace was treated as legacy = %q, %v", workspaceID, ok) } diff --git a/pkg/projects/legacy_workspace_migration.go b/pkg/projects/legacy_workspace_migration.go index 13ece5f97..68f8fdbe4 100644 --- a/pkg/projects/legacy_workspace_migration.go +++ b/pkg/projects/legacy_workspace_migration.go @@ -126,20 +126,21 @@ func mapLegacyWorkspaceConfig(config *appconfig.Config, workspace domain.Workspa if err := json.Unmarshal([]byte(workspace.ConfigJSON), &legacy); err != nil { return compose.WorkspaceSpec{}, fmt.Errorf("decode legacy git workspace preset %s: %w", workspace.ID, err) } - cloneTarget, err := workspaces.NormalizeGitCloneTarget(workspace.ID, legacy.CloneTarget) + target, err := workspaces.NormalizeWorkspaceTarget(workspace.ID, legacy.Target) if err != nil { return compose.WorkspaceSpec{}, err } - cloneURL := workspaces.ApplyGitCredentials(legacy.URL, legacy) - if strings.TrimSpace(cloneURL) == "" { + if strings.TrimSpace(legacy.URL) == "" { return compose.WorkspaceSpec{}, fmt.Errorf("legacy git workspace preset %s has no url", workspace.ID) } return compose.WorkspaceSpec{ Provider: "git", - URL: cloneURL, - Branch: strings.TrimSpace(legacy.Branch), - Commit: strings.TrimSpace(legacy.Commit), - Path: cloneTarget, + URL: legacy.URL, + Ref: legacy.Ref, + Username: legacy.Username, + Password: legacy.Password, + Token: legacy.Token, + Target: target, }, nil case "file": // Keep a valid v2 shape in the returned spec. The synthetic-project @@ -163,7 +164,7 @@ func mapLegacyWorkspaceConfig(config *appconfig.Config, workspace domain.Workspa if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { return compose.WorkspaceSpec{}, fmt.Errorf("legacy file workspace preset %s content is outside the data root", workspace.ID) } - return compose.WorkspaceSpec{Provider: "local", Path: filepath.ToSlash(relative)}, nil + return compose.WorkspaceSpec{Provider: "file", Path: filepath.ToSlash(relative), Target: "."}, nil default: return compose.WorkspaceSpec{}, fmt.Errorf("legacy workspace preset %s has unsupported type %q", workspace.ID, workspace.Type) } diff --git a/pkg/projects/legacy_workspace_migration_test.go b/pkg/projects/legacy_workspace_migration_test.go index da388b6dd..436ce2c6b 100644 --- a/pkg/projects/legacy_workspace_migration_test.go +++ b/pkg/projects/legacy_workspace_migration_test.go @@ -16,17 +16,18 @@ func TestMapLegacyWorkspaceConfigToExistingV2Shapes(t *testing.T) { ID: "git-workspace", Type: "git", ConfigJSON: `{ + "provider":"git", "url":"https://example.test/team/repo.git", - "branch":"main", - "commit":"abc123", - "credential":"user:token", - "path":"source" + "ref":"abc123", + "target":"source", + "username":"user", + "token":"${TOKEN}" }`, }) if err != nil { t.Fatalf("mapLegacyWorkspaceConfig returned error: %v", err) } - if workspace.Provider != "git" || workspace.URL != "https://user:token@example.test/team/repo.git" || workspace.Branch != "main" || workspace.Commit != "abc123" || workspace.Path != "source" { + if workspace.Provider != "git" || workspace.URL != "https://example.test/team/repo.git" || workspace.Ref != "abc123" || workspace.Target != "source" || workspace.Username != "user" || workspace.Token != "${TOKEN}" { t.Fatalf("mapped git workspace = %#v", workspace) } }) @@ -44,7 +45,7 @@ func TestMapLegacyWorkspaceConfigToExistingV2Shapes(t *testing.T) { t.Fatalf("mapLegacyWorkspaceConfig returned error: %v", err) } wantPath := filepath.ToSlash(filepath.Join("workspaces", workspaceID, workspaces.FileWorkspaceContentDirName)) - if workspace.Provider != "local" || workspace.Path != wantPath { + if workspace.Provider != "file" || workspace.Path != wantPath { t.Fatalf("mapped file workspace = %#v, want path %q", workspace, wantPath) } }) @@ -54,21 +55,21 @@ func TestMapLegacyWorkspaceConfigPreservesCommitPin(t *testing.T) { workspace, err := mapLegacyWorkspaceConfig(nil, domain.WorkspaceConfig{ ID: "git-workspace", Type: "git", - ConfigJSON: `{"url":"https://example.test/repo.git","commit":"abc123"}`, + ConfigJSON: `{"provider":"git","url":"https://example.test/repo.git","ref":"abc123"}`, }) if err != nil { t.Fatalf("mapLegacyWorkspaceConfig returned error: %v", err) } - if workspace.Commit != "abc123" { - t.Fatalf("mapped commit = %q, want abc123", workspace.Commit) + if workspace.Ref != "abc123" { + t.Fatalf("mapped ref = %q, want abc123", workspace.Ref) } } func TestLegacyDefaultNormalizedProjectMapsNamedWorkspaces(t *testing.T) { projection := legacyWorkspaceProjection{ workspaces: map[string]compose.WorkspaceSpec{ - "source": {Provider: "git", URL: "https://example.test/source.git", Path: "."}, - "tasks": {Provider: "git", URL: "https://example.test/tasks.git", Path: "."}, + "source": {Provider: "git", URL: "https://example.test/source.git", Target: "."}, + "tasks": {Provider: "git", URL: "https://example.test/tasks.git", Target: "."}, }, nameByID: map[string]string{ "workspace-source": "source", @@ -150,7 +151,7 @@ func TestLegacyProjectWorkspaceNamesAreStableAndValid(t *testing.T) { func TestLegacySingleWorkspaceDoesNotBecomeDefaultForUnassignedAgents(t *testing.T) { projection := legacyWorkspaceProjection{ workspaces: map[string]compose.WorkspaceSpec{ - "source": {Provider: "git", URL: "https://example.test/source.git", Path: "."}, + "source": {Provider: "git", URL: "https://example.test/source.git", Target: "."}, }, nameByID: map[string]string{"workspace-source": "source"}, } @@ -176,8 +177,8 @@ func TestLegacySingleWorkspaceDoesNotBecomeDefaultForUnassignedAgents(t *testing func TestLegacyMultipleWorkspacesRemainReapplicableWithUnassignedAgent(t *testing.T) { projection := legacyWorkspaceProjection{ workspaces: map[string]compose.WorkspaceSpec{ - "source": {Provider: "git", URL: "https://example.test/source.git", Path: "."}, - "tasks": {Provider: "git", URL: "https://example.test/tasks.git", Path: "."}, + "source": {Provider: "git", URL: "https://example.test/source.git", Target: "."}, + "tasks": {Provider: "git", URL: "https://example.test/tasks.git", Target: "."}, }, nameByID: map[string]string{ "workspace-source": "source", @@ -223,7 +224,7 @@ func TestLegacyMultipleWorkspacesRemainReapplicableWithUnassignedAgent(t *testin func TestLegacyLoaderWorkspaceDoesNotOverwriteWorkspaceLessSourceAgent(t *testing.T) { projection := legacyWorkspaceProjection{ workspaces: map[string]compose.WorkspaceSpec{ - "tasks": {Provider: "git", URL: "https://example.test/tasks.git", Path: "."}, + "tasks": {Provider: "git", URL: "https://example.test/tasks.git", Target: "."}, }, nameByID: map[string]string{"workspace-tasks": "tasks"}, } @@ -253,7 +254,7 @@ func TestLegacyLoaderWorkspaceDoesNotOverwriteWorkspaceLessSourceAgent(t *testin scheduled = agent } } - if scheduled.Workspace == nil || scheduled.Workspace.Provider != "git" || scheduled.Workspace.Path != "." { + if scheduled.Workspace == nil || scheduled.Workspace.Provider != "git" || scheduled.Workspace.Target != "." { t.Fatalf("compatibility loader agent workspace = %#v", scheduled.Workspace) } } diff --git a/pkg/projects/records.go b/pkg/projects/records.go index 0ba41e311..b8451a900 100644 --- a/pkg/projects/records.go +++ b/pkg/projects/records.go @@ -251,10 +251,11 @@ func AgentSkillsFromCompose(values []compose.NormalizedSkillSpec, sourcePath str for _, value := range values { out = append(out, domain.AgentSkill{ Name: value.Name, - Source: value.Source, + Provider: value.Provider, URL: value.URL, - Path: value.Path, Ref: value.Ref, + Path: value.Path, + Format: value.Format, Username: value.Username, Password: value.Password, Token: value.Token, diff --git a/pkg/projects/records_test.go b/pkg/projects/records_test.go index 28420593f..52cadbac9 100644 --- a/pkg/projects/records_test.go +++ b/pkg/projects/records_test.go @@ -179,8 +179,8 @@ func TestNewAgentDefinitionFromSpecCarriesSkills(t *testing.T) { Name: "reviewer", Provider: "codex", Skills: []compose.NormalizedSkillSpec{ - {Name: "pdf", Source: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/pdf", Ref: "main", Token: "${GIT_TOKEN}"}, - {Name: "local-review", Source: "file", Path: "/tmp/skills/local-review"}, + {Name: "pdf", Provider: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/pdf", Ref: "main", Token: "${GIT_TOKEN}"}, + {Name: "local-review", Provider: "file", Path: "/tmp/skills/local-review"}, }, } @@ -202,13 +202,13 @@ func TestNewAgentDefinitionFromSpecCarriesSkills(t *testing.T) { func TestManagedAgentDefinitionUnchangedComparesSkills(t *testing.T) { existing := domain.AgentDefinition{ ID: "agent-1", Name: "Agent", Enabled: true, Provider: "codex", ConfigJSON: "{}", - Skills: []domain.AgentSkill{{Name: "pdf", Source: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/pdf"}}, + Skills: []domain.AgentSkill{{Name: "pdf", Provider: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/pdf"}}, } current := existing if !ManagedAgentDefinitionUnchanged(existing, current) { t.Fatalf("matching skills should be unchanged") } - current.Skills = []domain.AgentSkill{{Name: "docx", Source: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/docx"}} + current.Skills = []domain.AgentSkill{{Name: "docx", Provider: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/docx"}} if ManagedAgentDefinitionUnchanged(existing, current) { t.Fatalf("different skills should mark managed agent changed") } diff --git a/pkg/runs/coverage_shape_workflows_test.go b/pkg/runs/coverage_shape_workflows_test.go index 9b0e6821f..9460368bd 100644 --- a/pkg/runs/coverage_shape_workflows_test.go +++ b/pkg/runs/coverage_shape_workflows_test.go @@ -28,6 +28,7 @@ import ( "agent-compose/pkg/sessions" "agent-compose/pkg/storage/sessionstore" "agent-compose/pkg/volumes" + "agent-compose/pkg/workspaces" agentcomposev2 "agent-compose/proto/agentcompose/v2" ) @@ -141,7 +142,7 @@ func TestRunsPreparationWorkspaceAndStatusWorkflows(t *testing.T) { if err := os.WriteFile(filepath.Join(sourceDir, "README.md"), []byte("hello"), 0o644); err != nil { t.Fatal(err) } - spec := `{"variables":[{"name":"PROJECT_VAR","value":"project"}],"agents":[{"name":"worker","workspace":{"provider":"local","path":"."}}]}` + spec := `{"variables":[{"name":"PROJECT_VAR","value":"project"}],"agents":[{"name":"worker","workspace":{"provider":"file","path":"."}}]}` store := &fakePreparationStore{ project: domain.ProjectRecord{ID: "project-1", Name: "Project", SourcePath: sourceDir}, revision: domain.ProjectRevisionRecord{ProjectID: "project-1", Revision: 1, SpecJSON: spec}, @@ -187,11 +188,11 @@ func TestRunsPreparationWorkspaceAndStatusWorkflows(t *testing.T) { if path, err := ResolveLocalProjectWorkspacePath(store.project, "."); err != nil || path == "" { t.Fatalf("ResolveLocalProjectWorkspacePath path=%q err=%v", path, err) } - gitConfig, err := projectRunGitWorkspaceConfig(run, &compose.WorkspaceSpec{Provider: "git", URL: "https://example.test/repo.git", Branch: "main", Commit: "abc123", Path: "."}) + gitConfig, err := projectRunGitWorkspaceConfig(run, &compose.WorkspaceSpec{Provider: "git", URL: "https://example.test/repo.git", Ref: "abc123", Target: "."}) if err != nil { t.Fatalf("projectRunGitWorkspaceConfig returned error: %v", err) } - if !strings.Contains(gitConfig.ConfigJSON, `"branch":"main"`) || !strings.Contains(gitConfig.ConfigJSON, `"commit":"abc123"`) { + if !strings.Contains(gitConfig.ConfigJSON, `"ref":"abc123"`) || !strings.Contains(gitConfig.ConfigJSON, `"target":"."`) { t.Fatalf("git workspace config lost revision fields: %s", gitConfig.ConfigJSON) } if _, err := projectRunGitWorkspaceConfig(run, &compose.WorkspaceSpec{Provider: "git"}); err == nil { @@ -206,14 +207,28 @@ func TestRunsPreparationWorkspaceAndStatusWorkflows(t *testing.T) { if _, err := controller.prepareProjectRunWorkspace(ctx, run, store.project, nil, &compose.WorkspaceSpec{Provider: "s3"}); err == nil || !strings.Contains(err.Error(), "unsupported") { t.Fatalf("unsupported provider err=%v", err) } - localWorkspace, err := controller.prepareProjectRunWorkspace(ctx, run, store.project, nil, &compose.WorkspaceSpec{Provider: "local", Path: "."}) + localWorkspace, err := controller.prepareProjectRunWorkspace(ctx, run, store.project, nil, &compose.WorkspaceSpec{Provider: "file", Path: "."}) if err != nil || localWorkspace == nil || localWorkspace.Type != "file" { t.Fatalf("agent local workspace = %#v/%v", localWorkspace, err) } - if _, err := (&Controller{}).materializeLocalProjectRunWorkspace(run, store.project, &compose.WorkspaceSpec{Provider: "local", Path: "."}); err == nil { + targetedWorkspace, err := controller.prepareProjectRunWorkspace(ctx, run, store.project, nil, &compose.WorkspaceSpec{Provider: "file", Path: ".", Target: "nested"}) + if err != nil || targetedWorkspace == nil { + t.Fatalf("targeted local workspace = %#v/%v", targetedWorkspace, err) + } + targetedRoot, err := workspaces.FileWorkspaceContentRoot(controller.config, *targetedWorkspace) + if err != nil { + t.Fatalf("targeted workspace root: %v", err) + } + if _, err := os.Stat(filepath.Join(targetedRoot, "nested", "README.md")); err != nil { + t.Fatalf("targeted workspace file: %v", err) + } + if _, err := os.Stat(filepath.Join(targetedRoot, "README.md")); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("workspace source was copied outside target: %v", err) + } + if _, err := (&Controller{}).materializeLocalProjectRunWorkspace(run, store.project, &compose.WorkspaceSpec{Provider: "file", Path: "."}); err == nil { t.Fatalf("materialize without config returned nil error") } - if _, err := controller.materializeLocalProjectRunWorkspace(run, store.project, &compose.WorkspaceSpec{Provider: "local", Path: "missing"}); err == nil { + if _, err := controller.materializeLocalProjectRunWorkspace(run, store.project, &compose.WorkspaceSpec{Provider: "file", Path: "missing"}); err == nil { t.Fatalf("materialize missing local path returned nil error") } if snapshot := toSandboxWorkspaceSnapshot(domain.WorkspaceConfig{ID: "workspace", Name: "Workspace", Type: "file", ConfigJSON: "{}"}); snapshot.ID != "workspace" { diff --git a/pkg/runs/preparation.go b/pkg/runs/preparation.go index 0c01b3656..f39c52fb7 100644 --- a/pkg/runs/preparation.go +++ b/pkg/runs/preparation.go @@ -192,9 +192,13 @@ type canonicalRevisionWorkspace struct { Name string `json:"name"` Provider string `json:"provider"` URL string `json:"url"` - Branch string `json:"branch"` - Commit string `json:"commit"` + Ref string `json:"ref"` Path string `json:"path"` + Format string `json:"format"` + Target string `json:"target"` + Username string `json:"username"` + Password string `json:"password"` + Token string `json:"token"` Workspace *agentcomposev2.WorkspaceSpec `json:"workspace"` } @@ -220,9 +224,9 @@ func restoreCanonicalProjectWorkspaces(data []byte, spec *agentcomposev2.Project } if strings.TrimSpace(workspace.Provider) == "" && strings.TrimSpace(workspace.URL) == "" && - strings.TrimSpace(workspace.Branch) == "" && - strings.TrimSpace(workspace.Commit) == "" && - strings.TrimSpace(workspace.Path) == "" { + strings.TrimSpace(workspace.Ref) == "" && + strings.TrimSpace(workspace.Path) == "" && + strings.TrimSpace(workspace.Target) == "" { continue } if key := strings.TrimSpace(workspace.Key); key != "" { @@ -233,9 +237,13 @@ func restoreCanonicalProjectWorkspaces(data []byte, spec *agentcomposev2.Project spec.Workspaces[i].Workspace = &agentcomposev2.WorkspaceSpec{ Provider: workspace.Provider, Url: workspace.URL, - Branch: workspace.Branch, - Commit: workspace.Commit, + Ref: workspace.Ref, Path: workspace.Path, + Format: workspace.Format, + Target: workspace.Target, + Username: workspace.Username, + Password: workspace.Password, + Token: workspace.Token, } } return nil @@ -277,9 +285,13 @@ func ComposeWorkspaceSpecFromV2(workspace *agentcomposev2.WorkspaceSpec) *compos Name: workspace.GetName(), Provider: workspace.GetProvider(), URL: workspace.GetUrl(), - Branch: workspace.GetBranch(), - Commit: workspace.GetCommit(), + Ref: workspace.GetRef(), Path: workspace.GetPath(), + Format: workspace.GetFormat(), + Target: workspace.GetTarget(), + Username: workspace.GetUsername(), + Password: workspace.GetPassword(), + Token: workspace.GetToken(), } } @@ -304,7 +316,7 @@ func ProjectRunWorkspaceSpecsFromV2(projectWorkspaces []*agentcomposev2.NamedWor agent := ComposeWorkspaceSpecFromV2(agentWorkspace) if agent != nil { hasName := strings.TrimSpace(agent.Name) != "" - hasInline := strings.TrimSpace(agent.Provider) != "" || strings.TrimSpace(agent.URL) != "" || strings.TrimSpace(agent.Branch) != "" || strings.TrimSpace(agent.Commit) != "" || strings.TrimSpace(agent.Path) != "" + hasInline := agent.ContentSource().HasContent() || strings.TrimSpace(agent.Target) != "" switch { case hasInline: return nil, agent, nil diff --git a/pkg/runs/preparation_legacy_workspace_test.go b/pkg/runs/preparation_legacy_workspace_test.go index 17676335c..932792e24 100644 --- a/pkg/runs/preparation_legacy_workspace_test.go +++ b/pkg/runs/preparation_legacy_workspace_test.go @@ -26,7 +26,7 @@ func TestPrepareProjectRunReusesManagedAgentWorkspacePreset(t *testing.T) { revision: domain.ProjectRevisionRecord{ ProjectID: projectID, Revision: 1, - SpecJSON: `{"name":"legacy-v1-default","agents":[{"name":"worker","workspace":{"provider":"local","path":"workspaces/legacy-file/content"}}]}`, + SpecJSON: `{"name":"legacy-v1-default","agents":[{"name":"worker","workspace":{"provider":"file","path":"workspaces/legacy-file/content"}}]}`, }, agent: domain.AgentDefinition{ID: "managed-agent", WorkspaceID: workspace.ID}, workspace: workspace, @@ -70,13 +70,13 @@ func TestPrepareProjectRunKeepsGitAndOrdinaryV2OnWorkspaceResolver(t *testing.T) { name: "legacy git workspace", project: domain.ProjectRecord{ID: legacyProjectID, Name: projects.LegacyDefaultProjectName}, - specJSON: `{"name":"legacy-v1-default","agents":[{"name":"worker","workspace":{"provider":"git","url":"https://example.test/repo.git","branch":"main","path":"."}}]}`, + specJSON: `{"name":"legacy-v1-default","agents":[{"name":"worker","workspace":{"provider":"git","url":"https://example.test/repo.git","ref":"main","target":"."}}]}`, workspace: domain.WorkspaceConfig{ID: "legacy-git", Type: "git", ConfigJSON: `{"url":"https://example.test/repo.git"}`}, }, { name: "ordinary v2 local workspace", project: domain.ProjectRecord{ID: "project-1", Name: "project", SourcePath: "/project/source"}, - specJSON: `{"name":"project","agents":[{"name":"worker","workspace":{"provider":"local","path":"workspaces/legacy-file/content"}}]}`, + specJSON: `{"name":"project","agents":[{"name":"worker","workspace":{"provider":"file","path":"workspaces/legacy-file/content"}}]}`, workspace: domain.WorkspaceConfig{ID: "legacy-file", Type: "file", ConfigJSON: `{}`}, }, } diff --git a/pkg/runs/preparation_workspace_compat_test.go b/pkg/runs/preparation_workspace_compat_test.go index d4d67ff5c..784ac0df5 100644 --- a/pkg/runs/preparation_workspace_compat_test.go +++ b/pkg/runs/preparation_workspace_compat_test.go @@ -12,7 +12,7 @@ func TestDecodeRevisionSpecSupportsCanonicalWorkspaceShape(t *testing.T) { "workspaces":[{ "key":"repo-root", "name":"repo-root", - "provider":"local", + "provider":"file", "path":"workspaces/local-repo" }], "agents":[{"name":"reviewer"}] @@ -24,7 +24,7 @@ func TestDecodeRevisionSpecSupportsCanonicalWorkspaceShape(t *testing.T) { t.Fatalf("workspaces = %#v", decoded.GetWorkspaces()) } workspace := decoded.GetWorkspaces()[0] - if workspace.GetName() != "repo-root" || workspace.GetWorkspace().GetProvider() != "local" || workspace.GetWorkspace().GetPath() != "workspaces/local-repo" { + if workspace.GetName() != "repo-root" || workspace.GetWorkspace().GetProvider() != "file" || workspace.GetWorkspace().GetPath() != "workspaces/local-repo" { t.Fatalf("workspace = %#v", workspace) } } @@ -36,9 +36,8 @@ func TestDecodeRevisionSpecSupportsCanonicalGitCommitShape(t *testing.T) { "key":"repo-root", "provider":"git", "url":"https://example.test/repo.git", - "branch":"main", - "commit":"abc123", - "path":"." + "ref":"abc123", + "target":"." }], "agents":[{"name":"reviewer"}] }`) @@ -46,7 +45,7 @@ func TestDecodeRevisionSpecSupportsCanonicalGitCommitShape(t *testing.T) { t.Fatalf("DecodeRevisionSpec returned error: %v", err) } workspace := decoded.GetWorkspaces()[0].GetWorkspace() - if workspace.GetBranch() != "main" || workspace.GetCommit() != "abc123" { + if workspace.GetRef() != "abc123" || workspace.GetTarget() != "." { t.Fatalf("canonical git workspace = %#v", workspace) } } @@ -56,14 +55,14 @@ func TestDecodeRevisionSpecPreservesNestedWorkspaceShape(t *testing.T) { "name":"workspace-project", "workspaces":[{ "name":"repo-root", - "workspace":{"provider":"git","url":"https://example.test/repo.git","branch":"main","commit":"abc123"} + "workspace":{"provider":"git","url":"https://example.test/repo.git","ref":"abc123","target":"."} }] }`) if err != nil { t.Fatalf("DecodeRevisionSpec returned error: %v", err) } workspace := decoded.GetWorkspaces()[0] - if workspace.GetName() != "repo-root" || workspace.GetWorkspace().GetProvider() != "git" || workspace.GetWorkspace().GetUrl() != "https://example.test/repo.git" || workspace.GetWorkspace().GetBranch() != "main" || workspace.GetWorkspace().GetCommit() != "abc123" { + if workspace.GetName() != "repo-root" || workspace.GetWorkspace().GetProvider() != "git" || workspace.GetWorkspace().GetUrl() != "https://example.test/repo.git" || workspace.GetWorkspace().GetRef() != "abc123" || workspace.GetWorkspace().GetTarget() != "." { t.Fatalf("workspace = %#v", workspace) } } @@ -85,7 +84,7 @@ func TestIntegrationProjectRevisionDoesNotSelectOmittedAgentWorkspace(t *testing normalized, err := compose.Normalize(&compose.ProjectSpec{ Name: "workspace-project", Workspaces: map[string]compose.WorkspaceSpec{ - "repo-root": {Provider: "local", Path: "workspaces/local-repo"}, + "repo-root": {Provider: "file", Path: "workspaces/local-repo"}, }, Agents: map[string]compose.AgentSpec{ "reviewer": { @@ -125,7 +124,7 @@ func TestIntegrationProjectRevisionNamedInlineWorkspaceRoundTrip(t *testing.T) { normalized, err := compose.Normalize(&compose.ProjectSpec{ Name: "workspace-project", Workspaces: map[string]compose.WorkspaceSpec{ - "docs-repo": {Provider: "local", Path: "workspaces/local-docs"}, + "docs-repo": {Provider: "file", Path: "workspaces/local-docs"}, }, Agents: map[string]compose.AgentSpec{ "reviewer": { @@ -133,7 +132,7 @@ func TestIntegrationProjectRevisionNamedInlineWorkspaceRoundTrip(t *testing.T) { Driver: &compose.DriverSpec{Docker: &compose.DockerDriverSpec{}}, Workspace: &compose.WorkspaceSpec{ Name: "repo-root", - Provider: "local", + Provider: "file", Path: "workspaces/local-repo", }, }, @@ -161,7 +160,7 @@ func TestIntegrationProjectRevisionNamedInlineWorkspaceRoundTrip(t *testing.T) { if projectWorkspace != nil { t.Fatalf("project workspace = %#v, want nil for inline override", projectWorkspace) } - if agentWorkspace == nil || agentWorkspace.Name != "repo-root" || agentWorkspace.Provider != "local" || agentWorkspace.Path != "workspaces/local-repo" { + if agentWorkspace == nil || agentWorkspace.Name != "repo-root" || agentWorkspace.Provider != "file" || agentWorkspace.Path != "workspaces/local-repo" { t.Fatalf("agent workspace = %#v", agentWorkspace) } } diff --git a/pkg/runs/project_workspace_resume_integration_test.go b/pkg/runs/project_workspace_resume_integration_test.go index e4fb4e074..18724aaeb 100644 --- a/pkg/runs/project_workspace_resume_integration_test.go +++ b/pkg/runs/project_workspace_resume_integration_test.go @@ -266,7 +266,7 @@ const projectWorkspaceRevisionSpec = `{ "name": "Project Local Resume", "variables": [{"name": "SOURCE_VERSION", "value": %q}], "workspaces": [ - {"name": "local-source", "workspace": {"provider": "local", "path": "workspace"}} + {"name": "local-source", "workspace": {"provider": "file", "path": "workspace"}} ], "agents": [ {"name": "worker", "provider": "codex", "workspace": {"name": "local-source"}} diff --git a/pkg/runs/workspace.go b/pkg/runs/workspace.go index b3d2e66f3..1f3854920 100644 --- a/pkg/runs/workspace.go +++ b/pkg/runs/workspace.go @@ -5,11 +5,13 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "strings" "agent-compose/pkg/compose" appconfig "agent-compose/pkg/config" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" "agent-compose/pkg/workspaces" ) @@ -36,13 +38,13 @@ func (c *Controller) prepareProjectRunWorkspace(ctx context.Context, run domain. } provider := strings.ToLower(strings.TrimSpace(workspace.Provider)) switch provider { - case "local": + case sources.ProviderFile: config, err := c.materializeLocalProjectRunWorkspace(run, project, workspace) if err != nil { return nil, err } return &config, nil - case "git": + case sources.ProviderGit: config, err := projectRunGitWorkspaceConfig(run, workspace) if err != nil { return nil, err @@ -89,7 +91,18 @@ func (c *Controller) materializeLocalProjectRunWorkspace(run domain.ProjectRunRe return domain.WorkspaceConfig{}, fmt.Errorf("open local workspace source %s: %w", sourceDir, err) } defer func() { _ = sourceRoot.Close() }() - if err := workspaces.CopyRootDirectoryContents(sourceRoot, content.AbsRoot); err != nil { + target, err := workspaces.NormalizeWorkspaceTarget(workspaceID, workspace.Target) + if err != nil { + return domain.WorkspaceConfig{}, err + } + destination := content.AbsRoot + if target != "." { + destination = filepath.Join(content.AbsRoot, target) + if err := os.MkdirAll(destination, 0o755); err != nil { + return domain.WorkspaceConfig{}, fmt.Errorf("create local workspace target %s: %w", target, err) + } + } + if err := workspaces.CopyRootDirectoryContents(sourceRoot, destination); err != nil { return domain.WorkspaceConfig{}, fmt.Errorf("materialize local workspace snapshot: %w", err) } return config, nil @@ -100,14 +113,12 @@ func projectRunGitWorkspaceConfig(run domain.ProjectRunRecord, workspace *compos if strings.TrimSpace(workspace.URL) == "" { return domain.WorkspaceConfig{}, fmt.Errorf("git workspace url is required") } - if _, err := workspaces.NormalizeGitCloneTarget(workspaceID, workspace.Path); err != nil { + if _, err := workspaces.NormalizeWorkspaceTarget(workspaceID, workspace.Target); err != nil { return domain.WorkspaceConfig{}, err } payload, err := json.Marshal(workspaces.GitWorkspaceConfig{ - URL: strings.TrimSpace(workspace.URL), - Branch: strings.TrimSpace(workspace.Branch), - Commit: strings.TrimSpace(workspace.Commit), - CloneTarget: strings.TrimSpace(workspace.Path), + Source: workspace.ContentSource(), + Target: strings.TrimSpace(workspace.Target), }) if err != nil { return domain.WorkspaceConfig{}, fmt.Errorf("encode git workspace config: %w", err) diff --git a/pkg/skills/resolve.go b/pkg/skills/resolve.go index 6d86482a0..e57cb0c82 100644 --- a/pkg/skills/resolve.go +++ b/pkg/skills/resolve.go @@ -12,9 +12,7 @@ import ( "net/http" "net/url" "os" - "os/exec" "path/filepath" - "regexp" "strings" "syscall" "time" @@ -22,14 +20,10 @@ import ( appconfig "agent-compose/pkg/config" "agent-compose/pkg/execution" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" "agent-compose/pkg/workspaces" ) -var ( - credentialURLPattern = regexp.MustCompile(`([a-zA-Z][a-zA-Z0-9+.-]*:)//[^@\s]+@`) - gitRemoteHelperURLPattern = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*::`) -) - const ( DefaultDownloadLimitBytes = 64 << 20 MaxZipExpandedBytes = 256 << 20 @@ -111,16 +105,22 @@ func (r Resolver) Projected(skills []ResolvedSkill) []execution.ResolvedAgentSki } func (r Resolver) resolveOne(ctx context.Context, spec domain.AgentSkill) (ResolvedSkill, error) { - source := strings.ToLower(strings.TrimSpace(spec.Source)) - switch source { - case "file": + provider := strings.ToLower(strings.TrimSpace(spec.Provider)) + switch provider { + case sources.ProviderFile: + if spec.Format == sources.FormatZIP { + return r.resolveZip(ctx, spec) + } return r.resolveFile(spec) - case "git": + case sources.ProviderGit: return r.resolveGit(ctx, spec) - case "zip": + case sources.ProviderHTTP: + if spec.Format != sources.FormatZIP { + return ResolvedSkill{}, fmt.Errorf("skill %s http format %q is not supported", spec.Name, spec.Format) + } return r.resolveZip(ctx, spec) default: - return ResolvedSkill{}, fmt.Errorf("skill %s source %q is not supported", spec.Name, spec.Source) + return ResolvedSkill{}, fmt.Errorf("skill %s provider %q is not supported", spec.Name, spec.Provider) } } @@ -165,32 +165,21 @@ func (r Resolver) resolveGit(ctx context.Context, spec domain.AgentSkill) (Resol return ResolvedSkill{}, fmt.Errorf("validate git skill %s url: %w", spec.Name, err) } } - urlValue := gitURLWithCredentials(rawURL, spec, r.Env) - if urlValue == "" { + if rawURL == "" { return ResolvedSkill{}, fmt.Errorf("skill %s git url is required", spec.Name) } - if err := validateGitOperand("git url", urlValue); err != nil { - return ResolvedSkill{}, fmt.Errorf("validate git skill %s url: %w", spec.Name, err) - } - if err := validateGitURLScheme(urlValue); err != nil { - return ResolvedSkill{}, fmt.Errorf("validate git skill %s url: %w", spec.Name, err) - } - ref := strings.TrimSpace(spec.Ref) - if err := validateGitOperand("git ref", ref); err != nil { - return ResolvedSkill{}, fmt.Errorf("validate git skill %s ref: %w", spec.Name, err) - } - commit, err := gitResolve(ctx, urlValue, ref) + source := domain.AgentSkillSource(spec) + source.URL = rawURL + gitClient := sources.GitClient{Env: r.Env} + resolved, err := gitClient.Resolve(ctx, source) if err != nil { return ResolvedSkill{}, fmt.Errorf("resolve git skill %s ref: %w", spec.Name, err) } - key := cacheKey("git", gitCacheURL(rawURL), commit, spec.Path) + key := cacheKey("git", gitCacheURL(rawURL), resolved.Commit, spec.Path) dst := filepath.Join(r.CacheRoot, key) if err := ensureCachedDir(dst, func(tmp string) error { cloneDir := filepath.Join(tmp, "repo") - if err := runGit(ctx, "", "clone", "--no-checkout", "--", urlValue, cloneDir); err != nil { - return err - } - if err := runGit(ctx, cloneDir, "checkout", commit); err != nil { + if err := gitClient.CheckoutCommit(ctx, source, resolved.Commit, cloneDir); err != nil { return err } subdir := strings.TrimSpace(spec.Path) @@ -211,7 +200,7 @@ func (r Resolver) resolveGit(ctx context.Context, spec domain.AgentSkill) (Resol if err := validateSkillDir(spec.Name, content); err != nil { return ResolvedSkill{}, err } - if err := touchArtifactManifest(dst, "git", commit); err != nil { + if err := touchArtifactManifest(dst, "git", resolved.Commit); err != nil { return ResolvedSkill{}, err } return ResolvedSkill{Name: spec.Name, LocalDir: content}, nil @@ -252,7 +241,7 @@ func (r Resolver) resolveZip(ctx context.Context, spec domain.AgentSkill) (Resol if archivePath == "" { archivePath = strings.TrimSpace(spec.Path) } else if strings.HasPrefix(strings.ToLower(archivePath), "http://") || strings.HasPrefix(strings.ToLower(archivePath), "https://") { - path, done, err := r.download(ctx, archivePath) + path, done, err := r.download(ctx, archivePath, domain.AgentSkillSource(spec)) if err != nil { return ResolvedSkill{}, fmt.Errorf("download skill %s zip: %w", spec.Name, err) } @@ -322,7 +311,7 @@ func safeArtifactSubdir(root, subdir string) (string, error) { return target, nil } -func (r Resolver) download(ctx context.Context, rawURL string) (string, func(), error) { +func (r Resolver) download(ctx context.Context, rawURL string, source sources.Source) (string, func(), error) { if err := validateDownloadURL(rawURL); err != nil { return "", nil, err } @@ -350,6 +339,7 @@ func (r Resolver) download(ctx context.Context, rawURL string) (string, func(), if err != nil { return "", nil, err } + sources.ApplyHTTPAuthentication(req, source, r.Env) resp, err := clientCopy.Do(req) if err != nil { return "", nil, err @@ -780,84 +770,6 @@ func cacheKey(parts ...string) string { return hex.EncodeToString(h.Sum(nil)) } -func runGit(ctx context.Context, dir string, args ...string) error { - cmd := exec.CommandContext(ctx, "git", args...) - if strings.TrimSpace(dir) != "" { - cmd.Dir = dir - } - output, err := cmd.CombinedOutput() - if err == nil { - return nil - } - message := strings.TrimSpace(string(output)) - if message == "" { - message = err.Error() - } - return fmt.Errorf("git %s failed: %s", redactGitArgs(args), redactSecrets(message)) -} - -func gitResolve(ctx context.Context, urlValue, ref string) (string, error) { - if err := validateGitOperand("git url", urlValue); err != nil { - return "", err - } - target := strings.TrimSpace(ref) - if target == "" { - target = "HEAD" - } - if err := validateGitOperand("git ref", target); err != nil { - return "", err - } - cmd := exec.CommandContext(ctx, "git", "ls-remote", "--", urlValue, target) - output, err := cmd.Output() - if err != nil || strings.TrimSpace(string(output)) == "" { - if target == "HEAD" { - cmd = exec.CommandContext(ctx, "git", "ls-remote", "--symref", "--", urlValue, "HEAD") - output, err = cmd.Output() - } - } - if err != nil { - return "", err - } - for _, line := range strings.Split(string(output), "\n") { - fields := strings.Fields(line) - if len(fields) >= 2 && len(fields[0]) == 40 { - return fields[0], nil - } - } - return "", fmt.Errorf("could not resolve ref %q", target) -} - -func validateGitOperand(label, value string) error { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - return nil - } - if strings.HasPrefix(trimmed, "-") { - return fmt.Errorf("%s must not start with '-'", label) - } - return nil -} - -func validateGitURLScheme(value string) error { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - return nil - } - if gitRemoteHelperURLPattern.MatchString(trimmed) { - return fmt.Errorf("git remote helper URLs are not supported") - } - parsed, err := url.Parse(trimmed) - if err != nil || parsed.Scheme == "" { - return nil - } - switch strings.ToLower(parsed.Scheme) { - case "http", "https", "ssh", "git", "file": - return nil - default: - return fmt.Errorf("git url scheme %q is not supported", parsed.Scheme) - } -} - func gitCacheURL(raw string) string { trimmed := strings.TrimSpace(raw) parsed, err := url.Parse(trimmed) @@ -956,18 +868,6 @@ func copyWithExpandedLimit(dst io.Writer, src io.Reader, expanded *uint64, limit return nil } -func redactGitArgs(args []string) string { - redacted := make([]string, 0, len(args)) - for _, arg := range args { - redacted = append(redacted, redactSecrets(arg)) - } - return strings.Join(redacted, " ") -} - -func redactSecrets(value string) string { - return credentialURLPattern.ReplaceAllString(value, "$1//xxxxx@") -} - func resolveSecretRefs(value string, env map[string]string) string { if !strings.HasPrefix(value, "${") || !strings.HasSuffix(value, "}") { return value @@ -978,28 +878,3 @@ func resolveSecretRefs(value string, env map[string]string) string { } return os.Getenv(name) } - -func gitURLWithCredentials(raw string, spec domain.AgentSkill, env map[string]string) string { - parsed, err := url.Parse(raw) - if err != nil || parsed.Scheme == "" || parsed.Host == "" { - return raw - } - username := resolveSecretRefs(strings.TrimSpace(spec.Username), env) - password := resolveSecretRefs(strings.TrimSpace(spec.Password), env) - token := resolveSecretRefs(strings.TrimSpace(spec.Token), env) - if token != "" { - if username == "" { - username = "oauth2" - } - parsed.User = url.UserPassword(username, token) - return parsed.String() - } - if username != "" || password != "" { - if password != "" { - parsed.User = url.UserPassword(username, password) - } else { - parsed.User = url.User(username) - } - } - return parsed.String() -} diff --git a/pkg/skills/resolve_test.go b/pkg/skills/resolve_test.go index e4397c48d..e98b819ec 100644 --- a/pkg/skills/resolve_test.go +++ b/pkg/skills/resolve_test.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "encoding/json" + "io" "net/http" "os" "os/exec" @@ -13,6 +14,7 @@ import ( "testing" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" ) func TestResolverResolvesFileSkill(t *testing.T) { @@ -21,7 +23,7 @@ func TestResolverResolvesFileSkill(t *testing.T) { writeSkill(t, source, "pdf") resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{root}} - resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "file", Path: source}}) + resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "file", Path: source}}) if err != nil { t.Fatalf("Resolve returned error: %v", err) } @@ -40,7 +42,7 @@ func TestResolverArtifactManifestOmitsSourcePathAndCredentials(t *testing.T) { writeSkill(t, source, "pdf") resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{root}, Env: map[string]string{"TOKEN": secret}} - resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "file", Path: source, Token: "${TOKEN}"}}) + resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "file", Path: source, Token: "${TOKEN}"}}) if err != nil { t.Fatalf("Resolve returned error: %v", err) } @@ -63,10 +65,10 @@ func TestResolverArtifactManifestOmitsSourcePathAndCredentials(t *testing.T) { func TestResolverResolvesZipSkillSubdir(t *testing.T) { root := t.TempDir() archivePath := filepath.Join(root, "skills.zip") - writeZipSkill(t, archivePath, "skills/pdf", "pdf") + writeZipSkill(t, archivePath, "", "pdf") resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{root}} - resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "zip", URL: archivePath, Path: "skills/pdf"}}) + resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "file", Format: "zip", Path: archivePath}}) if err != nil { t.Fatalf("Resolve returned error: %v", err) } @@ -78,6 +80,31 @@ func TestResolverResolvesZipSkillSubdir(t *testing.T) { } } +func TestResolverResolvesGitSkillWithSharedClient(t *testing.T) { + root := t.TempDir() + repository := filepath.Join(root, "repository") + writeSkill(t, filepath.Join(repository, "skills", "pdf"), "pdf") + runTestGit(t, repository, "init", "-b", "main") + runTestGit(t, repository, "config", "user.email", "agent-compose@example.test") + runTestGit(t, repository, "config", "user.name", "Agent Compose") + runTestGit(t, repository, "add", ".") + runTestGit(t, repository, "commit", "-m", "add skill") + resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{root}} + + resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{ + Name: "pdf", Provider: "git", URL: repository, Ref: "main", Path: "skills/pdf", + }}) + if err != nil { + t.Fatalf("Resolve returned error: %v", err) + } + if len(resolved) != 1 { + t.Fatalf("resolved = %#v", resolved) + } + if _, err := os.Stat(filepath.Join(resolved[0].LocalDir, "SKILL.md")); err != nil { + t.Fatalf("resolved Git SKILL.md: %v", err) + } +} + func TestResolverRejectsGitSubdirTraversal(t *testing.T) { root := t.TempDir() repo := filepath.Join(root, "repo") @@ -89,7 +116,7 @@ func TestResolverRejectsGitSubdirTraversal(t *testing.T) { runTestGit(t, repo, "commit", "-m", "add skill") resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{root}} - _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "git", URL: repo, Path: "../../.."}}) + _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "git", URL: repo, Path: "../../.."}}) if err == nil { t.Fatalf("expected Resolve to reject git subdir traversal") } @@ -109,7 +136,7 @@ func TestResolverRejectsLocalGitOutsideAllowedRoots(t *testing.T) { for _, rawURL := range []string{outside, "file://" + filepath.ToSlash(outside)} { t.Run(rawURL, func(t *testing.T) { - _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "git", URL: rawURL}}) + _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "git", URL: rawURL}}) if err == nil { t.Fatalf("expected Resolve to reject local git outside allowed roots") } @@ -128,7 +155,7 @@ func TestResolverRejectsRemoteGitPrivateHosts(t *testing.T) { "http://169.254.169.254/repo.git", } { t.Run(rawURL, func(t *testing.T) { - _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "git", URL: rawURL}}) + _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "git", URL: rawURL}}) if err == nil { t.Fatalf("expected Resolve to reject private git host") } @@ -143,9 +170,8 @@ func TestResolverRejectsZipSubdirTraversal(t *testing.T) { root := t.TempDir() archivePath := filepath.Join(root, "skills.zip") writeZipSkill(t, archivePath, "skills/pdf", "pdf") - resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{root}} - _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "zip", URL: archivePath, Path: "../../.."}}) + _, err := safeArtifactSubdir(archivePath, "../../..") if err == nil { t.Fatalf("expected Resolve to reject zip subdir traversal") } @@ -160,7 +186,7 @@ func TestResolverRejectsSkillNameMismatch(t *testing.T) { writeSkill(t, source, "pdf") resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{root}} - if _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "docx", Source: "file", Path: source}}); err == nil { + if _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "docx", Provider: "file", Path: source}}); err == nil { t.Fatalf("expected Resolve to reject mismatched skill name") } } @@ -172,7 +198,7 @@ func TestResolverRejectsLocalSourceOutsideAllowedRoots(t *testing.T) { writeSkill(t, outside, "pdf") resolver := Resolver{CacheRoot: filepath.Join(root, "cache"), LocalSourceRoots: []string{allowed}} - if _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "file", Path: outside}}); err == nil { + if _, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "file", Path: outside}}); err == nil { t.Fatalf("expected Resolve to reject outside local source") } } @@ -184,7 +210,7 @@ func TestResolverAllowsComposeSourceRoot(t *testing.T) { writeSkill(t, source, "pdf") resolver := Resolver{CacheRoot: filepath.Join(root, "cache")} - resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Source: "file", Path: source, SourceRoot: sourceRoot}}) + resolved, err := resolver.Resolve(context.Background(), []domain.AgentSkill{{Name: "pdf", Provider: "file", Path: source, SourceRoot: sourceRoot}}) if err != nil { t.Fatalf("Resolve returned error: %v", err) } @@ -193,16 +219,6 @@ func TestResolverAllowsComposeSourceRoot(t *testing.T) { } } -func TestGitURLWithCredentials(t *testing.T) { - got := gitURLWithCredentials("https://git.example/repo.git", domain.AgentSkill{ - Username: "user", - Token: "${GIT_TOKEN}", - }, map[string]string{"GIT_TOKEN": "secret"}) - if got != "https://user:secret@git.example/repo.git" { - t.Fatalf("git url = %q", got) - } -} - func TestResolveSecretRefsUsesScopedEnvWhenProvided(t *testing.T) { t.Setenv("GIT_TOKEN", "daemon-token") if got := resolveSecretRefs("${GIT_TOKEN}", map[string]string{}); got != "" { @@ -223,47 +239,6 @@ func TestGitCacheURLStripsCredentials(t *testing.T) { } } -func TestValidateGitOperandRejectsOptionLikeValue(t *testing.T) { - for _, value := range []string{"--upload-pack=touch /tmp/pwned", "-c core.sshCommand=bad"} { - t.Run(value, func(t *testing.T) { - if err := validateGitOperand("git url", value); err == nil { - t.Fatalf("expected option-like git operand to be rejected") - } - }) - } -} - -func TestValidateGitURLSchemeRejectsRemoteHelpers(t *testing.T) { - for _, value := range []string{"ext::sh -c id", "hg::https://example.invalid/repo"} { - t.Run(value, func(t *testing.T) { - if err := validateGitURLScheme(value); err == nil { - t.Fatalf("expected git remote helper URL to be rejected") - } - }) - } -} - -func TestValidateGitURLSchemeRejectsUnsupportedSchemes(t *testing.T) { - if err := validateGitURLScheme("ftp://example.invalid/repo.git"); err == nil { - t.Fatalf("expected unsupported git URL scheme to be rejected") - } - for _, value := range []string{ - "https://example.invalid/repo.git", - "https://[2001:db8::1]/repo.git", - "ssh://user@[2001:db8::1]/repo.git", - "https://example.invalid/repo.git?note=a::b", - "ssh://git@example.invalid/repo.git", - "git@example.invalid:org/repo.git", - "/tmp/repo.git", - } { - t.Run(value, func(t *testing.T) { - if err := validateGitURLScheme(value); err != nil { - t.Fatalf("validateGitURLScheme returned error: %v", err) - } - }) - } -} - func TestValidateDownloadURLRejectsPrivateHosts(t *testing.T) { if err := validateDownloadURL("file:///tmp/skill.zip"); err == nil { t.Fatalf("expected non-http scheme to be rejected") @@ -301,11 +276,35 @@ func TestDownloadRejectsRedirectToPrivateHost(t *testing.T) { }, nil })} resolver := Resolver{HTTPClient: client} - if _, _, err := resolver.download(context.Background(), "http://93.184.216.34/skill.zip"); err == nil { + if _, _, err := resolver.download(context.Background(), "http://93.184.216.34/skill.zip", sources.Source{}); err == nil { t.Fatalf("expected redirect to private host to be rejected") } } +func TestDownloadAppliesSourceAuthentication(t *testing.T) { + client := &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) { + if got := request.Header.Get("Authorization"); got != "Bearer skill-secret" { + t.Errorf("Authorization = %q", got) + } + return &http.Response{ + StatusCode: http.StatusOK, + Status: "200 OK", + Header: http.Header{"Content-Type": []string{"application/zip"}}, + Body: io.NopCloser(strings.NewReader("not-a-zip")), + Request: request, + }, nil + })} + resolver := Resolver{HTTPClient: client, Env: map[string]string{"TOKEN": "skill-secret"}} + path, cleanup, err := resolver.download(context.Background(), "https://example.com/skill.zip", sources.Source{Token: "${TOKEN}"}) + if err != nil { + t.Fatalf("download returned error: %v", err) + } + cleanup() + if path == "" { + t.Fatal("download path is empty") + } +} + func TestExtractZipRejectsBackslashTraversal(t *testing.T) { root := t.TempDir() archivePath := filepath.Join(root, "escape.zip") @@ -422,13 +421,6 @@ func TestCopyWithExpandedLimitTracksActualBytes(t *testing.T) { } } -func TestRedactSecrets(t *testing.T) { - got := redactSecrets("fatal: https://user:secret@git.example/repo.git failed") - if strings.Contains(got, "secret") || !strings.Contains(got, "https://xxxxx@git.example") { - t.Fatalf("redacted = %q", got) - } -} - func writeSkill(t *testing.T, dir, name string) { t.Helper() if err := os.MkdirAll(dir, 0o755); err != nil { diff --git a/pkg/sources/git.go b/pkg/sources/git.go new file mode 100644 index 000000000..d01545fa5 --- /dev/null +++ b/pkg/sources/git.go @@ -0,0 +1,223 @@ +package sources + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "net/url" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" +) + +var ( + credentialURLPattern = regexp.MustCompile(`([a-zA-Z][a-zA-Z0-9+.-]*:)//[^@\s]+@`) + gitRemoteHelperURLPattern = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*::`) +) + +type GitClient struct { + Env map[string]string +} + +type ResolvedGit struct { + Commit string +} + +func (c GitClient) Resolve(ctx context.Context, source Source) (ResolvedGit, error) { + source = source.Normalized() + if source.Provider != ProviderGit { + return ResolvedGit{}, fmt.Errorf("git source provider must be %q", ProviderGit) + } + if source.URL == "" { + return ResolvedGit{}, errors.New("git source url is required") + } + if err := validateGitOperand("git url", source.URL); err != nil { + return ResolvedGit{}, err + } + if err := validateGitURLScheme(source.URL); err != nil { + return ResolvedGit{}, err + } + target := source.Ref + if target == "" { + target = "HEAD" + } + if err := validateGitOperand("git ref", target); err != nil { + return ResolvedGit{}, err + } + tempRoot, err := os.MkdirTemp("", "agent-compose-git-resolve-*") + if err != nil { + return ResolvedGit{}, fmt.Errorf("create git ref resolver: %w", err) + } + defer func() { _ = os.RemoveAll(tempRoot) }() + repository := filepath.Join(tempRoot, "repository.git") + if err := c.run(ctx, "", source, "init", "--bare", "--", repository); err != nil { + return ResolvedGit{}, fmt.Errorf("initialize git ref resolver: %w", err) + } + if err := c.run(ctx, repository, source, "fetch", "--depth=1", "--no-tags", "--", source.URL, target); err != nil { + return ResolvedGit{}, fmt.Errorf("resolve git ref %q: %w", target, err) + } + output, err := c.runOutput(ctx, repository, source, "rev-parse", "--verify", "FETCH_HEAD^{commit}") + if err != nil { + return ResolvedGit{}, fmt.Errorf("resolve git ref %q to commit: %w", target, err) + } + commit := strings.TrimSpace(string(output)) + if commit == "" { + return ResolvedGit{}, fmt.Errorf("resolve git ref %q: no commit found", target) + } + return ResolvedGit{Commit: commit}, nil +} + +func (c GitClient) Checkout(ctx context.Context, source Source, destination string) (ResolvedGit, error) { + resolved, err := c.Resolve(ctx, source) + if err != nil { + return ResolvedGit{}, err + } + if err := c.CheckoutCommit(ctx, source, resolved.Commit, destination); err != nil { + return ResolvedGit{}, err + } + return resolved, nil +} + +func (c GitClient) CheckoutCommit(ctx context.Context, source Source, commit, destination string) error { + source = source.Normalized() + if source.Provider != ProviderGit { + return fmt.Errorf("git source provider must be %q", ProviderGit) + } + if source.URL == "" { + return errors.New("git source url is required") + } + commit = strings.TrimSpace(commit) + if commit == "" { + return errors.New("git checkout commit is required") + } + if strings.TrimSpace(destination) == "" { + return errors.New("git checkout destination is required") + } + if err := validateGitOperand("git url", source.URL); err != nil { + return err + } + if err := validateGitURLScheme(source.URL); err != nil { + return err + } + if err := validateGitOperand("git commit", commit); err != nil { + return err + } + if err := c.run(ctx, "", source, "clone", "--no-checkout", "--", source.URL, destination); err != nil { + return fmt.Errorf("clone git source: %w", err) + } + if err := c.run(ctx, destination, source, "checkout", commit); err != nil { + return fmt.Errorf("checkout git source commit %s: %w", commit, err) + } + return nil +} + +func (c GitClient) run(ctx context.Context, dir string, source Source, args ...string) error { + _, err := c.runCombinedOutput(ctx, dir, source, args...) + return err +} + +func (c GitClient) runOutput(ctx context.Context, dir string, source Source, args ...string) ([]byte, error) { + cmd := c.command(ctx, dir, source, args...) + output, err := cmd.Output() + if err == nil { + return output, nil + } + return nil, c.commandError(source, args, output, err) +} + +func (c GitClient) runCombinedOutput(ctx context.Context, dir string, source Source, args ...string) ([]byte, error) { + cmd := c.command(ctx, dir, source, args...) + output, err := cmd.CombinedOutput() + if err == nil { + return output, nil + } + return nil, c.commandError(source, args, output, err) +} + +func (c GitClient) command(ctx context.Context, dir string, source Source, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, "git", args...) + if strings.TrimSpace(dir) != "" { + cmd.Dir = dir + } + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + if header := c.authorizationHeader(source); header != "" { + cmd.Env = append(cmd.Env, + "GIT_CONFIG_COUNT=1", + "GIT_CONFIG_KEY_0=http.extraHeader", + "GIT_CONFIG_VALUE_0="+header, + ) + } + return cmd +} + +func (c GitClient) authorizationHeader(source Source) string { + username := ResolveEnvReference(source.Username, c.Env) + password := ResolveEnvReference(source.Password, c.Env) + token := ResolveEnvReference(source.Token, c.Env) + if token != "" { + if username == "" { + username = "oauth2" + } + password = token + } + if username == "" && password == "" { + return "" + } + encoded := base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) + return "Authorization: Basic " + encoded +} + +func (c GitClient) commandError(source Source, args []string, output []byte, err error) error { + message := strings.TrimSpace(string(output)) + if message == "" { + message = err.Error() + } + for _, secret := range []string{ + ResolveEnvReference(source.Password, c.Env), + ResolveEnvReference(source.Token, c.Env), + } { + if secret != "" { + message = strings.ReplaceAll(message, secret, "xxxxx") + } + } + message = credentialURLPattern.ReplaceAllString(message, "$1//xxxxx@") + return fmt.Errorf("git %s failed: %s", strings.Join(redactGitArgs(args), " "), message) +} + +func redactGitArgs(args []string) []string { + redacted := make([]string, len(args)) + for i, arg := range args { + redacted[i] = credentialURLPattern.ReplaceAllString(arg, "$1//xxxxx@") + } + return redacted +} + +func validateGitOperand(label, value string) error { + if strings.HasPrefix(strings.TrimSpace(value), "-") { + return fmt.Errorf("%s must not start with '-'", label) + } + return nil +} + +func validateGitURLScheme(value string) error { + value = strings.TrimSpace(value) + if gitRemoteHelperURLPattern.MatchString(value) { + return errors.New("git remote helper URLs are not supported") + } + parsed, err := url.Parse(value) + if err != nil || parsed.Scheme == "" { + return nil + } + if parsed.User != nil { + return errors.New("git URL userinfo is not supported") + } + switch strings.ToLower(parsed.Scheme) { + case "http", "https", "ssh", "git", "file": + return nil + default: + return fmt.Errorf("git url scheme %q is not supported", parsed.Scheme) + } +} diff --git a/pkg/sources/git_test.go b/pkg/sources/git_test.go new file mode 100644 index 000000000..a3420b7a8 --- /dev/null +++ b/pkg/sources/git_test.go @@ -0,0 +1,141 @@ +package sources + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestGitClientResolveAndCheckout(t *testing.T) { + repository := t.TempDir() + runTestGit(t, "", "init", "-b", "main", repository) + runTestGit(t, repository, "config", "user.email", "agent-compose@example.test") + runTestGit(t, repository, "config", "user.name", "Agent Compose") + if err := os.WriteFile(filepath.Join(repository, "script.js"), []byte("scheduler.agent('review');\n"), 0o644); err != nil { + t.Fatal(err) + } + runTestGit(t, repository, "add", "script.js") + runTestGit(t, repository, "commit", "-m", "add script") + wantCommit := strings.TrimSpace(runTestGit(t, repository, "rev-parse", "HEAD")) + + source := Source{Provider: ProviderGit, URL: repository, Ref: "main"} + client := GitClient{} + resolved, err := client.Resolve(context.Background(), source) + if err != nil { + t.Fatalf("Resolve returned error: %v", err) + } + if resolved.Commit != wantCommit { + t.Fatalf("commit = %q, want %q", resolved.Commit, wantCommit) + } + destination := filepath.Join(t.TempDir(), "checkout") + if err := client.CheckoutCommit(context.Background(), source, resolved.Commit, destination); err != nil { + t.Fatalf("CheckoutCommit returned error: %v", err) + } + data, err := os.ReadFile(filepath.Join(destination, "script.js")) + if err != nil || !strings.Contains(string(data), "scheduler.agent") { + t.Fatalf("checked out script = %q, err=%v", data, err) + } +} + +func TestGitClientResolvesBranchTagAndCommit(t *testing.T) { + repository := t.TempDir() + runTestGit(t, "", "init", "-b", "main", repository) + runTestGit(t, repository, "config", "user.email", "agent-compose@example.test") + runTestGit(t, repository, "config", "user.name", "Agent Compose") + if err := os.WriteFile(filepath.Join(repository, "README.md"), []byte("one\n"), 0o644); err != nil { + t.Fatal(err) + } + runTestGit(t, repository, "add", "README.md") + runTestGit(t, repository, "commit", "-m", "first") + firstCommit := strings.TrimSpace(runTestGit(t, repository, "rev-parse", "HEAD")) + runTestGit(t, repository, "tag", "-a", "v1.0.0", "-m", "release") + if err := os.WriteFile(filepath.Join(repository, "README.md"), []byte("two\n"), 0o644); err != nil { + t.Fatal(err) + } + runTestGit(t, repository, "commit", "-am", "second") + branchCommit := strings.TrimSpace(runTestGit(t, repository, "rev-parse", "HEAD")) + + client := GitClient{} + for _, test := range []struct { + name string + ref string + want string + }{ + {name: "branch", ref: "main", want: branchCommit}, + {name: "annotated tag", ref: "v1.0.0", want: firstCommit}, + {name: "commit", ref: firstCommit, want: firstCommit}, + } { + t.Run(test.name, func(t *testing.T) { + resolved, err := client.Resolve(context.Background(), Source{Provider: ProviderGit, URL: repository, Ref: test.ref}) + if err != nil { + t.Fatalf("Resolve returned error: %v", err) + } + if resolved.Commit != test.want { + t.Fatalf("commit = %q, want %q", resolved.Commit, test.want) + } + }) + } + if _, err := client.Resolve(context.Background(), Source{Provider: ProviderGit, URL: repository, Ref: "missing"}); err == nil { + t.Fatal("Resolve missing ref returned nil error") + } +} + +func TestGitClientRejectsUnsafeOperandsAndSchemes(t *testing.T) { + client := GitClient{} + for _, source := range []Source{ + {Provider: ProviderGit, URL: "--upload-pack=bad"}, + {Provider: ProviderGit, URL: "ext::sh -c id"}, + {Provider: ProviderGit, URL: "ftp://example.invalid/repo.git"}, + {Provider: ProviderGit, URL: "https://user:secret@example.invalid/repo.git"}, + {Provider: ProviderGit, URL: "https://example.invalid/repo.git", Ref: "--bad"}, + } { + if _, err := client.Resolve(context.Background(), source); err == nil { + t.Fatalf("Resolve(%#v) returned nil error", source) + } + } +} + +func TestGitClientInjectsCredentialsOutsideArguments(t *testing.T) { + const secret = "super-secret-token" + client := GitClient{Env: map[string]string{"TOKEN": secret}} + source := Source{Provider: ProviderGit, URL: "https://example.invalid/repo.git", Token: "${TOKEN}"} + cmd := client.command(context.Background(), "", source, "ls-remote", "--", source.URL, "HEAD") + if strings.Contains(strings.Join(cmd.Args, " "), secret) { + t.Fatalf("git arguments contain credential: %#v", cmd.Args) + } + joinedEnv := strings.Join(cmd.Env, "\n") + if !strings.Contains(joinedEnv, "GIT_CONFIG_VALUE_0=Authorization: Basic ") || strings.Contains(joinedEnv, source.Token) { + t.Fatalf("git credential environment is not resolved safely") + } +} + +func TestGitClientRedactsCredentialsFromErrors(t *testing.T) { + const secret = "super-secret-token" + client := GitClient{Env: map[string]string{"TOKEN": secret}} + source := Source{Provider: ProviderGit, URL: "https://example.invalid/repo.git", Token: "${TOKEN}"} + err := client.commandError( + source, + []string{"clone", "https://user:" + secret + "@example.invalid/repo.git"}, + []byte("fatal: authentication failed for https://user:"+secret+"@example.invalid/repo.git"), + errors.New("exit status 128"), + ) + message := err.Error() + if strings.Contains(message, secret) || strings.Contains(message, "user:") || !strings.Contains(message, "xxxxx") { + t.Fatalf("redacted error = %q", message) + } +} + +func runTestGit(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, output) + } + return string(output) +} diff --git a/pkg/sources/source.go b/pkg/sources/source.go new file mode 100644 index 000000000..c747790fc --- /dev/null +++ b/pkg/sources/source.go @@ -0,0 +1,96 @@ +package sources + +import ( + "fmt" + "net/http" + "os" + "regexp" + "strings" +) + +const ( + ProviderFile = "file" + ProviderGit = "git" + ProviderHTTP = "http" + + FormatZIP = "zip" +) + +var exactEnvReferencePattern = regexp.MustCompile(`^\$\{[A-Za-z_][A-Za-z0-9_]*\}$`) + +// Source describes where content is obtained and how it is interpreted. +// Compose authoring types expose these fields inline in their owning object. +type Source struct { + Provider string `yaml:"provider,omitempty" json:"provider,omitempty"` + URL string `yaml:"url,omitempty" json:"url,omitempty"` + Ref string `yaml:"ref,omitempty" json:"ref,omitempty"` + Path string `yaml:"path,omitempty" json:"path,omitempty"` + Format string `yaml:"format,omitempty" json:"format,omitempty"` + Username string `yaml:"username,omitempty" json:"username,omitempty"` + Password string `yaml:"password,omitempty" json:"password,omitempty"` + Token string `yaml:"token,omitempty" json:"token,omitempty"` +} + +func (s Source) Normalized() Source { + s.Provider = strings.ToLower(strings.TrimSpace(s.Provider)) + s.URL = strings.TrimSpace(s.URL) + s.Ref = strings.TrimSpace(s.Ref) + s.Path = strings.TrimSpace(s.Path) + s.Format = strings.ToLower(strings.TrimSpace(s.Format)) + s.Username = strings.TrimSpace(s.Username) + s.Password = strings.TrimSpace(s.Password) + s.Token = strings.TrimSpace(s.Token) + return s +} + +func (s Source) HasContent() bool { + s = s.Normalized() + return s.Provider != "" || s.URL != "" || s.Ref != "" || s.Path != "" || s.Format != "" || + s.Username != "" || s.Password != "" || s.Token != "" +} + +func (s Source) HasAuthentication() bool { + s = s.Normalized() + return s.Username != "" || s.Password != "" || s.Token != "" +} + +func ValidateSecretReference(field, value string) error { + value = strings.TrimSpace(value) + if value == "" { + return nil + } + if !exactEnvReferencePattern.MatchString(value) { + return fmt.Errorf("%s must be an environment reference like ${NAME}", field) + } + return nil +} + +func ResolveEnvReference(value string, env map[string]string) string { + value = strings.TrimSpace(value) + if !exactEnvReferencePattern.MatchString(value) { + return value + } + name := strings.TrimSuffix(strings.TrimPrefix(value, "${"), "}") + if env != nil { + return env[name] + } + return os.Getenv(name) +} + +// ApplyHTTPAuthentication applies Source authentication without placing +// credentials in the request URL. A token takes precedence over basic auth. +func ApplyHTTPAuthentication(request *http.Request, source Source, env map[string]string) { + if request == nil { + return + } + username := ResolveEnvReference(source.Username, env) + password := ResolveEnvReference(source.Password, env) + token := ResolveEnvReference(source.Token, env) + if token != "" { + request.Header.Set("Authorization", "Bearer "+token) + return + } + if username != "" || password != "" { + request.SetBasicAuth(username, password) + } +} diff --git a/pkg/storage/configstore/schema_coverage_test.go b/pkg/storage/configstore/schema_coverage_test.go index 33b21eda1..d76eb9e36 100644 --- a/pkg/storage/configstore/schema_coverage_test.go +++ b/pkg/storage/configstore/schema_coverage_test.go @@ -665,7 +665,7 @@ func testConfigStoreCRUDCoverageWorkflows(t *testing.T) { ID: "agent-1", Name: "Agent", Enabled: true, Provider: "codex", Model: "gpt", SystemPrompt: "prompt", Driver: driverpkg.RuntimeDriverBoxlite, GuestImage: "guest:latest", WorkspaceID: workspace.ID, EnvItems: []domain.SandboxEnvVar{{Name: "TOKEN", Value: "secret", Secret: true}}, CapsetIDs: []string{"dev"}, - Skills: []domain.AgentSkill{{Name: "pdf", Source: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/pdf", Token: "${GIT_TOKEN}"}}, + Skills: []domain.AgentSkill{{Name: "pdf", Provider: "git", URL: "https://github.com/anthropics/skills.git", Path: "skills/pdf", Token: "${GIT_TOKEN}"}}, }) if err != nil { t.Fatalf("CreateAgentDefinition returned error: %v", err) @@ -696,7 +696,7 @@ func testConfigStoreCRUDCoverageWorkflows(t *testing.T) { } managedAgent, err := store.UpsertManagedAgentDefinition(ctx, domain.AgentDefinition{ ID: "managed-agent-1", Name: "Managed", Enabled: true, Provider: "codex", ManagedProjectID: "project-1", ManagedAgentName: "worker", ManagedProjectRevision: 1, - Skills: []domain.AgentSkill{{Name: "local-review", Source: "file", Path: "/tmp/skills/local-review"}}, + Skills: []domain.AgentSkill{{Name: "local-review", Provider: "file", Path: "/tmp/skills/local-review"}}, }) if err != nil { t.Fatalf("UpsertManagedAgentDefinition returned error: %v", err) diff --git a/pkg/workspaces/git_workspace.go b/pkg/workspaces/git_workspace.go index b434ab883..8b6e133fa 100644 --- a/pkg/workspaces/git_workspace.go +++ b/pkg/workspaces/git_workspace.go @@ -4,25 +4,19 @@ import ( "context" "encoding/json" "fmt" - "net/url" "os" - "os/exec" "path/filepath" "strings" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" ) const GitWorkspaceTempDirName = ".agent-compose-git-clone" type GitWorkspaceConfig struct { - URL string `json:"url"` - Branch string `json:"branch,omitempty"` - Commit string `json:"commit,omitempty"` - Credential string `json:"credential,omitempty"` - Username string `json:"username,omitempty"` - Password string `json:"password,omitempty"` - CloneTarget string `json:"path,omitempty"` + sources.Source + Target string `json:"target,omitempty"` } type gitWorkspace struct { @@ -38,12 +32,14 @@ func (w gitWorkspace) Prepare(ctx context.Context, session *domain.Sandbox) erro if err := json.Unmarshal([]byte(w.workspace.ConfigJSON), &cfg); err != nil { return fmt.Errorf("decode workspace config %s: %w", w.workspace.ID, err) } - cloneURL := strings.TrimSpace(cfg.URL) - if cloneURL == "" { + cfg.Source = cfg.Normalized() + if cfg.Provider == "" { + cfg.Provider = sources.ProviderGit + } + if cfg.URL == "" { return fmt.Errorf("workspace config %s missing git url", w.workspace.ID) } - cloneURL = ApplyGitCredentials(cloneURL, cfg) - cloneTarget, err := NormalizeGitCloneTarget(w.workspace.ID, cfg.CloneTarget) + target, err := NormalizeWorkspaceTarget(w.workspace.ID, cfg.Target) if err != nil { return err } @@ -57,32 +53,29 @@ func (w gitWorkspace) Prepare(ctx context.Context, session *domain.Sandbox) erro if err := cleanupGitCloneTempDir(workspaceRoot); err != nil { return fmt.Errorf("prepare workspace %s failed: %w", w.workspace.Name, err) } - if cloneTarget == "." { - if err := cloneGitWorkspaceRoot(ctx, workspaceRoot, cloneURL, cfg); err != nil { + if target == "." { + if err := cloneGitWorkspaceRoot(ctx, workspaceRoot, cfg.Source); err != nil { return fmt.Errorf("prepare workspace %s failed: %w", w.workspace.Name, err) } return nil } - clonePath := filepath.Join(workspaceRoot, cloneTarget) + clonePath := filepath.Join(workspaceRoot, target) if err := os.MkdirAll(filepath.Dir(clonePath), 0o755); err != nil { return fmt.Errorf("prepare workspace %s failed: create clone parent: %w", w.workspace.Name, err) } - if err := gitClone(ctx, cloneURL, cfg, clonePath); err != nil { - return fmt.Errorf("prepare workspace %s failed: %w", w.workspace.Name, err) - } - if err := gitCheckoutCommit(ctx, clonePath, cfg); err != nil { + if _, err := (sources.GitClient{}).Checkout(ctx, cfg.Source, clonePath); err != nil { return fmt.Errorf("prepare workspace %s failed: %w", w.workspace.Name, err) } return nil } -func NormalizeGitCloneTarget(workspaceID, raw string) (string, error) { +func NormalizeWorkspaceTarget(workspaceID, raw string) (string, error) { trimmed := strings.TrimSpace(raw) if trimmed == "" { return ".", nil } if filepath.IsAbs(trimmed) { - return "", fmt.Errorf("workspace config %s has invalid clone path %q", workspaceID, trimmed) + return "", fmt.Errorf("workspace config %s has invalid target %q", workspaceID, trimmed) } clean := filepath.Clean(trimmed) if clean == "." { @@ -90,7 +83,7 @@ func NormalizeGitCloneTarget(workspaceID, raw string) (string, error) { } parentPrefix := ".." + string(filepath.Separator) if clean == ".." || strings.HasPrefix(clean, parentPrefix) { - return "", fmt.Errorf("workspace config %s has invalid clone path %q", workspaceID, trimmed) + return "", fmt.Errorf("workspace config %s has invalid target %q", workspaceID, trimmed) } return clean, nil } @@ -103,12 +96,9 @@ func cleanupGitCloneTempDir(workspaceRoot string) error { return nil } -func cloneGitWorkspaceRoot(ctx context.Context, workspaceRoot, cloneURL string, cfg GitWorkspaceConfig) error { +func cloneGitWorkspaceRoot(ctx context.Context, workspaceRoot string, source sources.Source) error { tempDir := filepath.Join(workspaceRoot, GitWorkspaceTempDirName) - if err := gitClone(ctx, cloneURL, cfg, tempDir); err != nil { - return err - } - if err := gitCheckoutCommit(ctx, tempDir, cfg); err != nil { + if _, err := (sources.GitClient{}).Checkout(ctx, source, tempDir); err != nil { _ = os.RemoveAll(tempDir) return err } @@ -168,95 +158,3 @@ func MoveWorkspaceEntry(src, dst string) error { } return nil } - -func gitClone(ctx context.Context, cloneURL string, cfg GitWorkspaceConfig, clonePath string) error { - return runGitCommand(ctx, "", "git clone", GitCloneArgs(cloneURL, cfg, clonePath)...) -} - -func GitCloneArgs(cloneURL string, cfg GitWorkspaceConfig, clonePath string) []string { - args := []string{"clone", "--depth", "1"} - if branch := strings.TrimSpace(cfg.Branch); branch != "" { - args = append(args, "--branch", branch) - } - args = append(args, cloneURL, clonePath) - return args -} - -func gitCheckoutCommit(ctx context.Context, clonePath string, cfg GitWorkspaceConfig) error { - commit := strings.TrimSpace(cfg.Commit) - if commit == "" { - return nil - } - // Fast path: fetch only the requested object so the clone stays shallow. - // Works when commit is a full SHA or an exact ref name and the server allows - // fetching it directly (e.g. uploadpack.allowReachableSHA1InWant). - if err := runGitCommand(ctx, clonePath, "git fetch", GitCommitFetchArgs(commit)...); err == nil { - return runGitCommand(ctx, clonePath, "git checkout", "checkout", "FETCH_HEAD") - } - // Fallback: the object could not be fetched directly. This happens for an - // abbreviated SHA (git treats it as a ref name), a commit that is not a ref - // tip, or a server that rejects by-SHA fetches. Deepen all branches and tags - // so any reachable commit, abbreviated SHA, or tag resolves locally. - if err := runGitCommand(ctx, clonePath, "git fetch", GitDeepenFetchArgs(true)...); err != nil { - // --unshallow only applies to a shallow repo; retry without it. - if err := runGitCommand(ctx, clonePath, "git fetch", GitDeepenFetchArgs(false)...); err != nil { - return err - } - } - return runGitCommand(ctx, clonePath, "git checkout", "checkout", commit) -} - -func GitCommitFetchArgs(commit string) []string { - return []string{"fetch", "--depth", "1", "origin", commit} -} - -func GitDeepenFetchArgs(unshallow bool) []string { - args := []string{"fetch"} - if unshallow { - args = append(args, "--unshallow") - } - return append(args, "--tags", "origin", "+refs/heads/*:refs/remotes/origin/*") -} - -func runGitCommand(ctx context.Context, dir, action string, args ...string) error { - cmd := exec.CommandContext(ctx, "git", args...) - if strings.TrimSpace(dir) != "" { - cmd.Dir = dir - } - output, err := cmd.CombinedOutput() - if err == nil { - return nil - } - message := strings.TrimSpace(string(output)) - if message == "" { - message = err.Error() - } - return fmt.Errorf("%s failed: %s", action, message) -} - -func ApplyGitCredentials(cloneURL string, cfg GitWorkspaceConfig) string { - trimmedURL := strings.TrimSpace(cloneURL) - if trimmedURL == "" { - return "" - } - credential := strings.TrimSpace(cfg.Credential) - if credential == "" { - user := strings.TrimSpace(cfg.Username) - pass := strings.TrimSpace(cfg.Password) - if user != "" || pass != "" { - credential = url.QueryEscape(user) + ":" + url.QueryEscape(pass) - } - } - if credential == "" { - return trimmedURL - } - if strings.Contains(trimmedURL, "@") { - return trimmedURL - } - for _, prefix := range []string{"https://", "http://"} { - if strings.HasPrefix(trimmedURL, prefix) { - return prefix + credential + "@" + strings.TrimPrefix(trimmedURL, prefix) - } - } - return trimmedURL -} diff --git a/pkg/workspaces/provisioner_git_state_test.go b/pkg/workspaces/provisioner_git_state_test.go index d6d788d1d..da068abce 100644 --- a/pkg/workspaces/provisioner_git_state_test.go +++ b/pkg/workspaces/provisioner_git_state_test.go @@ -17,6 +17,7 @@ import ( appconfig "agent-compose/pkg/config" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" ) func TestProvisionerGitWorkspaceReadyPreservesState(t *testing.T) { @@ -141,9 +142,7 @@ func newProvisionerGitStateFixture(t *testing.T) *provisionerGitStateFixture { sourceTip := strings.TrimSpace(runProvisionerGitStateCommand(t, sourcePath, "rev-parse", "HEAD")) configJSON, err := json.Marshal(GitWorkspaceConfig{ - URL: "file://" + filepath.ToSlash(sourcePath), - Branch: branch, - Commit: commit, + Source: sources.Source{Provider: sources.ProviderGit, URL: "file://" + filepath.ToSlash(sourcePath), Ref: commit}, }) if err != nil { t.Fatalf("marshal Git workspace config: %v", err) diff --git a/pkg/workspaces/workspace_coverage_test.go b/pkg/workspaces/workspace_coverage_test.go index ba47bd62e..48a291fbb 100644 --- a/pkg/workspaces/workspace_coverage_test.go +++ b/pkg/workspaces/workspace_coverage_test.go @@ -15,6 +15,7 @@ import ( appconfig "agent-compose/pkg/config" domain "agent-compose/pkg/model" + "agent-compose/pkg/sources" ) func TestWorkspaceFileAndPathWorkflows(t *testing.T) { @@ -303,33 +304,17 @@ func TestWorkspaceGitHelpers(t *testing.T) { {raw: "../repo", wantErr: true}, {raw: "repo/../../escape", wantErr: true}, } { - got, err := NormalizeGitCloneTarget("ws-1", tc.raw) + got, err := NormalizeWorkspaceTarget("ws-1", tc.raw) if tc.wantErr { if err == nil { - t.Fatalf("NormalizeGitCloneTarget(%q) returned nil error", tc.raw) + t.Fatalf("NormalizeWorkspaceTarget(%q) returned nil error", tc.raw) } continue } if err != nil || got != tc.want { - t.Fatalf("NormalizeGitCloneTarget(%q) = %q/%v, want %q", tc.raw, got, err, tc.want) + t.Fatalf("NormalizeWorkspaceTarget(%q) = %q/%v, want %q", tc.raw, got, err, tc.want) } } - if got := GitCloneArgs("https://example.test/repo.git", GitWorkspaceConfig{Branch: "main"}, "/tmp/workspace"); strings.Join(got, "\x00") != strings.Join([]string{"clone", "--depth", "1", "--branch", "main", "https://example.test/repo.git", "/tmp/workspace"}, "\x00") { - t.Fatalf("GitCloneArgs = %#v", got) - } - if got := GitCommitFetchArgs("e413509"); strings.Join(got, "\x00") != strings.Join([]string{"fetch", "--depth", "1", "origin", "e413509"}, "\x00") { - t.Fatalf("GitCommitFetchArgs = %#v", got) - } - if !strings.Contains(strings.Join(GitDeepenFetchArgs(true), " "), "--unshallow") || strings.Contains(strings.Join(GitDeepenFetchArgs(false), " "), "--unshallow") { - t.Fatalf("GitDeepenFetchArgs returned unexpected values") - } - if got := ApplyGitCredentials("https://example.test/repo.git", GitWorkspaceConfig{Username: "u ser", Password: "p@ss"}); !strings.Contains(got, "u+ser:p%40ss@") { - t.Fatalf("ApplyGitCredentials username/password = %q", got) - } - if got := ApplyGitCredentials("ssh://example.test/repo.git", GitWorkspaceConfig{Credential: "token"}); got != "ssh://example.test/repo.git" { - t.Fatalf("ApplyGitCredentials ssh = %q", got) - } - src := filepath.Join(t.TempDir(), "src") dst := filepath.Join(t.TempDir(), "dst") if err := os.MkdirAll(filepath.Join(src, "nested"), 0o755); err != nil { @@ -366,9 +351,7 @@ func testWorkspaceGitPrepareWorkflow(t *testing.T) { rootWorkspace := t.TempDir() rootWorkspaceConfig := encodeGitWorkspaceConfigForTest(t, GitWorkspaceConfig{ - URL: cloneURL, - Branch: "main", - Commit: sourceRepo.firstCommit, + Source: sources.Source{Provider: sources.ProviderGit, URL: cloneURL, Ref: sourceRepo.firstCommit}, }) if err := PrepareGitWorkspace(context.Background(), &domain.Sandbox{ Summary: domain.SandboxSummary{ID: "session-git-root", WorkspacePath: rootWorkspace}, @@ -393,9 +376,8 @@ func testWorkspaceGitPrepareWorkflow(t *testing.T) { Name: "Git Nested Workspace", Type: "git", ConfigJSON: encodeGitWorkspaceConfigForTest(t, GitWorkspaceConfig{ - URL: cloneURL, - Branch: "main", - CloneTarget: "nested/repo", + Source: sources.Source{Provider: sources.ProviderGit, URL: cloneURL, Ref: "main"}, + Target: "nested/repo", }), }); err != nil { t.Fatalf("PrepareGitWorkspace nested returned error: %v", err) diff --git a/proto/agentcompose/v2/agentcompose.pb.go b/proto/agentcompose/v2/agentcompose.pb.go index 0e2045913..714b84f09 100644 --- a/proto/agentcompose/v2/agentcompose.pb.go +++ b/proto/agentcompose/v2/agentcompose.pb.go @@ -5366,10 +5366,14 @@ type WorkspaceSpec struct { state protoimpl.MessageState `protogen:"open.v1"` Provider string `protobuf:"bytes,1,opt,name=provider,proto3" json:"provider,omitempty"` Url string `protobuf:"bytes,2,opt,name=url,proto3" json:"url,omitempty"` - Branch string `protobuf:"bytes,3,opt,name=branch,proto3" json:"branch,omitempty"` + Ref string `protobuf:"bytes,3,opt,name=ref,proto3" json:"ref,omitempty"` Path string `protobuf:"bytes,4,opt,name=path,proto3" json:"path,omitempty"` Name string `protobuf:"bytes,5,opt,name=name,proto3" json:"name,omitempty"` - Commit string `protobuf:"bytes,6,opt,name=commit,proto3" json:"commit,omitempty"` + Format string `protobuf:"bytes,6,opt,name=format,proto3" json:"format,omitempty"` + Target string `protobuf:"bytes,7,opt,name=target,proto3" json:"target,omitempty"` + Username string `protobuf:"bytes,8,opt,name=username,proto3" json:"username,omitempty"` + Password string `protobuf:"bytes,9,opt,name=password,proto3" json:"password,omitempty"` + Token string `protobuf:"bytes,10,opt,name=token,proto3" json:"token,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -5418,9 +5422,9 @@ func (x *WorkspaceSpec) GetUrl() string { return "" } -func (x *WorkspaceSpec) GetBranch() string { +func (x *WorkspaceSpec) GetRef() string { if x != nil { - return x.Branch + return x.Ref } return "" } @@ -5439,9 +5443,37 @@ func (x *WorkspaceSpec) GetName() string { return "" } -func (x *WorkspaceSpec) GetCommit() string { +func (x *WorkspaceSpec) GetFormat() string { + if x != nil { + return x.Format + } + return "" +} + +func (x *WorkspaceSpec) GetTarget() string { + if x != nil { + return x.Target + } + return "" +} + +func (x *WorkspaceSpec) GetUsername() string { + if x != nil { + return x.Username + } + return "" +} + +func (x *WorkspaceSpec) GetPassword() string { if x != nil { - return x.Commit + return x.Password + } + return "" +} + +func (x *WorkspaceSpec) GetToken() string { + if x != nil { + return x.Token } return "" } @@ -13875,13 +13907,14 @@ func (x *StartRunResponse) GetStarted() bool { type SkillSpec struct { state protoimpl.MessageState `protogen:"open.v1"` Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` - Source string `protobuf:"bytes,2,opt,name=source,proto3" json:"source,omitempty"` + Provider string `protobuf:"bytes,2,opt,name=provider,proto3" json:"provider,omitempty"` Url string `protobuf:"bytes,3,opt,name=url,proto3" json:"url,omitempty"` Path string `protobuf:"bytes,4,opt,name=path,proto3" json:"path,omitempty"` Ref string `protobuf:"bytes,5,opt,name=ref,proto3" json:"ref,omitempty"` Username string `protobuf:"bytes,6,opt,name=username,proto3" json:"username,omitempty"` Password string `protobuf:"bytes,7,opt,name=password,proto3" json:"password,omitempty"` Token string `protobuf:"bytes,8,opt,name=token,proto3" json:"token,omitempty"` + Format string `protobuf:"bytes,9,opt,name=format,proto3" json:"format,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -13923,9 +13956,9 @@ func (x *SkillSpec) GetName() string { return "" } -func (x *SkillSpec) GetSource() string { +func (x *SkillSpec) GetProvider() string { if x != nil { - return x.Source + return x.Provider } return "" } @@ -13972,6 +14005,13 @@ func (x *SkillSpec) GetToken() string { return "" } +func (x *SkillSpec) GetFormat() string { + if x != nil { + return x.Format + } + return "" +} + type ResolveResourceIDRequest struct { state protoimpl.MessageState `protogen:"open.v1"` Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` @@ -16854,14 +16894,19 @@ const file_agentcompose_v2_agentcompose_proto_rawDesc = "" + "\x04name\x18\x01 \x01(\tR\x04name\x12\x19\n" + "\x05value\x18\x02 \x01(\tH\x00R\x05value\x88\x01\x01\x12\x16\n" + "\x06secret\x18\x03 \x01(\bR\x06secretB\b\n" + - "\x06_value\"\x95\x01\n" + + "\x06_value\"\xf5\x01\n" + "\rWorkspaceSpec\x12\x1a\n" + "\bprovider\x18\x01 \x01(\tR\bprovider\x12\x10\n" + - "\x03url\x18\x02 \x01(\tR\x03url\x12\x16\n" + - "\x06branch\x18\x03 \x01(\tR\x06branch\x12\x12\n" + + "\x03url\x18\x02 \x01(\tR\x03url\x12\x10\n" + + "\x03ref\x18\x03 \x01(\tR\x03ref\x12\x12\n" + "\x04path\x18\x04 \x01(\tR\x04path\x12\x12\n" + "\x04name\x18\x05 \x01(\tR\x04name\x12\x16\n" + - "\x06commit\x18\x06 \x01(\tR\x06commit\"!\n" + + "\x06format\x18\x06 \x01(\tR\x06format\x12\x16\n" + + "\x06target\x18\a \x01(\tR\x06target\x12\x1a\n" + + "\busername\x18\b \x01(\tR\busername\x12\x1a\n" + + "\bpassword\x18\t \x01(\tR\bpassword\x12\x14\n" + + "\x05token\x18\n" + + " \x01(\tR\x05token\"!\n" + "\vNetworkSpec\x12\x12\n" + "\x04mode\x18\x01 \x01(\tR\x04mode\"\xe7\x01\n" + "\rSchedulerSpec\x12\x18\n" + @@ -17634,16 +17679,17 @@ const file_agentcompose_v2_agentcompose_proto_rawDesc = "" + "\x10StartRunResponse\x12-\n" + "\x03run\x18\x01 \x01(\v2\x1b.agentcompose.v2.RunSummaryR\x03run\x12\x1a\n" + "\bwarnings\x18\x02 \x03(\tR\bwarnings\x12\x18\n" + - "\astarted\x18\x03 \x01(\bR\astarted\"\xbd\x01\n" + + "\astarted\x18\x03 \x01(\bR\astarted\"\xd9\x01\n" + "\tSkillSpec\x12\x12\n" + - "\x04name\x18\x01 \x01(\tR\x04name\x12\x16\n" + - "\x06source\x18\x02 \x01(\tR\x06source\x12\x10\n" + + "\x04name\x18\x01 \x01(\tR\x04name\x12\x1a\n" + + "\bprovider\x18\x02 \x01(\tR\bprovider\x12\x10\n" + "\x03url\x18\x03 \x01(\tR\x03url\x12\x12\n" + "\x04path\x18\x04 \x01(\tR\x04path\x12\x10\n" + "\x03ref\x18\x05 \x01(\tR\x03ref\x12\x1a\n" + "\busername\x18\x06 \x01(\tR\busername\x12\x1a\n" + "\bpassword\x18\a \x01(\tR\bpassword\x12\x14\n" + - "\x05token\x18\b \x01(\tR\x05token\"_\n" + + "\x05token\x18\b \x01(\tR\x05token\x12\x16\n" + + "\x06format\x18\t \x01(\tR\x06format\"_\n" + "\x18ResolveResourceIDRequest\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x123\n" + "\x05kinds\x18\x02 \x03(\x0e2\x1d.agentcompose.v2.ResourceKindR\x05kinds\"r\n" + diff --git a/proto/agentcompose/v2/agentcompose.proto b/proto/agentcompose/v2/agentcompose.proto index f56c72f80..f8ba7ec48 100644 --- a/proto/agentcompose/v2/agentcompose.proto +++ b/proto/agentcompose/v2/agentcompose.proto @@ -725,10 +725,14 @@ message EnvVarUpdateSpec { message WorkspaceSpec { string provider = 1; string url = 2; - string branch = 3; + string ref = 3; string path = 4; string name = 5; - string commit = 6; + string format = 6; + string target = 7; + string username = 8; + string password = 9; + string token = 10; } message NetworkSpec { @@ -1611,13 +1615,14 @@ message StartRunResponse { message SkillSpec { string name = 1; - string source = 2; + string provider = 2; string url = 3; string path = 4; string ref = 5; string username = 6; string password = 7; string token = 8; + string format = 9; } message ResolveResourceIDRequest { diff --git a/test/e2e/docker_workspace_resume_host_daemon_test.go b/test/e2e/docker_workspace_resume_host_daemon_test.go index e11cd2e59..e6ab53ad9 100644 --- a/test/e2e/docker_workspace_resume_host_daemon_test.go +++ b/test/e2e/docker_workspace_resume_host_daemon_test.go @@ -78,7 +78,7 @@ func TestE2EDockerFileWorkspaceResumePreservesState(t *testing.T) { Workspaces: []*agentcomposev2.NamedWorkspaceSpec{{ Name: "source", Workspace: &agentcomposev2.WorkspaceSpec{ - Provider: "local", + Provider: "file", Path: ".", }, }}, From 5133ece8b4e4ddbabbaed622a2bd43f29b468347 Mon Sep 17 00:00:00 2001 From: "chencong.fu@chaitin.com" Date: Fri, 17 Jul 2026 22:00:44 +0800 Subject: [PATCH 2/2] fix(compose): contain git scheduler script paths --- pkg/compose/script_source.go | 26 ++++++++++++++++++++- pkg/compose/script_source_test.go | 39 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/pkg/compose/script_source.go b/pkg/compose/script_source.go index 2c0c6eecb..6522d642d 100644 --- a/pkg/compose/script_source.go +++ b/pkg/compose/script_source.go @@ -175,7 +175,31 @@ func (r *defaultScriptSourceResolver) readGit(ctx context.Context, source source if _, err := (sources.GitClient{Env: r.env}).Checkout(ctx, source, checkoutDir); err != nil { return nil, err } - return readScriptFileWithContext(ctx, filepath.Join(checkoutDir, filepath.FromSlash(source.Path))) + scriptPath, err := resolveGitScriptPath(checkoutDir, source.Path) + if err != nil { + return nil, err + } + return readScriptFileWithContext(ctx, scriptPath) +} + +func resolveGitScriptPath(checkoutDir, sourcePath string) (string, error) { + root, err := filepath.EvalSymlinks(checkoutDir) + if err != nil { + return "", fmt.Errorf("resolve git checkout directory: %w", err) + } + resolved, err := filepath.EvalSymlinks(filepath.Join(root, filepath.FromSlash(sourcePath))) + if err != nil { + return "", fmt.Errorf("resolve git script path %q: %w", sourcePath, err) + } + relative, err := filepath.Rel(root, resolved) + if err != nil { + return "", fmt.Errorf("resolve git script path %q relative to checkout: %w", sourcePath, err) + } + parentPrefix := ".." + string(filepath.Separator) + if filepath.IsAbs(relative) || relative == ".." || strings.HasPrefix(relative, parentPrefix) { + return "", fmt.Errorf("git script path %q must stay within the repository", sourcePath) + } + return resolved, nil } func readScriptFileWithContext(ctx context.Context, path string) ([]byte, error) { diff --git a/pkg/compose/script_source_test.go b/pkg/compose/script_source_test.go index 31285ade9..3847d7896 100644 --- a/pkg/compose/script_source_test.go +++ b/pkg/compose/script_source_test.go @@ -76,6 +76,45 @@ func TestDefaultScriptSourceResolverReadsGitFile(t *testing.T) { } } +func TestDefaultScriptSourceResolverRejectsEscapingGitSymlink(t *testing.T) { + repository := t.TempDir() + for _, args := range [][]string{ + {"init", "-b", "main"}, + {"config", "user.email", "agent-compose@example.test"}, + {"config", "user.name", "Agent Compose"}, + } { + cmd := exec.Command("git", args...) + cmd.Dir = repository + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, output) + } + } + outside := filepath.Join(t.TempDir(), "host-secret.js") + if err := os.WriteFile(outside, []byte("host secret"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(repository, "scheduler.js")); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"add", "scheduler.js"}, {"commit", "-m", "add scheduler symlink"}} { + cmd := exec.Command("git", args...) + cmd.Dir = repository + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, output) + } + } + + data, err := NewDefaultScriptSourceResolver(nil).Resolve(context.Background(), sources.Source{ + Provider: sources.ProviderGit, + URL: repository, + Ref: "main", + Path: "scheduler.js", + }) + if err == nil || !strings.Contains(err.Error(), "must stay within the repository") { + t.Fatalf("escaping git symlink data=%q err=%v", data, err) + } +} + func TestDefaultScriptSourceResolverHTTPFailures(t *testing.T) { t.Run("status and query redaction", func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {