feat: materialize workspace project sessions#2224
Conversation
|
Claiming this PR to continue the remaining backend workspace-project work from #2222. I’ll keep the next slice focused and update this branch with tests. |
|
Correction to the previous comment: shell expansion stripped some backticked text while posting. Pushed CI fixes in aad293b. What changed:
Local verification:
Remaining #2222 work is still separate from this PR's current backend slice: multi-worktree teardown/restore, SCM/PR repo-aware observation and claim, root change flow, and status aggregation. |
098bf9a to
30909c0
Compare
…-19/workspace-projects-backend # Conflicts: # backend/internal/session_manager/manager.go
neversettle17-101
left a comment
There was a problem hiding this comment.
Review: approve (no blocking issues).
Solid, well-structured addition. CreateWorkspaceProject/DestroyWorkspaceProject provision the root-as-repo parent plus children with a shared branch, the branch-collision resolver picks a name free in every repo, and the spawn rollback path is threaded through destroySpawnWorkspace for all failure points. ListWorkspaceRepos→CreateWorkspaceProject wiring and the session_worktrees cascade FK make partial-failure cleanup safe. The packages build cleanly and the new tests cover the create/destroy and zombie paths.
A couple of minor, non-blocking observations below.
| if err != nil { | ||
| for i := len(created) - 1; i >= 0; i-- { | ||
| _ = w.forceDestroyPath(ctx, created[i].repoPath, created[i].outputPath) | ||
| } |
There was a problem hiding this comment.
Minor: forceDestroyPath removes the worktree + dir but leaves the -b branch ref created by worktreeAddNewBranchArgs in each already-created repo. On a partial-create rollback those branches are leaked; a retry then resolves to branch-2 via workspaceProjectBranch. Not harmful (consistent with how Destroy leaves branches generally), but worth a comment or a branch -D on rollback if you want clean retries.
| } | ||
|
|
||
| func isZombie(pid int) bool { | ||
| out, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output() |
There was a problem hiding this comment.
isZombie shells out to ps on every Alive() call. Current callers invoke Alive once per CLI command / runfile read, so the fork cost is negligible here — fine as-is. Flagging only so that if Alive ever moves onto a polling/reconcile hot path, this should switch to reading /proc/<pid>/stat on Linux to avoid a per-probe process spawn.
Summary
Scope
This is the first backend slice for #2222. It does not yet implement multi-repo kill/restore/save lifecycle, SCM/PR aggregation across child repos, or frontend/API exposure.
Tests
Closes part of #2222