Skip to content

Commit 3f1509c

Browse files
Antriksh JainCopilot
andcommitted
fix(azure.ai.agents): scope deploy-hook Next: block to deployed service + canonical README
Two consensus findings from the 3-reviewer pass on commits 3.1+3.2: (1) multi-service state leak — GPT-5.5 (originator), confirmed by Sonnet 4.6 and Opus 4.7. In a multi-agent project, deploying one service caused ResolveAfterDeploy to emit show/invoke rows for EVERY service in the project, all attached to the deployed service's artifact note. Fix: filter state.Services to serviceConfig.Name at the deploy-hook call site via a new filterServicesByName helper. ResolveAfterDeploy itself is left untouched — show/doctor callers still get the project-wide view. (2) README casing mismatch — Sonnet 4.6 (originator), confirmed by GPT-5.5 and Opus 4.7. readmeExists accepted README.md / readme.md / README.MD, but ResolveAfterDeploy always emits the literal "see <path>/README.md" pointer. On case-sensitive filesystems (Linux, WSL) with only a lowercase readme.md on disk, the collision rule would fire on the canonical-casing suggestion, REPLACE the aka.ms fallback, and leave the user with a pointer that does not resolve. Fix: tighten readmeExists to only check the canonical "README.md" — same casing the resolver emits. Regression tests: - TestAugmentDeployNote_LowercaseReadme_DoesNotReplaceFallback (skipped on case-insensitive filesystems via a runtime probe; runs on Linux CI) - TestAugmentDeployNote_MultiServiceState_ScopedToDeployedService - TestFilterServicesByName Pre-flight green: gofmt, vet, build, full extension test suite, golangci-lint 0 issues. Live smoke against the hello-world-python-invocations sample shows unchanged output (single-service project with canonical README — neither fix's scenario applies; the existing happy path is preserved). Refs PR Azure#8057 critique items C2 and 3-reviewer consensus on phase-3 wiring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a6784c2 commit 3f1509c

2 files changed

Lines changed: 132 additions & 6 deletions

File tree

cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,12 @@ func (p *AgentServiceTargetProvider) finalizeDeploy(
976976
// aka.ms link emitted by deployArtifacts is preserved when the
977977
// enrichment is skipped or short-circuits.
978978
if state, _ := nextstep.AssembleState(ctx, p.azdClient); state != nil {
979+
// Scope to the service just deployed. ResolveAfterDeploy renders a
980+
// show/invoke pair per state.Services entry; without this filter a
981+
// multi-agent project would attach guidance for other services to
982+
// this artifact's note.
983+
state.Services = filterServicesByName(state.Services, serviceConfig.Name)
984+
979985
projectRoot := ""
980986
if proj, err := p.azdClient.Project().Get(ctx, nil); err == nil && proj.Project != nil {
981987
projectRoot = proj.Project.Path
@@ -1610,12 +1616,13 @@ func augmentDeployNote(state *nextstep.State, artifacts []*azdext.Artifact, proj
16101616
if projectRoot == "" || relativePath == "" {
16111617
return false
16121618
}
1613-
for _, name := range []string{"README.md", "readme.md", "README.MD"} {
1614-
if _, err := os.Stat(filepath.Join(projectRoot, relativePath, name)); err == nil {
1615-
return true
1616-
}
1617-
}
1618-
return false
1619+
// Only consider the canonical casing — ResolveAfterDeploy emits
1620+
// "see <relPath>/README.md" verbatim. Accepting other casings here
1621+
// would yield a broken pointer on case-sensitive filesystems and,
1622+
// because suggestionsIncludeReadme triggers the replace branch,
1623+
// would silently discard the working aka.ms fallback.
1624+
_, err := os.Stat(filepath.Join(projectRoot, relativePath, "README.md"))
1625+
return err == nil
16191626
}
16201627

16211628
suggestions := nextstep.ResolveAfterDeploy(state, cachedPayload, readmeExists)
@@ -1671,6 +1678,22 @@ func suggestionsIncludeReadme(suggestions []nextstep.Suggestion) bool {
16711678
return false
16721679
}
16731680

1681+
// filterServicesByName narrows the assembled state's service slice to a
1682+
// single entry by name. Used by the deploy hook so the rendered "Next:"
1683+
// block reflects only the service whose artifact note is being augmented,
1684+
// not every agent service in the project.
1685+
func filterServicesByName(services []nextstep.ServiceState, name string) []nextstep.ServiceState {
1686+
if name == "" {
1687+
return services
1688+
}
1689+
for i := range services {
1690+
if services[i].Name == name {
1691+
return services[i : i+1]
1692+
}
1693+
}
1694+
return nil
1695+
}
1696+
16741697
// protocolEndpointInfo holds a displayable protocol label and its invocation URL.
16751698
type protocolEndpointInfo struct {
16761699
Protocol string

cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,3 +1277,106 @@ func TestAugmentDeployNote_NoServicesIsNoOp(t *testing.T) {
12771277
augmentDeployNote(&nextstep.State{}, []*azdext.Artifact{artifact}, "/tmp", "")
12781278
require.Equal(t, "static aka.ms link", artifact.Metadata["note"])
12791279
}
1280+
1281+
// TestAugmentDeployNote_LowercaseReadme_DoesNotReplaceFallback locks the
1282+
// casing-mismatch guard: when only a lowercase readme.md exists on a
1283+
// case-sensitive filesystem, the resolver would still emit a literal
1284+
// "README.md" pointer that does not resolve on disk and the aka.ms
1285+
// fallback would be lost. The fix tightens readmeExists to the canonical
1286+
// casing so the append branch fires and the static link is preserved.
1287+
func TestAugmentDeployNote_LowercaseReadme_DoesNotReplaceFallback(t *testing.T) {
1288+
t.Parallel()
1289+
1290+
tmp := t.TempDir()
1291+
// Detect case-sensitivity at runtime; the fix is meaningful only on
1292+
// case-sensitive filesystems (Linux, WSL). On Windows NTFS and default
1293+
// macOS APFS the OS resolves "README.md" → "readme.md" transparently,
1294+
// which would make readmeExists return true even after the fix.
1295+
probe := filepath.Join(tmp, "case-probe.txt")
1296+
require.NoError(t, os.WriteFile(probe, nil, 0o600))
1297+
if _, err := os.Stat(filepath.Join(tmp, "CASE-PROBE.TXT")); err == nil {
1298+
t.Skip("case-insensitive filesystem — readmeExists casing guard is a no-op here")
1299+
}
1300+
1301+
servicePath := filepath.Join(tmp, "src", "echo")
1302+
require.NoError(t, os.MkdirAll(servicePath, 0o750))
1303+
// Only lowercase readme.md exists; canonical README.md does not.
1304+
require.NoError(t, os.WriteFile(filepath.Join(servicePath, "readme.md"), []byte("sample"), 0o600))
1305+
1306+
state := &nextstep.State{
1307+
Services: []nextstep.ServiceState{
1308+
{
1309+
Name: "echo",
1310+
RelativePath: "src/echo",
1311+
Protocol: "invocations",
1312+
IsDeployed: true,
1313+
},
1314+
},
1315+
}
1316+
1317+
artifact := &azdext.Artifact{
1318+
Kind: azdext.ArtifactKind_ARTIFACT_KIND_ENDPOINT,
1319+
Metadata: map[string]string{
1320+
"label": "Agent endpoint (invocations)",
1321+
"note": "static aka.ms link",
1322+
},
1323+
}
1324+
1325+
augmentDeployNote(state, []*azdext.Artifact{artifact}, tmp, "")
1326+
1327+
got := artifact.Metadata["note"]
1328+
require.Contains(t, got, "static aka.ms link",
1329+
"aka.ms fallback must survive when only lowercase readme.md exists on disk")
1330+
require.NotContains(t, got, "see src/echo/README.md",
1331+
"resolver must not emit a README pointer that does not match what is on disk")
1332+
}
1333+
1334+
// TestAugmentDeployNote_MultiServiceState_ScopedToDeployedService locks
1335+
// the deploy-hook contract that the rendered Next: block reflects only
1336+
// the service whose artifact note is being augmented. The hook applies
1337+
// filterServicesByName to the assembled state before invoking the
1338+
// resolver.
1339+
func TestAugmentDeployNote_MultiServiceState_ScopedToDeployedService(t *testing.T) {
1340+
t.Parallel()
1341+
1342+
state := &nextstep.State{
1343+
Services: []nextstep.ServiceState{
1344+
{Name: "alpha", RelativePath: "src/alpha", Protocol: "invocations", IsDeployed: true},
1345+
{Name: "beta", RelativePath: "src/beta", Protocol: "invocations", IsDeployed: true},
1346+
},
1347+
}
1348+
state.Services = filterServicesByName(state.Services, "alpha")
1349+
1350+
artifact := &azdext.Artifact{
1351+
Kind: azdext.ArtifactKind_ARTIFACT_KIND_ENDPOINT,
1352+
Metadata: map[string]string{
1353+
"label": "Agent endpoint (invocations)",
1354+
"note": "static aka.ms link",
1355+
},
1356+
}
1357+
1358+
augmentDeployNote(state, []*azdext.Artifact{artifact}, "/tmp", "")
1359+
1360+
got := artifact.Metadata["note"]
1361+
require.NotContains(t, got, "beta",
1362+
"other-service guidance must not leak into the deployed service's note")
1363+
require.Contains(t, got, "Next:", "Next: block should be present for the deployed service")
1364+
}
1365+
1366+
// TestFilterServicesByName covers the helper used at the deploy-hook call site.
1367+
func TestFilterServicesByName(t *testing.T) {
1368+
t.Parallel()
1369+
1370+
services := []nextstep.ServiceState{
1371+
{Name: "alpha"},
1372+
{Name: "beta"},
1373+
{Name: "gamma"},
1374+
}
1375+
1376+
require.Equal(t, []nextstep.ServiceState{{Name: "beta"}}, filterServicesByName(services, "beta"),
1377+
"match returns single-element slice")
1378+
require.Nil(t, filterServicesByName(services, "missing"),
1379+
"no match returns nil caller short-circuits on empty Services")
1380+
require.Equal(t, services, filterServicesByName(services, ""),
1381+
"empty name returns input unchanged (defensive)")
1382+
}

0 commit comments

Comments
 (0)