Lessons from six NemoClaw refactorings that backfired

[cv]

A first pass through closed issues, merged PRs, reverts, and current code found six strong examples where the design goal was sound but the implementation removed distinctions that the system still needed.

1. The first messaging-plan migration was the right design in the wrong-sized change

Intent: Replace several flat messaging fields with one schema-versioned SandboxMessagingPlan.

• PR #4945 changed 82 files, with 2,954 additions and 1,703 deletions.
• It made the plan the sole persisted authority across onboarding, rebuild, status, doctor, inventory, and channel lifecycle code.
• It also added a ten-file validation subsystem with about 1,027 production lines.
• PR #5115 reverted the complete change six minutes after merge.

The repository does not record a concrete runtime failure in the revert, so we should not infer one. However, the replacement is informative:

• PR #5123 implemented the same accepted issue one day later.
• It used one 105-line plan-validation.ts module instead of the ten-file validation subsystem.
• The current design remains compact in src/lib/messaging/plan-validation.ts and src/lib/state/registry-messaging.ts.

Assessment: The plan model was good. The first migration coupled the schema change, persistence cutover, compatibility removal, validation framework, and every consumer into one change.

Lesson: Separate the canonical-model change from consumer migration and old-path removal. A concept can survive even when its first abstraction does not.

2. Centralizing E2E credentials erased job-specific meaning

Intent: Give legacy and Vitest E2E jobs one canonical hosted-inference environment.

• PR #5600 changed 18 files, with 1,850 additions and 3,582 deletions.
• Targeted workflow tests passed.
• PR #5684 reverted it about four hours later.

The rollback gives a concrete cause. Full post-merge E2E found widespread failures because the shared exporter treated several credential forms as interchangeable. Failures included:

• NVIDIA_INFERENCE_API_KEY not containing the required nvapi-* value.
• Inconsistent fallback and alias behavior between direct and reusable jobs.
• Endpoint validation failures and HTTP 401 responses.

The removed composite action still does not exist:

• .github/actions/export-e2e-hosted-inference/action.yaml
• .github/actions/export-e2e-hosted-inference/export.sh

Current workflows retain explicit consumer-level routing. For example, .github/workflows/e2e-standard-profile.yaml wires the relevant keys at the consuming job. The later PR #5688 used a narrower rule: Vitest jobs consume only NVIDIA_INFERENCE_API_KEY.

Assessment: This is the clearest DRY refactoring failure. The duplication represented distinct trust and provider contracts rather than accidental repetition.

Lesson: Do not centralize secret routing until every consumer uses the same credential semantics. Similar environment-variable names do not establish one contract.

3. Gateway authority modeled snapshots but missed a valid lifecycle transition

Intent: Give each gateway one explicit lifecycle owner and fail closed when ownership changes.

• PR #7246 introduced the gateway authority contract.
• PR #7319 integrated the default packaged service across 60 files, with 3,506 additions and 817 deletions.
• The security goal was sound: prevent NemoClaw from starting or stopping a gateway that another supervisor owns.

Issue #8215 exposed the missing transition:

1. Onboarding recorded packaged-service.
2. Uninstall removed the service file.
3. Teardown revalidation resolved the gateway as standalone.
4. The authority guard treated that expected transition as ownership drift.
5. nemohermes uninstall --yes failed instead of completing cleanup.

PR #8239 added a narrow uninstall exception. It remains visible in src/lib/actions/uninstall/run-plan.ts and src/lib/onboard/gateway-teardown-authority.ts.

Assessment: The abstraction represented two states accurately but did not represent the operation that legitimately moves between them.

Lesson: An authority model needs allowed transitions, not only state equality. Test the complete teardown order, including effects that remove the evidence later guards inspect.

4. A shared forward adapter collapsed “failure” into “empty”

This architectural extraction landed inside PR #4442, rather than a PR titled refactor.

Intent: Reuse forward cleanup and recovery behavior for the optional Hermes dashboard.

The adapter used:

deps.runCaptureOpenshell(args, opts) ?? ""

That converted two different states into one value:

• null: openshell forward list failed, so ownership was unknown.
• "": the list succeeded and contained no entries.

Issue #8522 traced the consequence. Cleanup interpreted a failed ownership query as no-entry and attempted forward stop when it was supposed to fail closed.

PR #8529 repaired the shared boundary. Current src/lib/onboard/forward-cleanup.ts now defines the runner as string | null and maps null to list-failed.

Assessment: The abstraction reduced type complexity by discarding information that the safety decision required.

Lesson: Normalize representation, not meaning. Avoid sentinel coercions such as null ?? "" when downstream code distinguishes failure from an empty successful result.

5. Dashboard recovery extraction made status unbounded

Intent: Make dashboard URL derivation, health checks, forwarding, and recovery use one explicit contract.

• PR #2398 extracted the delivery chain into contract, health, and recovery modules.
• The PR changed 11 files, with 569 additions and 468 deletions.
• It added 52 focused tests, and the PR reported that npm test passed.

The new recoverDashboardChain() orchestration also replaced the existing gateway recovery path with unbounded OpenShell calls. PR #2471 records the bisect and impact:

• sandbox-survival, skip-permissions, sandbox-operations, and cloud-e2e hung at nemoclaw <name> status.
• Commit 9fbfbaca from PR refactor(cli): extract dashboard delivery chain into contract/health/recover modules #2398 was the sole bad revision in the bisect.
• The repair restored the pre-refactor orchestration in src/nemoclaw.ts while leaving the pure dashboard modules in place.

Issue #2562 later documented the missing architectural contract: OpenShell child-process calls had no shared timeout support. Current code no longer contains recoverDashboardChain. It now defines named timeout budgets in src/lib/adapters/openshell/timeouts.ts, and recovery call sites pass those budgets explicitly.

Assessment: The extracted modules were not the problem. The new composition changed a non-functional guarantee: a status command that previously completed could now wait forever.

Lesson: Bounded execution is part of an interface contract. An extracted recovery pipeline must preserve deadlines across every injected dependency.

6. A file move left a hidden runtime import pointing at the old directory

Intent: Group inference and onboarding support modules by feature without changing behavior.

• PR #3195 moved support modules across 75 files, with 220 additions and 215 deletions.
• Static imports, tests, scripts, installer references, and source checks were updated for the new paths.

Issue #4139 found a path-sensitive dependency that the migration missed. src/lib/onboard-ollama-proxy.ts moved to src/lib/inference/ollama/proxy.ts, but three lazy calls still used require("./onboard").

From the new directory, that path no longer named src/lib/onboard.ts. The require failed, a catch block hid the error, and the Ollama capability gate fell back to environment variables. A rebuild that supplied nonInteractive: true in memory could therefore enter an interactive prompt path.

PR #5898 fixed the defect in three files. It removed the lazy imports and added an explicit OllamaToolCapabilityInteraction dependency for non-interactive, auto-yes, and confirmation behavior. That typed boundary remains in src/lib/inference/ollama/proxy.ts.

Assessment: The refactor treated the move as a static import rewrite, but the module also contained runtime path resolution and swallowed load failures.

Lesson: A file move is not behavior-neutral when code uses dynamic imports, relative file access, worker entrypoints, or runtime discovery. Inventory those dependencies before moving the file, and do not hide module-load failures behind a fallback.

Common pattern

These cases share four warning signs:

1. The abstraction removed distinctions before proving they were irrelevant.

   • Credential source and trust context.
   • Failed versus empty command output.
   • Expected lifecycle transition versus ownership drift.

2. The change combined adoption with old-path removal.

   • The first messaging migration changed the model, all consumers, validation, and persistence together.

3. Tests followed the new abstraction instead of challenging its boundary.

   • PR ci(e2e): centralize Vitest inference env #5600 passed targeted workflow tests but failed live composition.
   • Gateway authority tests covered ownership states but missed uninstall ordering.
   • Forward tests missed nullable capture behavior.
   • PR refactor(cli): extract dashboard delivery chain into contract/health/recover modules #2398 passed 52 focused tests and npm test, but four live E2E lanes hung.
   • PR refactor(cli): group inference and onboard support modules #3195 updated static imports but did not exercise the swallowed dynamic-import failure.

4. The repair was narrower than the introducing change.

   • One compact messaging validator.
   • One canonical Vitest credential.
   • One uninstall-specific authority option.
   • One explicit string | null contract.
   • One restored recovery path followed by shared timeout contracts.
   • One typed interaction dependency instead of a runtime import.

Cases that do not meet the evidence threshold

• Issue #7986 initially blamed a messaging structure change, but maintainers closed it as a test precondition failure.
• Issue #8113 was valid for the tested release, but that release predated the existing fix. It was not a regression after the fix.
• Issue #8847 dropped an explicit recreation flag at a state boundary, but blame traces the faulty line to feature PR fix(dcode): use OpenRouter provider for OpenRouter routes #6616 rather than a refactoring.
• Issue #8970 resembles a failed dashboard centralization, but the available history does not prove that PR refactor(onboard): centralize reused dashboard metadata #5130 introduced the finalization defect.

The conclusion is not “avoid refactoring.” It is: preserve meaningful distinctions, migrate one boundary at a time, and test the composed lifecycle before deleting the explicit paths.

[apurvvkumaria]

De-duplicated addendum

I compared an independent history pass with the six cases above. I omitted the messaging-plan and E2E credential findings because this discussion already covers them.

1. A config I/O helper imported the application runner

Intent: Reuse shellQuote while hardening shared configuration file operations.

PR #1370 made config-io.ts import shellQuote from runner.ts. That pulled the application runner into a low-level module. The runner used a CommonJS require("./platform") that could not resolve the TypeScript source module during Vitest execution.

PR #1900 repaired main 49 minutes later. It extracted shellQuote into a dependency-free leaf module. The same dependency direction remains in src/lib/core/shell-quote.ts.

Assessment: The reuse removed duplicate code but reversed the intended dependency direction.

Lesson: Put shared utilities below their consumers. Do not import an application entry module from a low-level utility.

2. Gateway-token externalization omitted the interactive-shell consumer

Intent: Remove the agent gateway token from agent-readable openclaw.json.

PR #2378 moved the token into a runtime file and passed it through OPENCLAW_GATEWAY_TOKEN. The security goal was sound.

Issue #2480 showed that openclaw tui could no longer authenticate after nemoclaw <sandbox> connect. The runtime path did not supply the token to that execution identity under the affected Landlock and non-root flow.

PR #2482 reverted the change about 43 hours later. Current scripts/nemoclaw-start.sh has explicit, command-aware token preparation.

Assessment: The change improved credential custody for the agent gateway but omitted another valid consumer.

Lesson: Before moving credential custody, inventory every execution identity and every path by which it receives the credential.

3. Moving npm link earlier converted a two-minute failure into a twenty-minute failure

Intent: Move slow Brev E2E setup outside a 120-second test timeout.

PR #2186 moved sudo npm link into launchable setup. Cold Brev instances then hung until the 20-minute outer limit instead of failing after two minutes.

PR #2196 reverted the change about five hours later. It replaced package-manager work with the one direct CLI symlink that the test required.

Assessment: The refactoring moved an unbounded operation across a timeout boundary without bounding or simplifying it.

Lesson: Moving expensive work does not bound it. Simplify the operation or give it an explicit deadline.

4. The oclif migration preserved two command grammars and accumulated boundary repairs

This is a cumulative case, not one reverted pull request (PR).

Intent: Give NemoClaw one command base, parser, help system, and error path while preserving its sandbox-first public grammar.

PR #3611 moved commands onto a shared oclif base. PR #3635 then changed 190 files to align public dispatch with oclif while retaining NemoClaw grammar. PRs #3833 and #3838 reduced some migration bridges but retained direct command-ID execution.

Later repairs exposed the remaining boundary:

• PR #5209 preserved translated process.argv and stopped oclif from filling missing positionals from stdin.
• PR #5986 corrected user errors that printed an error but exited with status 0.
• PR #8125 stopped raw oclif parse-error stack traces from reaching users.

Current src/lib/cli/oclif-runner.ts still contains separate native-argv and direct command-ID execution paths.

Assessment: The migration centralized command registration but retained an impedance mismatch between public and framework-native grammar.

Lesson: A parser migration must preserve argv, stdin, help, error, and exit-code contracts. Command registration alone does not establish one command path.

[rsliter]

Additional cases from an independent history and code pass

I omitted the dashboard recovery, E2E credential, and moved-module findings because the main post already covers them.

1. A formatting sweep hid a terminal I/O rewrite

Intent: Expand Biome coverage to every tracked TypeScript file and apply consistent formatting.

PR #5020 changed 787 files, with 14,427 additions and 13,231 deletions. Inside that mostly mechanical diff, the uninstall prompt replaced a child-shell read with a manual fs.readSync(0) loop. The same function evaluated process.stdin.isTTY first.

Issue #5188 traced the result. Accessing process.stdin made fd 0 non-blocking on Linux. The next synchronous read received EAGAIN, the broad catch treated it as end-of-input, and every interactive uninstall immediately aborted.

The existing tests injected readLine, while CI used --yes. Neither path exercised a real terminal. PR #5163 added direct TTY detection, explicit EAGAIN and EINTR handling, and a PTY integration test. The resulting complexity remains in src/lib/core/stdin.ts.

Assessment: Formatter adoption was useful. Combining it with a semantic I/O rewrite inside a 787-file change made the behavior change difficult to review and left the production boundary untested.

Lesson: A mechanical formatting PR must not contain manual semantic rewrites. Interactive terminal behavior needs a PTY test.

2. The onboarding FSM migration kept two transition authorities active

Intent: Replace coarse imperative onboarding state with a serializable, observable state machine that supports resume and repair.

Issue #3802 defined a careful staged migration. PR #3848 began the machine layer on May 20, and the umbrella closed on June 16.

The merged design still had overlapping mutation paths. Four June 24 refactors had to distinguish record-only updates from legacy machine mutation:

• PR #5687
• PR #5689
• PR #5701
• PR #5708

PR #6253 then replaced three implicit recovery mechanisms with one explicit path. PR #7672 introduced strict final entry. PR #7716 finally made OnboardRuntime the normal transition authority on August 4 and removed the compatibility bridge.

The parallel implementation also created a concrete path-specific regression. PR #8216 found that the machine handler discarded the actual dashboard port returned after collision handling. The old imperative path persisted that value. The repair added a composed test that drives the production handler path. Current ownership is now explicit in src/lib/onboard/lifecycle-contracts.md.

Assessment: The FSM destination was reasonable. The migration strategy kept shadow state and compatibility mutation active for too long, so later work had to reconcile authorities before it could simplify behavior.

Lesson: Each merged migration slice should have one state-mutation authority. A shadow implementation may observe and compare, but it should not persist competing state.

Adjacent case: tar hardening rejected valid product archives

This is a fix that backfired, not a refactoring, so I would not count it among the discussion title.

PR #2163 correctly addressed a high-severity host-side tar traversal risk. Its 14 security tests covered malicious entries, but not representative NemoClaw backups containing absolute in-sandbox symlinks.

Issue #2317 showed that the new shared extractor blocked snapshot, rebuild, and backup operations. PR #2308 and PR #2488 added sandbox-aware symlink handling. The current exceptions remain visible in src/lib/state/sandbox.ts.

Lesson: Security boundary tests need both malicious inputs and representative valid product artifacts. A fail-closed control is still defective when normal product state is always classified as hostile.

These cases reinforce the main post: tests followed the new seam, while the missing evidence lived at the composed runtime boundary.

[cv]

A related category is architecture tax or complexity transfer, rather than an outright refactoring failure.

The behavior can become safer and the local functions can become simpler while the system becomes harder to understand. Complexity moves into contracts, phases, state transitions, dependency wiring, and exceptions.

1. Onboarding FSM: lower function complexity, higher system complexity

PR #5913 had a strong goal:

• Replace a handleSandboxState function with cognitive complexity 103.
• Introduce explicit resume decisions.
• Keep each affected function at cognitive complexity 10 or lower.
• Separate messaging reconciliation from sandbox state orchestration.

The refactor succeeded by its local metric. However, the original 603-line sandbox handler has evolved into three core modules totaling 2,326 lines:

• src/lib/onboard/machine/handlers/sandbox.ts: 1,295 lines
• src/lib/onboard/machine/handlers/sandbox-resume.ts: 351 lines
• src/lib/onboard/machine/handlers/sandbox-messaging.ts: 680 lines

The repository now tracks the broader problem explicitly:

• Issue #7695 says src/lib/onboard.ts has more than 200 runtime dependencies and still mixes dependency wiring with product decisions.
• Issue #7691 reports 358 production files and about 66,600 lines under src/lib/onboard/.

What happened: Branch complexity became distributed coordination complexity. Understanding one decision now requires following state, runtime, phase, resume, messaging, and persistence contracts across files.

Verdict: Strong architecture-tax example. The explicit decision model was worthwhile, but cognitive complexity per function was too narrow a success metric.

2. Transactional rebuild: data safety through a large compensating system

The architectural intent was excellent: rebuilding must not destroy a working sandbox and then fail without recovery.

Issue #2306 identified the original structural problem:

• Rebuild called the user-facing onboard() flow as a subroutine.
• Onboarding could call process.exit(), preventing rollback.
• Rebuild communicated through a global onboarding session.
• Credential resolution had competing authorities.

PR #2523 centralized credential resolution and added guards. However, it explicitly deferred the focused recreation path because the relevant onboarding functions contained 148 process.exit() calls.

The current rebuild surface has approximately:

• 65 production files and 13,336 lines
• 72 test files and 23,040 lines

src/lib/actions/sandbox/rebuild-pipeline.ts alone has:

• 564 lines
• 29 import declarations
• Separate backup, preflight, delete, recreate, restore, shields, messaging, MCP, route, registry rollback, and Hermes cron phases

What happened: Atomicity was achieved through preflight receipts, journals, rollback objects, phase results, revalidation, and recovery paths. Those controls improved data safety, but the pipeline became a second composition root spanning many product domains.

Verdict: Strong architecture-tax example. Much of the complexity is necessary, but the current dependency surface suggests the original focused recreation boundary remains incomplete.

3. Gateway lifecycle authority: stronger security, larger transition matrix

PR #7246 established explicit gateway lifecycle ownership. PR #7319 extended it to packaged services.

The security goal remains correct:

• Identify the gateway owner.
• Persist that authority.
• Revalidate it before effects.
• Refuse to mutate a gateway owned by another supervisor.

A path-based inventory of the current gateway lifecycle surface finds approximately:

• 48 production files and 9,789 lines
• 60 test files and 15,389 lines

Four central modules alone total more than 1,700 lines:

• gateway-management.ts
• gateway-ownership.ts
• gateway-host-runtime.ts
• gateway-teardown-authority.ts

The packaged-service → standalone uninstall exception from Issue #8215 shows how the transition matrix grows. The model must now distinguish:

• Current owner
• Recorded owner
• Supervisor source
• Gateway identity
• Operation type
• Expected service removal
• Credential mutation versus teardown
• Ordinary drift versus an allowed lifecycle transition

What happened: One ownership concept expanded into several state authorities and operation-specific exceptions.

Verdict: Mixed architecture tax. Much of this complexity is essential security complexity. The incidental part is that ownership detection, lifecycle policy, runtime inspection, and operation sequencing remain spread across several modules.

A weaker candidate: messaging

The messaging architecture currently covers about 148 production files and 22,700 lines across manifests, compilation, persistence, application, hooks, and onboarding.

However, I would not yet attribute that size to the plan architecture itself. Much of it comes from supporting several channels with different runtime and health contracts. It needs a feature-normalized comparison before it meets the same evidence threshold.

The broader lesson

These cases show why local complexity metrics are insufficient.

A refactor can reduce:

• Function length
• Cognitive complexity
• Duplicate branches
• Direct conditionals

While increasing:

• Fan-out
• Number of state authorities
• Transition count
• Cross-feature imports
• Required context for one decision
• Verification cost
• Exception flags

For architecture work, it is useful to track these measures together:

1. Composition-root import count and fan-out.
2. Number of modules needed to explain one user operation.
3. Number of persisted and runtime state authorities.
4. Number of operation-specific exceptions.
5. Runtime dependency cycles.
6. End-to-end transition coverage.
7. Flat-folder and production-line growth.

These cases should remain separate from ordinary regressions. They represent architecture changes that traded local simplicity for system complexity.

[cv]

Another useful category concerns test-oriented changes and the production contracts they validate.

I used a strict threshold here: a test-oriented PR must change production behavior, and a later issue or repair must attribute a production regression to that change. The current first-parent history contains 173 merged test-oriented PRs. Twenty-six of them changed production files. I did not find a later closed issue that proves one of those 26 PRs introduced a production regression.

The evidence does support three related patterns.

1. A behavior test entrenched a destructive production bug

PR #3653, test(cli): replace source-shape checks with behavior tests, had a sound goal:

• Replace source-text assertions with behavior tests.
• Extract production helpers for direct testing.
• Preserve gateway cleanup behavior.

It extracted verifyGatewayContainerRunning() and added this explicit test:

it("returns missing for existing but stopped containers", () => {
  const dockerInspect = vi.fn(() => dockerInspectResult(0, "false\n"));

  expect(verifyGatewayContainerRunning("nemoclaw", { dockerInspect })).toBe("missing");
});

That distinction was wrong. Treating an existing stopped container as missing sent onboarding into destructive cleanup.

Issue #4187 later reported the production impact:

• A host stop/start left the gateway container stopped.
• NemoClaw classified it as missing.
• NemoClaw deleted the existing gateway volumes.
• A new PVC was provisioned.
• The user’s workspace appeared empty.

PR #4210 changed the result model to:

• running
• stopped
• missing
• unknown

It then added non-destructive recovery for stopped.

The #3653 diff shows that the inline production code already collapsed stopped into missing before extraction. Therefore:

• The test PR did not introduce the production bug.
• It converted the bug into an explicit, passing behavioral contract.
• It made a dangerous assumption look intentional and protected it from incidental correction.

Classification: Test-entrenched production defect, not test-caused regression.

Lesson: Replacing source-shape tests with behavior tests is not sufficient. The expected behavior itself needs domain review.

2. A regression test created false confidence by covering only one topology

Issue #2553 fixed Ollama proxy-token divergence after re-onboarding.

PR #2606 then added a GPU double-onboard regression test.

Later, Issue #8704 reproduced the same HTTP 401 failure when onboarding a second sandbox through a second gateway.

The issue explicitly concludes that either:

• The production fix regressed, or
• The test(e2e): add GPU double-onboard E2E test for Ollama proxy token consistency #2606 test did not cover the second-sandbox and second-gateway variant.

The test did not cause the later defect. It proved one topology and was treated as broader evidence than it provided.

Classification: False-confidence regression guard.

Lesson: A lifecycle regression test must state its topology dimensions:

• Same or different sandbox
• Same or different gateway
• Same or different port
• Existing or rotated credential
• Shared or isolated runtime

A test named “double onboard” can hide several different state transitions.

3. Security E2E tests duplicated production logic and could pass during a regression

PR #1092 added security E2E coverage for credential sanitization and command injection.

Issue #1107 found that the tests reimplemented production behavior:

• Credential filtering was copied into test heredocs.
• Directory sanitization was reimplemented.
• Digest verification was duplicated.
• Telegram execution bypassed the production runAgentInSandbox() and shellQuote() path.
• Some test behavior had already drifted from production behavior.

The issue states that these tests could pass even if production regressed.

I did not find a later issue proving that this test design caused a specific production regression. However, it is a direct example of a virtuous security-test change producing misleading evidence.

Classification: Self-fulfilling test, not test-caused regression.

Cases excluded from the finding set

PR #4911

PR #4911 changed 248 production lines while replacing slow subprocess matrices with unit tests. It added several dependency-injected production seams, but no later issue attributes a regression to it.

PR #4437

PR #4437 was test-labeled but changed the production OpenClaw version in agents/openclaw/manifest.yaml. That is a governance warning because a test PR also acted as a dependency upgrade. However, no later issue directly attributes a production regression to that version change.

Recommended taxonomy

Team findings should distinguish these cases:

1. Test-caused regression
   A test-oriented change modifies production behavior, and a later bisect or repair identifies that change as the cause.

2. Test-entrenched defect
   The defect already exists, but a new test declares the wrong behavior correct.

3. False-confidence guard
   The test covers one variant but is treated as evidence for a broader contract.

4. Self-fulfilling test
   The test duplicates the implementation instead of exercising production code.

At present, #3653 is the strongest case, but it belongs in category 2 rather than category 1. A confirmed test-caused production regression still requires a later repair or bisect that names the test-oriented production change.

[senthilr-nv]

Four additional refactoring failures from a de-duplicated pass

I excluded cases already covered in the discussion. I used the same evidence threshold: a closed issue, repair, or current contract must connect the abstraction to a concrete regression.

1. Typed config I/O converted malformed state into missing state

Intent: Give credentials and the sandbox registry one typed JSON I/O layer with atomic writes and permission-aware errors.

PR #1552 introduced readConfigFile() as core substrate for later migration. The helper returned its fallback for every non-permission exception, including JSON.parse failures. The registry supplied an empty-registry fallback.

That combination made a malformed but present registry indistinguishable from a missing registry. Issue #8420 reproduced the destructive sequence:

1. load() parsed malformed sandboxes.json and received an empty registry.
2. registerSandbox() added one new entry to that fallback.
3. save() atomically replaced the malformed file.
4. Every pre-existing registry entry and the recoverable original bytes were lost.

PR #8443 split file reading from parsing. Current config-io.ts returns a fallback only for ENOENT; malformed JSON raises ConfigCorruptError, and registry mutation stops before a write.

Assessment: Atomic replacement made the write reliable, but the shared read abstraction supplied unverified fallback state to a destructive writer.

Lesson: A generic configuration helper must distinguish absent, malformed, unreadable, and valid state. A fallback is valid only for callers whose domain treats absence as initialization.

2. Stateless onboarding heartbeats required two process-wide registries

Intent: Remove silent onboarding waits while moving prebuild and readiness work out of the entrypoint.

PR #6166 described its 30-second progress output as stateless. It also emphasized removal of the earlier shared timing registry.

Two later issues showed that the observer needed state that the phase wrapper did not own:

• Issue #6651 showed that a heartbeat could print while an asynchronous prompt owned the terminal. It moved or overwrote the active menu selection.
• Issue #7156 showed Still working on Provider selection during model download and vLLM startup, after provider selection had completed.

The repairs added cross-cutting coordination:

• PR #6664 added a process-wide prompt-activity registry. Credential and messaging prompts now register their complete lifetime.
• PR #7164 added a process-wide phase-activity registry. Long operations now override the outer machine-state label.

Both contracts remain in prompt-activity.ts, phase-activity.ts, and phase-progress.ts.

Assessment: The phase wrapper reduced entrypoint code, but phase identity was not enough to describe terminal ownership or active work. The repair transferred that missing context into two process-wide registries.

Lesson: Background terminal output is not stateless. Its contract must include the current output owner and the operation that actually owns elapsed time.

3. A deny-all messaging fix erased an explicit empty plan

Intent: Reject channel mutations before destructive effects when an agent supports no messaging channels.

PR #5743 correctly distinguished an omitted channel allowlist from an explicit empty allowlist. It centralized that distinction in the messaging workflow planner. However, buildRebuildPlanFromSandboxEntry() then returned null whenever filtering produced zero channels.

That collapsed two different states:

• null: no persisted messaging decision exists.
• an empty plan: the operator explicitly removed the final channel.

Issue #5759 recorded the result. channels remove telegram succeeded and persisted an empty plan. Rebuild converted it to null, cleared NEMOCLAW_MESSAGING_PLAN_B64, rediscovered TELEGRAM_BOT_TOKEN from the environment, and re-added Telegram.

PR #5760 preserved a plan that was already empty and staged it into the rebuild environment. Current workflow-planner.ts and rebuild-messaging-stage.ts retain that distinction and its regression tests.

Assessment: The abstraction modeled channel eligibility but reused emptiness to mean both denial and explicit removal.

Lesson: Persisted absence and a persisted empty set are different commands. Filtering must preserve the source state that explains why the set is empty.

4. A trusted launcher was not a read-only launcher

Intent: Prevent sandbox-user startup files and curl configuration from forging DCode route-health evidence.

PR #6497 routed DCode probes through the image-baked /usr/local/bin/nemoclaw-start entrypoint. Reuse preserved the trusted proxy environment and avoided the untrusted login-shell path.

The reused boundary also owned runtime startup state. Issue #6504 proved that status, doctor, rebuild-preflight, or connect probes could delete /tmp/nemoclaw-observability-enabled when the probe environment omitted NEMOCLAW_OBSERVABILITY=1. A read-only health probe therefore disabled managed observability.

PR #6506 added a separate root-owned dcode-managed-exec mode that reconstructs the trusted proxy environment without running the stateful entrypoint. Current connect-inference-route-probe.ts uses that path and fails closed for older images where it is absent. Current dcode-launcher.sh exact-execs the probe before process-supervisor behavior.

Assessment: Reusing the trusted launcher preserved input trust but violated the probe effect contract. Trustworthy input handling did not imply read-only execution.

Lesson: A diagnostic capability needs both a trust contract and an effect contract. Do not reuse a lifecycle entrypoint for observation unless the observation path is proven side-effect free.

Common pattern

These cases add one refinement to the discussion theme: the abstractions often preserved one dimension and erased another.

• Config I/O preserved atomicity but erased state validity.
• Heartbeats preserved elapsed time but erased terminal and work ownership.
• Messaging filtering preserved eligibility but erased operator intent.
• DCode probing preserved input trust but erased the read-only effect boundary.

A useful review question is: Which distinctions did this shared boundary remove, and which downstream decisions still depend on them?