Gitspace start: concurrent starts create duplicate instances and lifecycle command failures are swallowed
Version
main at commit 475f24b39078a8feb8d130e79be021df8ab25f98
Summary
Two independent robustness defects in the Gitspace start and lifecycle path on current main. Both are pre-existing rather than introduced by any single recent change, and each stands on its own.
1. Concurrent start requests can create duplicate Gitspace instances
StartGitspaceAction reads the latest instance, checks whether it is busy, and if none exists or the last one is in a final state it creates a new instance:
The busy guard only rejects a start when an instance is already in a busy state:
The instance is moved into a busy state (Starting) only later, in submitAsyncOps, after the Create above has already run. Nothing (a lock, a transaction, or a unique constraint) spans the read, the busy check, and the create together. So two start requests for the same Gitspace that arrive close together, for example a double click, a client retry, or two open tabs, can both read a nil or final instance, both pass the busy check, and both create an instance.
Failure scenario: a user double clicks Start on a fresh Gitspace. Both requests create an instance and both trigger start orchestration, so setup commands run more than once, extra infrastructure is provisioned, and one instance is left orphaned in Starting until it times out to Error. The controller comment on this path already states these actions should be idempotent, so the current behavior contradicts the intended contract.
Suggested fix: make the read, the busy check, and the create atomic, for example a row lock or a transaction over the config, or a unique partial constraint on active instances per config, so a second concurrent start is rejected or attached to the start already running.
2. Lifecycle command failures are swallowed, so setup reports success after a command fails
ExecuteLifecycleCommands runs each command in a goroutine; on error the goroutine logs and returns, then the function returns nil unconditionally after the wait group:
The per command error captured inside the goroutine is never propagated to the caller. The step framework that calls this carries a StopOnFailure flag, and feature provided PostCreate and PostStart hooks are wired with StopOnFailure: true, so a failing feature command is meant to abort setup. Because the error is dropped, the step reports success, ExecuteSteps continues, and setup is marked complete.
Failure scenario: a devcontainer feature's postCreateCommand exits with a nonzero status. The Gitspace is reported as set up successfully with an incomplete environment. A later retry has no truthful failed state to recover from, and re-running can repeat earlier commands that are not idempotent.
Suggested fix: collect the goroutine errors, for example through an error channel or a mutex guarded slice, and return a non-nil error from ExecuteLifecycleCommands when any command fails, so StopOnFailure behaves as intended.
Automated report: this issue was produced and filed automatically, with no human review before posting. Two independent checks agreed it is a real bug, but if it misreads the code please say so and we will close it.
Found while running Ito (AI code review that runs your application, free for open source) against recently merged PRs. Full analysis.
Source: harness/harness