fix(cast): pass repo root to LocalAgentSource instead of .squad/ dir#892
fix(cast): pass repo root to LocalAgentSource instead of .squad/ dir#892tamirdresher merged 2 commits intodevfrom
Conversation
🟡 Impact Analysis — PR #892Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (1 file)
squad-cli (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
There was a problem hiding this comment.
Pull request overview
Fixes local cast agent discovery by passing the repo root (cwd) to LocalAgentSource, avoiding the erroneous double-nested .squad/.squad/agents lookup.
Changes:
- Update
runCast()to instantiateLocalAgentSourcewithcwdinstead ofpaths.teamDir. - Add Vitest regression coverage ensuring agents are discovered from
<repo>/.squad/agentsand not from.squad/.squad/agents. - Add a changeset documenting the CLI patch release, plus an agent history note capturing the learning.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/squad-cli/src/cli/commands/cast.ts |
Fixes the base path passed to LocalAgentSource so local project agents are discoverable. |
test/cli/cast.test.ts |
Adds regression tests to prove correct agent discovery and prevent the double-nested-path regression. |
.squad/agents/eecom/history.md |
Documents the root cause/fix pattern for future reference. |
.changeset/fix-cast-base-path.md |
Declares a patch release for @bradygaster/squad-cli with a summary of the fix. |
64b4590 to
9b5841e
Compare
🔍 Squad Review — Kaylee (Engineering)
Verdict: ✅ Ready to merge Review by Squad AI team (Kaylee — Engineering) · requested by Dina Berry |
diberry
left a comment
There was a problem hiding this comment.
required: Branch is behind dev — needs rebase before merge to ensure CI runs clean against latest.
suggestion: No test for .ai-team/agents fallback path. Low risk given the bug history but worth a third test case.
nit: Test charter uses YAML frontmatter but parseCharterMetadata only parses ## Identity markdown sections — template is misleading for future test authors.
nit: Changeset description reads ambiguously — consider: 'Passes repo root to LocalAgentSource instead of .squad/ dir, preventing a double-nested .squad/.squad/agents/ lookup.'
9b5841e to
262de1b
Compare
|
CI validation passed on fork: diberry#139 (7/7 checks green) |
| const paths = resolveSquadPaths(cwd); | ||
| if (!paths) { | ||
| fatal('No squad found. Run "squad init" first.'); | ||
| } | ||
|
|
||
| // Discover project agents | ||
| const projectSource = new LocalAgentSource(paths.teamDir); | ||
| const projectSource = new LocalAgentSource(cwd); | ||
| const projectAgents = await projectSource.listAgents(); |
There was a problem hiding this comment.
resolveSquadPaths(cwd) returns paths.teamDir as the team identity root in remote mode (team repo root) and as the .squad/ directory in local mode. Passing raw cwd into LocalAgentSource makes agent discovery depend on the caller already being at the repo root, and it also breaks remote mode by scanning <project>/.squad/agents instead of <teamDir>/.squad/agents. Use paths to derive the correct base path (e.g., local: parent of paths.teamDir / paths.projectDir; remote: paths.teamDir).
| it('discovers project agents using repo root, not .squad/ dir (#871)', async () => { | ||
| await scaffold(TEST_ROOT); | ||
|
|
||
| // Suppress console output during test | ||
| const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); | ||
|
|
||
| const { runCast } = await import('@bradygaster/squad-cli/commands/cast'); | ||
| await runCast(TEST_ROOT); | ||
|
|
||
| // If the bug were present (passing paths.teamDir = .squad/ to LocalAgentSource), | ||
| // it would look in .squad/.squad/agents/ — which doesn't exist — and find 0 agents. | ||
| // With the fix, it looks in TEST_ROOT/.squad/agents/ and finds our test agent. | ||
| const output = logSpy.mock.calls.map(c => c.join(' ')).join('\n'); | ||
| // Agent discovered from .squad/agents/test-agent/ (name derived from directory) | ||
| expect(output).toContain('test-agent'); | ||
| expect(output).toContain('Session Cast'); | ||
| }); | ||
|
|
||
| it('does not look in double-nested .squad/.squad/agents/ path', async () => { | ||
| await scaffold(TEST_ROOT); | ||
|
|
||
| // Create a decoy agent at the WRONG double-nested path | ||
| const wrongPath = join(TEST_ROOT, '.squad', '.squad', 'agents', 'decoy'); | ||
| await mkdir(wrongPath, { recursive: true }); | ||
| await writeFile(join(wrongPath, 'charter.md'), `---\nname: Decoy\nrole: Wrong\n---\n# Decoy\n`); | ||
|
|
||
| const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); | ||
|
|
||
| const { runCast } = await import('@bradygaster/squad-cli/commands/cast'); | ||
| await runCast(TEST_ROOT); | ||
|
|
||
| const output = logSpy.mock.calls.map(c => c.join(' ')).join('\n'); | ||
| // Should find test-agent from correct path, not decoy from wrong path | ||
| expect(output).toContain('test-agent'); | ||
| expect(output).not.toContain('decoy'); | ||
| }); |
There was a problem hiding this comment.
The new tests cover local-mode discovery when runCast() is invoked with the repo root, but they don’t cover two important path-resolution scenarios that this change affects: (1) invoking runCast() from a nested subdirectory (which resolveSquadPaths() supports via walk-up), and (2) remote mode where agents should be discovered from paths.teamDir (team repo root). Adding regression coverage for these cases would prevent future path regressions.
|
@copilot apply changes based on the comments in this thread |
…t(projectDir), remote: teamDir) - Local mode: pass path.resolve(paths.projectDir, '..') (repo root) instead of raw cwd - Remote mode: pass paths.teamDir (team repo root) for correct agent discovery - Add tests for nested subdirectory invocation and remote mode - Fix SAMPLE_CHARTER to use ## Identity sections matching parseCharterMetadata() - Update changeset description for clarity Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/d90f6c33-1da0-4f17-90f3-9bae6f3650da Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com>
Applied in commit 1284bf6. Changes based on the reviewer thread:
Also fixed |
…892) * fix(cast): pass repo root to LocalAgentSource instead of .squad/ dir * fix(cast): derive LocalAgentSource base path from paths (local: parent(projectDir), remote: teamDir) - Local mode: pass path.resolve(paths.projectDir, '..') (repo root) instead of raw cwd - Remote mode: pass paths.teamDir (team repo root) for correct agent discovery - Add tests for nested subdirectory invocation and remote mode - Fix SAMPLE_CHARTER to use ## Identity sections matching parseCharterMetadata() - Update changeset description for clarity Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/d90f6c33-1da0-4f17-90f3-9bae6f3650da Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com> --------- Co-authored-by: Dina Berry <diberry@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bradygaster <41929050+bradygaster@users.noreply.github.com>
What
\cast\ command passed \paths.teamDir\ (which is .squad/) to \LocalAgentSource, but \LocalAgentSource\ internally appends .squad/agents\ — resulting in .squad/.squad/agents\ (double-nested, wrong path). Agents were never discovered in local mode.
Fix
Changed
ew LocalAgentSource(paths.teamDir)\ to
ew LocalAgentSource(cwd)\ in \packages/squad-cli/src/cli/commands/cast.ts.
Tests
Added \ est/cli/cast.test.ts\ with two regression tests:
Verification
Closes #871