[release/0.23] Ensure that exit code is set as part of transitioning to final object state

[karolz-ms]

Backport of #118 to release/0.23

/cc @karolz-ms

Customer Impact

Testing

Risk

Regression?

[Copilot]

Pull request overview

Backport to release/0.23 to ensure exit codes are set atomically when transitioning Executable/Container objects into their final (terminal) states, including “synchronous completion during startup” scenarios (per #118 / Aspire issue).

Changes:

• Add ExitCode propagation to controllers.ExecutableStartResult (including clone/equality/apply/update logic).
• Update IDE runner + container stop path to carry exit code data through the “final state” transition.
• Add integration tests to assert the first observation of a terminal state already includes Status.ExitCode (and FinishTimestamp for containers).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/integration/executable_controller_test.go	Adds integration test covering synchronous IDE finish (Finished + ExitCode in the same first terminal update).
test/integration/container_controller_test.go	Adds integration test asserting stop-induced Exited state includes ExitCode/FinishTimestamp in the first terminal update.
internal/testutil/ctrlutil/test_ide_runner.go	Adds test helper to simulate “synchronous finish during startup” with exit code.
internal/exerunners/ide_executable_runner.go	Plumbs cached IDE exit code into the start result for synchronous completion paths.
controllers/executable_start_result.go	Adds ExitCode field and ensures it is compared/cloned/applied/updated appropriately.
controllers/container_controller.go	Changes stop helper to return inspected container data and uses it to capture final exit code on stop.
controllers/container_common.go	Updates stopContainer to return inspected container state after verification.

Comments suppressed due to low confidence (1)

internal/exerunners/ide_executable_runner.go:250

• rd.exitCode (and rd.state used just below) are read after releasing r.lock. Those fields are written under r.lock in the IDE notification handlers, so this creates a potential data race and can also read inconsistent values. Consider copying rd.state/rd.exitCode into locals while still holding the lock, then unlocking and using the locals to populate result/choose ExeState.

		result.ExitCode = pointers.Duplicate(rd.exitCode)
		if rd.state == runStateFailedToStart {
			result.ExeState = apiv1.ExecutableStateFailedToStart
		} else {
			result.ExeState = apiv1.ExecutableStateFinished
		}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When stopContainer() treats ErrNotFound as success it returns (nil, nil). In that case this code keeps returning the pre-stop inspected value, which can be stale (e.g., still Running) and prevents callers from accurately reflecting the post-stop state/exit code. Consider updating inspected unconditionally from stopInspected (even if nil), or explicitly setting inspected = nil when stopInspected == nil so the return value always represents the most recent known state.

Suggested change

	if stopInspected != nil {
	inspected = stopInspected
	}
	inspected = stopInspected

[Copilot]

The field comment says ExitCode is populated only when the run completed synchronously during startup (ExeState == Finished), but the implementation applies/updates it for any terminal ExeState when known (see applyTo() / UpdateFrom()). Please align the comment with the actual semantics (e.g., “exit code if known; may be populated when startup completes synchronously in a terminal state”).

Suggested change

	// Exit code of the Executable process.
	// Populated when the run completed synchronously during startup (ExeState == Finished).
	// Exit code of the Executable process, if known.
	// May be populated when startup completes synchronously in a terminal state.