Create folder during init#8210
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the azd ai agent init flow (azure.ai.agents extension) to mimic git clone behavior by creating a new subdirectory when initializing from a sample/template, reducing the risk of writing project files into an unexpected directory (e.g., a user’s home folder).
Changes:
- Add a
targetDirparameter to the shared manifest init path so project scaffolding can occur in a chosen directory. - When initializing from a template, derive a folder name from the template title, run
azd initinto that folder, andchdirthe extension process to keep subsequent file operations aligned. - Print a follow-up message indicating where the project was created.
Comments suppressed due to low confidence (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/init.go:716
- This env name construction appends "-dev" after sanitization, which can produce names longer than azd's 64-char environment name limit (cli/azd/pkg/environment/environment.go:150-152) when envBase is long. To avoid
azd initfailing, sanitize/truncate the complete "-dev" value (or leverage azd's environment name cleaning) so the final length is guaranteed valid.
sanitizedDirectoryName := sanitizeAgentName(envBase)
initArgs = append(
initArgs, "--environment", sanitizedDirectoryName+"-dev",
)
jongio
left a comment
There was a problem hiding this comment.
Issues to address:
- init.go:494 - folder name derivation is duplicated between the TemplateTypeAzd and default cases; extract a helper
…d adjusting folder creation hints
…e clarity and consistency
wbreza
left a comment
There was a problem hiding this comment.
Thanks for tackling this — moving azd ai agent init to a git clone-style folder layout is the right UX. A few observations beyond the prior reviewer comments:
High
1. folderNameFromTitle is only used in one of the two template branches
The new helper addresses jongio''s earlier "extract a helper" feedback, but the TemplateTypeAzd branch still calls sanitizeAgentName(selectedTemplate.Title) directly while the default (manifest) branch calls folderNameFromTitle(selectedTemplate.Title). For a template titled "Basic Agent (Python)" the two branches now produce different folders:
- Azd branch →
basic-agent-python/ - Manifest branch →
basic-agent/
Suggest routing both branches through folderNameFromTitle so the naming is consistent across template types.
2. Env-name derivation is inconsistent between branches
- Azd branch:
sanitizeAgentName(selectedTemplate.Title + "-dev")— derived from the raw title - Manifest branch (via
ensureProject):sanitizeAgentName(targetDir + "-dev")— derived from the sanitized folder name
Same template, different env name depending on type. Easiest fix: have both paths derive the env name from the resolved folder name (the manifest branch''s pattern), so folder + env stay paired.
3. Non-ASCII titles are silently destroyed
sanitizeAgentName''s regex [^a-z0-9-]+ strips all non-ASCII to hyphens, then trims/truncates and falls back to "my-agent" if empty. A template titled Échantillon becomes chantillon, and a title that is mostly non-ASCII (e.g. 日本語エージェント) collapses to my-agent with no warning. Worth at least surfacing a notice — perhaps including the original template title in the "Your project has been created in …" line — so users can see how their selection was mapped.
Medium
4. ./ prefix is POSIX-style on Windows
fmt.Printf("\nYour project has been created in ./%s\n", relPath)On Windows this prints ./src\my-agent — neither idiomatic Unix nor Windows. Consider filepath.ToSlash(relPath) for display, and either drop the literal ./ or quote the path.
5. Message doesn''t tell the user the next step
The follow-up tells the user where the project was created but not how to navigate to it. Many CLIs print a literal cd <folder> line for copy-paste — would be helpful here.
6. Long titles can truncate -dev off the env name
sanitizeAgentName(title + "-dev") sanitizes the combined string and then truncates to 63 chars. For long titles, -dev is silently dropped. Truncating the base to 59 chars before appending -dev keeps the suffix intact:
base := sanitizeAgentName(title)
if len(base) > 59 { base = strings.TrimRight(base[:59], "-") }
envName := base + "-dev"7. Inverted dirExisted semantics
_, dirExisted := os.Stat(folderName)
if dirExisted != nil { /* directory was NEW */ }The variable is an error, not a boolean — dirExisted != nil actually means "stat failed (directory probably did not exist)." This reads backwards and the same pattern is mirrored in the tests. Two improvements:
- Rename to
statErr(or compute anewlyCreatedbool) for clarity. - Use
errors.Is(err, fs.ErrNotExist)instead oferr != nil— otherwise a permission error onos.Statwould be treated as "newly created" and trigger the "created" message incorrectly.
8. Tests assert against re-implemented logic, not production code
The five new TestCreatedFolderPath_* tests locally re-execute the same filepath.Join(originalCwd, folderName) + filepath.Rel(...) computation and then assert the result equals itself. They don''t invoke newInitCommand, ensureProject, runInitFromManifest, sanitizeAgentName, or folderNameFromTitle. The tests will pass even if the production flow diverges.
Suggest extracting the path-derivation into a small pure helper, e.g.:
func formatCreatedFolderMessage(originalCwd, createdFolder string) string { ... }…and having both the production code and the tests call it. That makes the tests meaningful regression markers.
9. sanitizeAgentName and folderNameFromTitle have no direct unit tests
These two helpers are central to folder + env-name correctness but are only exercised transitively. Worth a small table-driven test for each, covering at minimum:
- Truncation at 63 chars (and the
-devsuffix loss in #6) - Empty-after-sanitize fallback to
"my-agent" - Non-ASCII input (related to #3)
- Trailing-
(...)stripping infolderNameFromTitle
Low
10. folderNameFromTitle is narrowly scoped under a generic name
It only strips a trailing (...) suffix. Either rename it more specifically (e.g. folderNameStrippingParenSuffix) or add a short doc comment so callers don''t expect broader normalization.
11. Help text and discoverability
The cobra command''s Short/Long text doesn''t mention the new "creates a new folder" behavior. Updating it would make the change discoverable from --help.
12. No opt-out flag for the prior behavior
Power users who deliberately want to scaffold into the current directory have no shortcut anymore. Not blocking — possible follow-up.
Overall the change is the right direction. The biggest things to address before merge are the two consistency issues (#1, #2) and the silent non-ASCII handling (#3); the rest can fold in here or in a follow-up.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Thanks @wbreza for all of the helpful feedback, I addressed all but 11 and 12. For 11, there are a lot of init behaviors that will not create a folder, so I am not sure what to put in the help text that isn't super long. For 12, we can put that as another issue for later if needed. |
…ject initialization
…one of the two template branches
…ng project initialization
…nt formatting across platforms
… navigation instructions
…roject initialization
…ity and update references in init and test files
Related to #8209, example implementation for changing
azd ai agent initto create a new dir when init'ing from a sample.