fix(msb): skip the recursive home chown when the image baked the agent user - #432

Adversarial review — the performance fix is genuinely necessary and correctly implemented, but found a real, live-reproduced gap in the safety claim before deciding approve vs. request-changes.

Performance claim verified, not just plausible: pulled the actual devenv image the linked issue references into a real msb sandbox and timed the pre-fix unconditional chown -R over its real baked Nix home (81,850 files, already correctly owned) — 759 seconds (12.6 minutes), worse than the issue's own quoted figure. The fix genuinely eliminates this on the "agent already exists" path. Not a theoretical justification.

Real gap found in the stated safety property. The PR body claims "a wrong skip fails loudly rather than degrading silently." I built the exact counterexample this implies should be caught: a base image where adduser agent succeeds but a baked subtree under /home/agent is root:root (simulating a COPY --chown=root:root in a custom image's Dockerfile). Loaded it into a real sandbox and ran the actual guest script verbatim:

• The top-level chown "agent:${_agrp}" /home/agent fixes /home/agent itself.
• The root-owned subtree underneath is never touched — _acq_created_agent stays 0 because id agent succeeded, so the recursive chown that used to silently heal this is skipped.
• The su agent -c "test -w /home/agent" backstop only probes the named directory's own write bit — it does not detect the unwritable subtree beneath it, so it passes trivially.
• Result: the agent user gets a silent, permanent EACCES on that subtree — not the loud failure the PR body describes for this exact scenario.

Also checked the "kits already chown the subtrees they stage" mitigation directly against every kit in agentic-coding-patterns/integrations/isolation/acq-kits/ that stages files under /home/agent — none of them run their own chown; the coverage they get is from acq's own generic file-copy path (_acq_msb_copy_file_verified), which only covers files acq itself copies in via a kit's files: spec, not content the base image baked. So that mitigating claim doesn't actually cover the failure mode it's meant to.

Severity assessment, and why this is a comment not a block: this only affects a custom ACQ_MSB_IMAGE override that ships an agent user but violates the documented base-image contract (docs/BACKEND_GUIDE.md already states complete home ownership as an operator requirement for custom images). The default docker/sandbox-templates:shell-docker and the real devenv image I tested are both fully agent-owned, so the common path — and the actual bug this PR fixes — is unaffected. This is closer to "a documented contract violation is no longer silently papered over" than a new vulnerability, but the silent-vs-loud framing in the PR body is inaccurate for this specific edge case and there's no test covering it either way.

Suggested, non-blocking: a one-line note in docs/KNOWN_FAILURE_MODES.md about this edge case for custom-image operators, and/or a regression test for the half-baked scenario, would close the gap between the claim and the actual guarantee. Both new #422 bats tests are correct as far as they go (verified the refute_regex against both a real captured call log and a synthetic old-form line — sound, not vacuous) but they only cover the mechanical branch wiring, not this safety property.

No objection to merging as-is given the narrow, documented-contract-violation-only blast radius — flagging for the author's/maintainer's call on whether to add the doc note before or after merge.

Converting my earlier comment-only review to an approval — my prior adversarial findings (performance claim verified live at 12.6 minutes eliminated; the silent-degradation gap on a custom base image with incompletely-owned baked subtrees) were deliberately non-blocking: the common path (default sandbox template, the real devenv image) is unaffected, and the edge case only bites a custom ACQ_MSB_IMAGE that already violates the documented home-ownership contract in docs/BACKEND_GUIDE.md. Nothing has changed since that assessment that would newly justify blocking.

All CI checks green (the one CANCELLED "Conventional Commit Title" run is a stale duplicate superseded by the SUCCESS run immediately after it). Approved.