fix(sdk): align host detection and target path semantics - #29
Conversation
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a18f0f7030
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cd "$tmp/modules/go-cobra" | ||
| GOWORK=off go test ./... |
There was a problem hiding this comment.
When preparing the next release, scripts/prepare-release.mjs first changes go-cobra/go.mod to require the not-yet-published core version and then runs make check; this standalone go test therefore tries to resolve that unavailable version and fails before the replacement-based consumer check below can run. Add the temporary local-core replacement for this invocation, or defer the standalone packaged-module test until after publication, so release preparation can complete.
AGENTS.md reference: AGENTS.md:L116-L124
Useful? React with 👍 / 👎.
| [ | ||
| "source", | ||
| "go-test-fixtures", | ||
| "node", | ||
| ["scripts/sync-go-testdata.mjs", "--check"], | ||
| rootPath, | ||
| ], |
There was a problem hiding this comment.
Adding this check makes scripts/sync-go-testdata.mjs and the copied go/spec and go/testdata artifacts part of shared golden-case parity enforcement, but docs/architecture.mmd still lists only check.mjs · sync-hosts.mjs and has no artifact or edge representing this new ownership boundary. Update the canonical diagram alongside this parity-pipeline change.
AGENTS.md reference: AGENTS.md:L140-L148
Useful? React with 👍 / 👎.
| for (const detectPath of host.detect) { | ||
| if ( | ||
| !isGenericDetectPath(detectPath) && | ||
| (await exists(expandHostPath(detectPath, home, cwd))) |
There was a problem hiding this comment.
With a valid custom host spec such as detect: ["~/.agents", ".agents"], the first path is skipped but the newly scanned second path auto-detects that host in any project containing the shared .agents directory. This can make agents: "auto"—including --yes workflows—select and write for a host that is not installed; extend the generic-path handling to project-relative shared roots (and apply the same fix in all four SDKs).
AGENTS.md reference: AGENTS.md:L83-L85
Useful? React with 👍 / 👎.
1 participant
Summary
Problem
Host detection and install target resolution did not follow the same contract. An agent could be missed when only a later detection path existed, while installation always selected the first configured path even when another compatible directory was already in use.
The updated contract separates agent-specific detection evidence from compatible target directories and makes target selection deterministic across all four SDKs.
Validation
Stack
This is PR 2 of 3 and is stacked on #28, followed by #30. While all three target
mainbecause the branches live in a fork, the review-specific commit isa18f0f7.