Pull request overview

Adds a new first-party azure.ai.rle azd extension scaffold, establishing the initial command surface area and packaging/build plumbing so the extension can be built, packed, and tested prior to implementing the real RLE workflow.

Changes:

• Introduces the azd ai rle command group with create, modify, version, and hidden metadata commands (stubbed with structured “not implemented” errors).
• Adds extension packaging metadata (extension.yaml, version.txt, CHANGELOG.md) plus docs and lint/spellcheck configs for the new module.
• Adds cross-platform build scripts and the Go module (go.mod/go.sum) for the extension.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Review: azure.ai.rle extension (new)

Thanks for contributing a new extension! A few items to address before this is ready.

Overview

This adds a new azd ai rle command group for managing RLE (Reinforcement Learning Environment) resources. The implementation is clean and follows the extension SDK patterns well. Tests pass and it builds cleanly.

Key items

1. Missing PR description. A 2949-line new extension needs a description explaining what RLE is, the motivation, the commands it adds, and any design decisions. Right now the body is empty.

2. Unreachable code. Five files define commands (create, list, show, sandbox, versions) that are never registered in root.go. These add ~500 lines of untestable surface area. Either wire them up (if intended for this PR) or remove them and add in a follow-up when they're needed.

3. Cross-platform line endings. scaffold.go:67 unconditionally converts \n to \r\n. Generated Dockerfiles and Python files will have Windows line endings on Linux/macOS, which can cause issues with container builds and shebangs.

4. Unquoted paths in user-facing output. init.go:55 prints cd %s without quoting. Paths containing spaces will break the suggested command. This repo's conventions require quoting paths (see AGENTS.md).

5. Bearer token over HTTP. The client sends RLE_BEARER_TOKEN regardless of the endpoint scheme. Consider logging a warning (or refusing) when sending credentials over non-TLS connections to non-localhost endpoints.

See inline comments for specifics.

Incremental review (commit d621e52)

New deploy enhancements look well-structured. The splitImageHost/normalizeRegistryLoginServer composition is clean, and using errors.AsType for the 404 check is idiomatic Go 1.26. A few items:

Previous review items (5 findings from my earlier pass) are still unresolved: unreachable command constructors, cross-platform line endings in scaffold.go, unquoted paths in init.go, bearer token over HTTP. Please address those alongside the new work.

Incremental review (commits 9dc600d, 6772f58)

The invoke implementation is taking shape. The approach of fetching Loom via shallow clone plus bundled wheel is reasonable for a dev workflow. A few items need attention before this can merge.

Prior review items: 3 of 5 findings from my earlier passes remain unresolved (unreachable command constructors in root.go, cross-platform CRLF in scaffold.go, unquoted path in init.go). The bearer-token item was addressed by removal.

New findings (this commit):

1. Binary wheel without provenance (assets/rle_sdk-0.1.3-py3-none-any.whl): Committing a pre-built .whl makes the package un-auditable. Add a README or comment documenting where this wheel is built, what source repo/tag it corresponds to, and how a reviewer can reproduce it. Alternatively, reference a published package feed so CI can fetch it at build time instead of vendoring the binary.

2. Deploy ignores stored endpoint (deploy.go): resolveControlPlaneEndpoint("") replaced resolveControlPlaneEndpoint(state.Endpoint). If a user had an endpoint saved in .azd-rle.json from the manifest, it's now silently ignored. Was this intentional? If so, note it in the changelog.

3. Fragile TOML patching (invoke.go:ensureLoomLocalSources): String-contains checks + append-to-EOF doesn't guarantee the new lines land inside the [tool.uv.sources] section when one already exists elsewhere in the file. Consider using a TOML library or at minimum insert after the existing header line rather than at EOF.

4. Most training flags are not exposed: Only 4 of ~15 parameters have CLI flags. The rest can't be overridden without editing source. Expose them or document why they're fixed.

5. ADO clone URL may require auth: defaultLoomRecipeRepo points to msdata.visualstudio.com. Will this work without credentials for users outside the org? If auth is needed, surface a clear error message.

The new commit (a95e826) reworks image resolution to derive per-environment image names from the environment name, with --image as an explicit override on both init and deploy. The self-bootstrapping deploy (works without prior init when rle.yaml exists) is a nice UX improvement. The errors.AsType usage and azdext.LocalError error handling are correct and idiomatic.

Three items from my earlier reviews remain unresolved:

1. scaffold.go:74 strings.ReplaceAll(content, "\n", "\r\n") forces Windows line endings on all platforms. Generated Dockerfiles and Python files will carry \r\n into Linux containers, which can break shebang parsing and cause git diff noise on non-Windows hosts. Remove this line; let files keep native \n endings.

2. init.go:57 cd %s should use %q to quote paths that may contain spaces (per the repo's path safety convention).

3. root.go Five command source files (create.go, list.go, show.go, sandbox.go, versions.go) are included in the PR but their constructors are never registered in NewRootCommand. If planned for a follow-up, remove them from this PR to reduce the diff.

New commit (f3ab2d2) restores deploy changes lost in the merge conflict. The ACR build integration and image host-qualification helpers work correctly for the described use case.

One new finding: az acr build is called without verifying that az is installed (see inline comment). The invoke command checks for git and uv with exec.LookPath before calling them; the same pattern should apply here.

Three items from earlier reviews remain unresolved:

1. scaffold.go:74 strings.ReplaceAll(content, "\\n", "\\r\\n") forces CRLF on all platforms. Dockerfiles and Python files with \r\n will break shebang parsing in Linux containers. Remove this line.

2. init.go:57 cd %s needs %q to quote paths with spaces (AGENTS.md convention).

3. root.go Five command source files (create.go, list.go, show.go, sandbox.go, versions.go) define constructors never called from NewRootCommand. Wire them in or remove them from this PR.

Checking in on this PR. My CHANGES_REQUESTED from the previous review cycle is still active. The three core items remain unaddressed, and there's been no author response in the thread.

Still blocking (from prior reviews):

1. scaffold.go:74 - forces Windows line endings on all generated files unconditionally. Dockerfiles and Python scripts with CRLF will break shebang parsing and cause issues in Linux containers. Remove the strings.ReplaceAll(content, "\\n", "\\r\\n") call.

2. init.go:57 - the cd path in user-facing output isn't quoted. Paths with spaces will break the suggested command. Use %q instead of %s (explicit repo convention in AGENTS.md).

3. root.go - five command source files (create.go, list.go, show.go, sandbox.go, versions.go) define constructors that are never registered. The test in root_test.go explicitly asserts they are NOT registered, confirming this is intentional dead code. Remove them from this PR and add them when they are ready to ship.

4. deploy.go - az acr build is invoked without verifying az is on PATH. The invoke command checks for git and uv before calling them; apply the same pattern here.

Additional item (see inline):

5. invoke.go (ensureLoomLocalSources) - the TOML patching approach appends lines to EOF rather than inserting under the existing [tool.uv.sources] header. If other TOML sections follow, the appended keys land in the wrong section.

@farhann1 - could you acknowledge these items or let me know if there's a reason they can't be addressed? Happy to discuss tradeoffs.

Generally I approve the code for a PrPr

In the future, you can use azd x init --internal to automatically scaffold all the required 1P specific extension files

Incremental review of bab9130 (fix: address review comments).

Flag type split into
emoteInvokeFlags, localRunFlags, and openEnvCallFlags is clean. The new project_endpoint.go module validates and normalizes the Foundry endpoint correctly, with good test coverage for both happy path and error cases. State migration from Project to ProjectEndpoint is consistent across deploy and invoke.

No new issues.

Incremental review of f5798ac (addressing prior feedback). The refactoring looks good: local container lifecycle cleanly separated into run.go, utility code extracted to focused modules (docker.go, runtime.go, identifiers.go), and tests are thorough. CODEOWNERS and CI added. One note: actionlint CI is failing on the new workflow file.

Thanks, scaffolding files look good 👍

Re-approving after rebase onto main. No extension code changes since my prior approval. The implementation is solid: proper error handling with typed errors, path traversal protection, input validation, signal handling, and good test coverage.

The refactoring since my last review looks good. The split of invoke into run + invoke, the new project-layer abstractions (docker.go, runtime.go, identifiers.go, build.go), and the remote playground proxy are well-structured.

The separation of concerns is much cleaner now:
un.go owns local container lifecycle, invoke.go owns remote sandbox management, and the project package provides shared Docker/build/runtime infrastructure.

A couple of minor notes (non-blocking):

• loadLocalRunState is called twice in the run path (once in �nsureLocalContainerEndpoint, once in Run()). Not a bug since it just reads a file, but you could hoist the state to avoid the redundant read.
• projectRouteSegment(state) is called inside the retry loop in createSandboxWhenImageReady but won't change between attempts. Could be hoisted above the loop.

Neither warrants blocking. The code is clean, error handling is consistent, and the security validations (path traversal checks, operation allowlist in the proxy) are solid.

Approving codeowners change