fix(temp): recover deleted managed namespace

[JAORMX]

What changed

Fixes the managed temporary namespace failure that caused Bash to disappear from later runs of a still-live session after the namespace pathname was deleted and recreated.

The recovery path now:

• reopens or recreates the exact configured private namespace under synchronization;
• retires stale cached root handles while preserving independently pinned in-flight operations;
• revalidates ownership, mode, type, symlink status, and visible inode identity before adoption or mutation;
• allocates each command lease from a freshly validated workspace and closes its handles with the lease;
• propagates configured runner failures instead of silently turning local, forked, Team, Parallel, or writable Subagent environments shell-less;
• keeps intentional no-Bash and no-FS environments unchanged;
• returns a path-free, actionable managed-storage failure when safe recovery is impossible.

Tests

Added coverage for:

• deletion and recreation after namespace/workspace handles are already open;
• stale inode replacement;
• unsafe file, symlink, ownership, and permission replacements;
• concurrent recovery, allocation, and sweeping;
• mutation-boundary identity checks;
• handle ownership and closure;
• same-session Bash use before and after recovery;
• direct-write Subagent recovery;
• read-only/forked child runner failure propagation;
• request-manifest advertised versus mount_unavailable decisions;
• intentional no-Bash and no-FS behavior;
• macOS managed-temp test selection.

Verification

• task lint passed.
• GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 task test passed.
• Focused recovery tests passed under -race and repeated concurrency stress.
• task site:build passed.
• Strict documentation link checks passed.
• go run ./cmd/mecademo passed.
• git diff --check passed.

task docs reaches the unchanged docs/acceptance/native-llm-gateway-login.md and fails its existing placeholder checks. Configuration generation produced no tracked drift, and the independent citation/link/site checks passed.

Fixes #1337

[jhrozek]

MoE panel review (spec / standards / test-adequacy / secure-code-reviewer / go-architect / library-reuse / duplication axes, run at Opus effort given the safety-critical concurrency surface) of this first round. This is a well-engineered fix for a genuinely well-diagnosed production incident — the core recovery/confinement logic is race-hardened (every mutation-relevant check is an fstat on an already-open handle, never a validate-then-reopen-by-path), and the test suite is unusually rigorous (asserting directory-entry counts and inode identity, not just "an error came back").

One note on process: two specialists directly disagreed on refreshLocked's inode-reuse handling (go-architect called it a blocker; secure-code-reviewer called it correct). I worked through the Unix semantics myself and secure-code-reviewer is right — as long as the retained handle's fd stays open across the external rm -rf && mkdir sequence, the kernel cannot hand the freed inode number to the newly-created directory (unlink-while-open keeps an inode pinned until the last fd closes), so SameFile correctly detects the difference. Not a live bug, but worth the one-line comment secure-code-reviewer suggested, so a future refactor that swaps fd-retention for a cached (dev,ino) pair doesn't silently reintroduce the incident (see inline comment on namespace_unix.go).

Blocking on: no acceptance plan / no disclosed spine waiver for what's at minimum Bounded (plausibly Architectural) work, and four cross-confirmed important findings below. Non-blocking advisories (magic-number layout coupling, dead code, an orphaned forker option, sweep-into-retired-tree bookkeeping, a mislabeled background/temp-scope error) are left for the author's judgment rather than posted individually.

PANEL: ship_blockers=1 important=6 advisory=9 reviewer_failures=0

[jhrozek]

[BLOCKER] No acceptance plan for what reads as Bounded-at-minimum work. This PR changes security/trust behavior (a configured shell that previously degraded silently to shell-less now hard-fails placement instead), adds cross-subsystem seams (forker.WithRunnerError, Namespace.AllocateWorkspace, osfs.WithManagedTemporaryAllocator, a new port.LLMProvider decorator), and changes operator-visible failure text — yet ships with no docs/acceptance/<slug>.md. Per docs/development-process.md, the only two carve-outs are a human-authorized Spike or an explicit human waiver ("skip the spine for this") — the PR body discloses neither. (Anchoring this comment on the ADR-0027 edit line since there's no plan file to anchor it to.) Could you add the plan retroactively, or confirm a waiver was given so it's on the record?

[jhrozek]

[IMPORTANT, cross-confirmed by 4 independent reviewers: Standards, Spec, go-architect, secure-code-reviewer] Real failure cause discarded — operator can no longer distinguish the failure modes this PR was written to make diagnosable.

The pre-PR code logged "err", err at WARN here; now the caller gets only the generic sentinel, and the same drop happens two lines below at fmt.Errorf("configured command runner unavailable; ...") (no %w) and in temporary_storage.go's allocator closure. For a fix motivated by exactly this kind of production diagnosis gap, losing "was it a symlink? wrong owner? wrong mode? ENOSPC?" at the one place that knows is the wrong direction. Redaction is already handled downstream — this PR's own TestManagedTemporaryPlacementErrorRemainsActionableAndRedacted proves sanitizePlacementProviderError/errors.Join strips a joined private cause on the way to the client — so wrapping and WARNing here costs nothing:

Suggested change

	return nil, server.ErrManagedTemporaryStorageUnavailable
	allocate, err := cfg.managedTemp.allocator(root)
	if err != nil {
	cfg.diag().Log(context.Background(), port.LevelWarn, "managed temporary storage unavailable; configured Bash will fail until restored", "root", root, "err", err)
	return nil, errors.Join(server.ErrManagedTemporaryStorageUnavailable, err)
	}

(adjust imports/signature as needed for this file's actual cfg.diag()/logging call shape — I don't have the surrounding context to get the exact call right, but the shape is: log the cause, join it into the sentinel rather than dropping it).

[jhrozek]

[IMPORTANT, cross-confirmed by 4 independent reviewers: library-reuse, go-architect, secure-code-reviewer, Spec] writableRunnerProvider is a filesystem preflight bolted into the port.LLMProvider seam, running per-turn, per-writable-child.

Four independent problems raised across the panel:

1. Wrong seam — runner availability is an Environment/CommandRunner fact (AGENTS.md's ADR-0211 seam); this adds a second, unrelated place that must be kept in sync with it.
2. Per-turn cost, discarded result — commandRunnerForRoot here opens a managed workspace, revalidates the namespace, constructs a full osfs.CommandRunner, and throws it away every single Stream call; the runner actually used by the child is a different, earlier-constructed one.
3. Uncancellable blocking flock now on the model hot path (secure-code-reviewer, CWE-833/400) — this call chain reaches OpenWorkspace's lockExclusive(workspaceLock, false), a raw blocking syscall.Flock with no LOCK_NB and no ctx. Before this PR that flock ran once at Build/placement-bind time; now it runs on every LLM turn of every writable subagent. A stalled peer (SIGSTOP, a debugger, an NFS stall) wedges the agent loop in a syscall that Run.Cancel/hardAbort cannot reach.
4. Redundant — failedCommandRunner (this same file) is already the correct, already-wired representation of this exact failure at the exact seam where the child actually runs Bash; it surfaces an actionable tool error at execute time without a provider wrapper.

Suggest deleting guardWritableRunner/writableRunnerProvider and doing the check once at child-Environment construction/execute instead (mirroring how failedCommandRunner already works), per the comment already on this type ("Rechecking also permits recovery without rebuilding a cached child engine" — that goal is still achievable at the execute seam).

[jhrozek]

[IMPORTANT] failedCommandRunner is non-nil, so roIsolationAvailable/mainRunner != nil gates (here and at line 7437) are now permanently true once construction happens once — even after the namespace recovers. (Spec axis finding.)

Before this PR, a failed newHardenedCommandRunner/buildSandboxedCommandRunner returned nil, and these != nil checks correctly gated team-member/agent-def Bash availability on ACTUAL runner health. Now a failure returns a non-nil failedCommandRunner{err: err} marker, so memberRunner != nil is true even when construction failed — roIsolationAvailable becomes true, allowShell becomes true, and the specific error captured at Build time is baked in and never re-evaluated. Contrast this with the writable-subagent path, which deliberately rechecks per-call (see the comment on writableRunnerProvider, even though that recheck is itself in the wrong place per the other comment). Once the namespace successfully recovers, these team/agent-def children are stuck permanently failing on a stale error, with no path to retry. Either gate on runner != nil && runner.(failedCommandRunner).err == nil-equivalent, or re-derive the runner at the point it's actually used rather than caching the Build-time result as the sole source of truth.

[jhrozek]

[IMPORTANT] Fork-runner-builder gate closure duplicated across 3 production sites (+3 test sites), and this PR is the proof of the cost.

The same three lines (if !<gate>(cfg) { return nil, nil }; return newHardenedRunnerForRoot(cfg, childRoot)) appear here, in buildSubagentTool (build.go ~7143-7147), and in buildTeamWiring's roRunnerBuilder/fkRunnerBuilder (build.go ~7554-7566) — varying only in the gate predicate (forceCopyShellAvailable/sandboxedShellAvailable). Widening the closure's return signature from tool.CommandRunner to (tool.CommandRunner, error) for THIS PR required editing all of them, which is exactly the kind of edit that's easy to miss in one spot (Rule of Three met twice over in production alone). Suggest extracting func forkRunnerBuilder(cfg Config, gate func(Config) bool) func(string) (tool.CommandRunner, error) and calling it from all three sites; the existing team/parallel/subagent shell tests are the oracle that nothing changed behaviorally.

[jhrozek]

[ADVISORY — resolved cross-agent disagreement, documenting the resolution] go-architect flagged this SameFile comparison as a blocker, hypothesizing that after an external rm -rf && mkdir, the filesystem could hand the freed inode back to the recreated directory, making this comparison silently report "unchanged" and retain the stale handle — reproducing the original incident.

That can't happen as long as n.root's fd stays open across the delete+recreate: POSIX unlink-while-open semantics keep the old inode alive/pinned (nlink may drop to 0, but the kernel won't recycle that inode number while a descriptor still references it) until the last close — which here is current.Close() a few lines below, i.e. after this comparison already ran. secure-code-reviewer's read is correct.

The invariant is real but implicit and load-bearing: if this code is ever refactored to cache (dev, ino) instead of holding the fd open, this protection silently breaks and reintroduces the exact incident #1337 describes. Worth one comment line here saying so, for the next person who touches this function.