afx workspace recover: preserve spawnedByArchitect on respawned builders#1156
Merged
Conversation
# Conflicts: # codev/resources/lessons-learned.md
amrmelsayed
added a commit
that referenced
this pull request
Jul 8, 2026
amrmelsayed
added a commit
that referenced
this pull request
Jul 8, 2026
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
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.
PIR Review: Preserve spawnedByArchitect across
afx workspace recoverFixes #1140
Summary
afx workspace recover --applyrespawned builders by shelling out toafx spawnwith an inherited environment, so every recovered builder'sspawned_by_architectwas regenerated from the recovery shell'sCODEV_ARCHITECT_NAME(typicallymain) instead of being read from itsglobal.dbrow. This PR makes recovery read each builder's recorded architect via the existinglookupBuilderSpawningArchitecthelper (the same one the anti-spoofing check uses) and force it into the child process env, so sibling-architect workspaces keep per-builder attribution across recovery. Legacy rows with no recorded architect fall back to the caller's env, reproducing the pre-fix behavior for those rows only.Files Changed
packages/codev/src/agent-farm/commands/workspace-recover.ts(+113 / −30)packages/codev/src/agent-farm/__tests__/workspace-recover.test.ts(+108 / −1)codev/plans/1140-afx-workspace-recover-respawne.md(+93 / −0)codev/reviews/1140-afx-workspace-recover-respawne.md(this file)codev/resources/lessons-learned.md(+1 line, cold tier)codev/state/pir-1140_thread.md(builder thread log)codev/projects/1140-afx-workspace-recover-respawne/status.yaml(porch-managed)Commits
031bca87[PIR afx workspace recover: respawned builders lose their original spawnedByArchitect (all attach to shell's architect) #1140] Plan draft8077b7b0[PIR afx workspace recover: respawned builders lose their original spawnedByArchitect (all attach to shell's architect) #1140] Preserve spawnedByArchitect when workspace recover respawns builders75ee3383[PIR afx workspace recover: respawned builders lose their original spawnedByArchitect (all attach to shell's architect) #1140] Thread: implement phase noteschore(porch)commits are porch-managed state transitions.Test Results
pnpm --filter @cluesmith/codev build: ✓ passpnpm test(full package suite): ✓ 3443 passed, 48 pre-existing skips, 0 failuresworkspace-recover.test.ts: 57/57 pass (11 new: 6 forderiveBuilderInfoWithArchitect, 5 forrespawnEnv)dev-approvalgate (approved 2026-07-08); the multi-architect respawn path is additionally pinned by the "preserves distinct attribution across builders in the same workspace" unit test.Architecture Updates
No arch changes needed. The fix does not move any module boundary or introduce new state; it aligns
workspace-recover.tswith the already-documented design (attribution lives solely inglobal.db, per the Issue #1118 hot-tier fact) by reusing the existinglookupBuilderSpawningArchitectread path instead of adding a second source of truth.Lessons Learned Updates
One COLD-tier lesson added to
codev/resources/lessons-learned.md(Architecture section): when a process derives identity from an inherited environment variable, every programmatic respawn/recovery path must explicitly set that variable from recorded state rather than inheriting it, otherwise the respawned process's identity silently becomes a function of who ran the recovery. Not HOT-tier: it is a recipe for a specific bug class, not a behavior-changing cross-cutting rule, and the hot file is at cap.Things to Look At During PR Review
respawnEnv: legacy rows (no recorded architect) return the base env untouched rather than hardcoding'main'. This keepsspawn.ts'sDEFAULT_ARCHITECT_NAMEas the single owner of the default. Confirm you agree that legacy-row recoveries attaching to the recovery shell's architect (today's behavior) is the right degradation.try/finallyrestructure inworkspaceRecover: theallRowsconstruction moved inside the existing DB block so the per-builder architect lookups complete beforecloseGlobalDb(). Behavior is otherwise unchanged, but it is the largest visual diff hunk; verify the connection still closes before any childafx spawnlaunches (it does: the confirm prompt and respawn loop remain outside the block).lookupBuilderSpawningArchitectdistinguishesnull(legacy row) fromundefined(no row); recovery collapses both tonullsince they demand the same fallback. The distinction remains load-bearing in the message-routing spoofing check and is untouched.How to Test Locally
mainarchitect:sqlite3 ~/.agent-farm/global.db "SELECT id, spawned_by_architect FROM builders WHERE workspace_path = '<ws>'"afx workspace recover --applyfrom main's terminalspawned_by_architectafx send architect "ping"routes to the original architect; the VS Code Agents view shows the correct owner