Remove hardcoded invoke suggestion from azd ai agent run - #7898
Conversation
azd ai agent run
Jon Gallant (jongio)
left a comment
There was a problem hiding this comment.
Changes look correct - removes the hardcoded "Hello!" suggestion that doesn't match most agent protocols.
Both locations (the Long description and the runtime fmt.Printf output) are cleaned up. The remaining \n prefix on the Printf properly preserves blank-line spacing. No test impact.
Minor note: invoke.go:63 still shows azd ai agent invoke --local "Hello!" in the invoke command's own examples. That's a different context (documenting invoke's syntax vs suggesting a specific payload from run), but worth considering whether the invoke examples should also use a more protocol-appropriate placeholder.
|
Copilot Rebase this PR on the latest main and make sure the fix is still relevant. |
Rebased onto latest main (commit |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/00b3c9aa-55ab-45f4-a96f-e75670bd89d1 Co-authored-by: therealjohn <1501196+therealjohn@users.noreply.github.com>
a087e97 to
4e9fa6d
Compare
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
Wallace Breza (wbreza)
left a comment
There was a problem hiding this comment.
Review Summary
The intended change (remove the hardcoded azd ai agent invoke --local "Hello!" suggestion from run.go) is correct, minimal, and addresses a real UX bug — the "Hello!" payload is not valid for most agents using the /invocations protocol. jongio''s spot-check of the intent is accurate.
However, there are two things to address before this is mergeable.
🔴 Branch is severely out of date with main
This PR currently shows a 1,098-file / +107,689 / −12,036 diff on GitHub. The branch has only two author commits (the Initial plan placeholder and bc8ab77 — the actual fix from 2026-04-23), but main has advanced significantly since then. As a result:
- The automated Copilot reviewer refused the review entirely (
exceeds the maximum number of files (300)) - Human reviewers can''t meaningfully diff the change against the surrounding context
run.gohas been substantially refactored onmainsince this branch forked (newnoPromptparameter threaded throughrunRun,extCtxplumbing onnewRunCommand,resolveConnectionCredentials,localAgentKey/session cleanup defer,setupDebugLoggingremoval, etc.), so the original patch will conflict and must be re-applied against the current file
Please rebase onto current main and force-push (--force-with-lease) so the PR diff reflects only the intended ~10-line change. The bug is still present on main today (lines 49 and 177–178 of run.go), so the fix is still needed and still valid — it just needs to be re-cut against the refactored file.
🟡 Confirm scope: third "Hello!" occurrence in invoke.go
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go:102 also contains the literal azd ai agent invoke --local "Hello!" string. It appears to live inside the invoke command''s own help/example text — where demonstrating the command syntax is legitimate and arguably correct — so this looks intentionally untouched. Could you confirm that''s the case so reviewers know it was a deliberate scoping choice rather than an oversight?
Once rebased
This is a fast-path approve. The change is surgical, no behavior risk, and matches the description exactly. No new tests needed (user-facing string removal only).
|
/check-enforcer override |
Wallace Breza (wbreza)
left a comment
There was a problem hiding this comment.
✅ Approving — rebase looks good
The rebase onto current main cleanly addressed the prior feedback. The diff is now properly scoped to 1 file, +2 / −8 in cli/azd/extensions/azure.ai.agents/internal/cmd/run.go:
- Removes the
Use a separate terminal to invoke the running agent: azd ai agent invoke --local "Hello!"paragraph from the cobraLongdescription. - Removes the three-line runtime
fmt.Println/fmt.Printfblock and correctly folds the leading newline into the survivingfmt.Printf("\nStarting agent on %s (Ctrl+C to stop)\n\n", url).
The change is surgical, no behavior risk, user-facing string only, and the invoke.go:102 occurrence was (correctly) left alone since it lives inside the invoke command''s own help example where the syntax is legitimately demonstrated.
Ready to merge. 🚀
azd ai agent runwas printing a staticazd ai agent invoke --local "Hello!"suggestion that is incorrect for agents using the/invocationsprotocol — the payload shape is agent-specific and"Hello!"is not a valid invocation body in most cases, leading to confusing runtime errors.Changes
runRun(): Remove the three-line "After startup, in another terminal, try: …" block; retain only theStarting agent on http://localhost:<port> (Ctrl+C to stop)line.newRunCommand()Longdescription: Remove the trailing "Use a separate terminal to invoke the running agent: …" paragraph for the same reason.