fix: detached runs, non-interactive port prompts, test command reporting - #565
Merged
Conversation
…test commands Three reports share one theme: azd app works when a human is watching an interactive terminal, and misbehaves the moment nobody is. #555 azd app run --detach dies immediately on Windows. This turned out to be five independent causes, each able to produce part of the report: * The detached child stays inside the parent's Job Object. DETACHED_PROCESS does not remove it, so a kill-on-close job takes the child down after CreateProcess succeeds but before it writes anything, which is exactly why run.log existed and was empty. The spawn now tries CREATE_BREAKAWAY_FROM_JOB first and falls back to the previous flags on ERROR_ACCESS_DENIED, building a fresh exec.Cmd per attempt since a failed one cannot be restarted. * Run state was written only after every service started and after a 10s dashboard wait, so status and stop had no PID to work with. It is now seeded immediately after cmd.Start(), and a persist failure does not fail the run. * The child re-ran LoadAzdEnvironment, which shells out to azd env get-values against an azd host that is already exiting. First bytes in run.log dropped from ~1590ms to ~328ms once skipped. The marker that identifies the child is now consumed at startup so services, hooks, and nested runs do not inherit it. * Parent and child keyed run state by different directories, so running from a subdirectory produced two state files and stop looked in a third place. cwd and azureYamlDir are collapsed into a single projectDir. * stop removed run state from an unconditional defer, discarding the only PID record when it failed. It now branches on manager liveness. #556 The port-conflict prompt turned io.EOF into a fatal error even though a non-interactive fallback already existed. readPromptLine now separates EOF with partial data (honour it, since ReadString returns both when the final answer has no trailing newline) from EOF with nothing (fall through to the documented auto-kill path), and still discards partial data on a genuine read error rather than letting a truncated read select a destructive menu entry. #557 Execution was already correct on main; the reporting was not. Validation had no idea which type was requested, so a service with both framework: vitest and an explicit test.e2e.command was reported as "vitest detected". Validation now takes the requested type, records the effective command, and reports a type it would silently skip as skipped rather than passed. Also restores the status command's package-level flag vars in its tests. Those leaked across runs and broke go test -count=2 in the very package these changes add tests to. Fixes #555 Fixes #556 Fixes #557 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Website Preview
Preview has been cleaned up as the PR was closed. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
==========================================
+ Coverage 61.09% 61.33% +0.24%
==========================================
Files 224 224
Lines 28989 29074 +85
==========================================
+ Hits 17711 17833 +122
+ Misses 10082 10040 -42
- Partials 1196 1201 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
🚀 Test This PRA preview build ( 🌐 Website PreviewLive Preview: https://jongio.github.io/azd-app/pr/565/ One-Line Install (Recommended)PowerShell (Windows): iex "& { $(irm https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/install-pr.ps1) } -PrNumber 565 -Version 0.20.0-pr565"Bash (macOS/Linux): curl -fsSL https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/install-pr.sh | bash -s 565 0.20.0-pr565UninstallWhen you're done testing: PowerShell (Windows): iex "& { $(irm https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/uninstall-pr.ps1) } -PrNumber 565"Bash (macOS/Linux): curl -fsSL https://raw.githubusercontent.com/jongio/azd-app/main/cli/scripts/uninstall-pr.sh | bash -s 565Build Info:
What to Test: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Three reports share one theme:
azd appworks when a human is watching an interactive terminal, and misbehaves the moment nobody is.#555
azd app run --detachdies immediately on Windows. This turned out to be five independent causes:DETACHED_PROCESSdoes not remove it, so a kill-on-close job takes the child down afterCreateProcesssucceeds but before it writes anything, which is exactly whyrun.logexisted and was empty. The spawn now requestsCREATE_BREAKAWAY_FROM_JOBfirst and falls back to the old flags onERROR_ACCESS_DENIED, with a freshexec.Cmdper attempt.statusandstophad no PID. It is now seeded right aftercmd.Start().LoadAzdEnvironment, shelling out toazd env get-valuesagainst an azd host that was already exiting. First bytes inrun.logwent from ~1590ms to ~328ms once skipped.stoplooked in a third place.stopremoved run state from an unconditionaldefer, discarding the only PID record when it failed.#556 The port-conflict prompt turned
io.EOFinto a fatal error even though a non-interactive fallback already existed.readPromptLinenow separates EOF with partial data (honour it) from EOF with nothing (fall through to the documented auto-kill path), and still discards partial data on a genuine read error.#557 Execution was already correct; the reporting was not. Validation had no idea which type was requested, so a service with both
framework: vitestand an explicittest.e2e.commandwas reported asvitest detected. Validation now takes the requested type, reports the effective command, and reports a type it would silently skip as skipped rather than passed.Full root-cause analysis, evidence tables, trade-offs and acceptance criteria:
docs/specs/detach-port-testcmd/spec.mdCloses #555
Closes #556
Closes #557
Checklist
Testing
39 packages, 0 failures.
mage lint0 issues.gofmtandgo vetclean. Cross-compiles for linux/amd64, darwin/arm64 and windows/amd64. Roughly 840 lines of tests to 460 of source.Every test that guards a fix was mutation-tested: the fix was temporarily reverted and the test confirmed to fail, then restored, so none of them are tautological.
#556 is covered end to end by pointing stdin at the OS null device, which is a character device that yields EOF instantly and so reproduces the exact reported condition. A pipe cannot, because it fails the interactive guard before reaching the crash site.
Quality Gates
mq(full pipeline)99b1fc7cr(code-review)pf(preflight)secops(security)th(test-health)Notes for reviewers
runstate.Writewas tried and deliberately reverted. Windows refuses to rename over a file another handle has open, so it converts a rare, self-correcting short read into a hard write failure that loses the PID. A bounded retry still starved under a sustained reader. Evidence is in the spec trade-offs.azd app startand tie MCP-started services to the MCP host lifetime. A follow-up issue will capture it.status_test.gorestores the status command's package-level flag vars. They leaked across runs and brokego test -count=2in the very package this PR adds tests to. A second pre-existing-count=2defect ininternal/serviceis documented in the test plan and left alone to keep this diff scoped.run.goandstatus_test.go, which several openidea/*PRs also touch (feat(status): add exit-code flag #563, feat(run): add no-deps repeat start option #544, feat(run): add --except to run every service but the named ones #513, feat(status): show uptime in azd app status #502, feat(run): add --env for inline environment overrides #498). No semantic overlap, mechanical conflicts only.