Repository navigation
Conversation
Provide a one-shot terminal confirmation route for a managed service whose storage marker no longer matches its host-local directory identity. Revalidate the deployment generation and exact prepared root before rebinding, while keeping normal startup strict. Generated-by: OpenAI Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Reviewed 7419c046add9189dcb455b96c0ccc29453d39fa8 (all eight files, +693/-60). I found no confirmed P0–P2 issue. The new runtime-host service repair-root command gives managed roots an explicit recovery route while retaining strict normal-startup identity checks.
I traced CLI parsing/dispatch and startup diagnostics through the new repair command, the managed deployment reader and the existing storage-root repair implementation. The candidate pins the canonical directory, root ID, directory identity and marker snapshot. Confirmation precedes the deployment/lifecycle locks; under those locks the command rechecks the active deployment or legacy configuration before the storage layer revalidates and atomically replaces the marker. The root ID remains unchanged. The new deployment reader returns configuration, not a root capability or launch authorization.
Validation completed:
- Fresh dependencies/patches, the normal clean root test build, CLI type checking, Biome on all eight files and
git diff --checkpassed. - 77 relevant original tests: 74 passed, three platform-condition skips, no failures. These cover the new recovery command, existing service management, managed deployment and storage-root authority. Account home/cache/control paths were isolated in temporary directories; the existing suites' reported sandbox permission failures did not occur here.
- I invoked the actual compiled CLI with a real pseudoterminal. Piped
yeswas rejected, an empty terminal answer cancelled without mutation, and explicityesrepaired both legacy and canonical deployments while preservingrootId. Replacing the directory, legacy configuration or canonical deployment revision during confirmation prevented repair. Ordinaryservestill rejected the stale marker and printed explicit recovery guidance. - Removing only the deployment/configuration recheck from the loaded repair module makes two unchanged regression tests fail: both changed configurations are incorrectly accepted. The directory/marker checks remain active in that control.
P3 — retain the client-directory selector in legacy recovery guidance. At cli-core.ts's legacy-service catch path and runtime-host-root-repair-command.ts:225–231, the suggested command retains --root but loses a custom service's client-data directory. Starting from a legacy --managed-service-config under an isolated custom client directory, following the printed command returns “Runtime Host managed service configuration is unavailable”. Supplying the existing --client-data-root <that directory> option succeeds. Including that selector in the diagnostic would make this older installation path easier to recover; the explicit repair command itself works with it.
Exact-head CI test and Windows recovery pass. CLI package validation remains red at the production dependency audit (two moderate/two high findings), so installed-package qualification was skipped. The dependency manifests, lockfile and release/audit scripts are unchanged from current main; this is not a demonstrated PR-introduced dependency regression. I do not describe all CI as green.
The static merge with main 0d19f242336a31e8aeb300372d3d3c896b1e99ef is clean. No protocol/epoch files change. I did not build that merged tree, test a real remount/reboot, launch a native service supervisor, or recover a user's installed LaunchAgent. The tests use deliberately stale markers and temporary deployments on Linux.
Preserve the custom client data directory when suggesting explicit storage root recovery, including legacy managed-service config startup. Keep canonical root-ID recovery independent of client selection. Generated-by: OpenAI Codex
|
Codex follow-up: fixed the P3 legacy recovery guidance in The existing root-repair suite passes 8/8, including custom-directory guidance with spaces. Full build, typecheck, lint, format and both documented knip checks also pass. This addresses the diagnostic selector; I have not tested a real reboot or an installed LaunchAgent. |
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Reviewed 356ed7b2fb36f2360fb06e1d52a07e2aafce9ff0, focusing on the single follow-up to 7419c046 (four files, +23/-3). The original implementation is unchanged; this commit retains the client-directory selector in recovery diagnostics. I found no new confirmed P0–P2 issue.
The previous custom-client-directory P3 is fixed. cli-core.ts:323–329 derives the legacy client directory from the managed service configuration path; the service-management catch also passes its selected client directory. runtime-host-root-repair-command.ts:227–237 includes that selector for legacy recovery and omits it when selecting a canonical deployment by root ID.
I ran the actual compiled CLI against temporary deployments with stale markers and paths containing spaces. Following the printed legacy startup command now reaches interactive confirmation and repairs the marker while preserving rootId; piped confirmation is still refused. Loading the three production modules from the previous head reproduces the missing-configuration failure for the same custom-directory scenario. Default-client recovery continues to work in both versions. The management command also retains both selectors in human, JSON and framed errors; that check used a real storage identity error with the backend operation substituted, without launching a service supervisor.
Remaining P3 — canonical startup does not print the recovery hint. Starting the canonical deployment with serve --root-id … --deployment-id … --config-revision … against a stale marker returns root_identity_changed from packages/storage/src/root-authority.ts:272–278. The diagnostic helper at packages/cli/src/runtime-host-root-repair-command.ts:225 only handles root_identity_collision, so this path prints the identity error without the explicit recovery command. I reproduced this on both heads. Manually running service repair-root --managed-root-id <id> succeeds after terminal confirmation and preserves the root ID. This is a diagnostic follow-up, not a demonstrated loss of recovery or relaxation of startup checks.
Validation: fresh dependencies and patches, the normal clean root test build, CLI type checking, Biome on the four changed files and git diff --check passed. The four relevant original suites completed with 74 passed, three platform-condition skips, no failures. Account paths and deployments were isolated in temporary directories.
Exact-head CI test and Windows recovery pass. CLI package validation remains red at the production dependency audit (two moderate/two high findings), with installed-package qualification skipped. The manifests, lockfile and release audit implementation are identical to current main, so this is not a demonstrated dependency regression introduced by this PR.
The static merge with main 0d19f242336a31e8aeb300372d3d3c896b1e99ef is clean; no protocol/epoch files change. I did not build a separate merged tree, test a real remount/reboot, or recover an installed macOS LaunchAgent. These results cover Linux temporary deployments and the compiled CLI.
Include identity-change errors in the explicit repair diagnostic while retaining strict startup checks. Extend the existing canonical regression with a real stale-marker error and verify its root ID selector and original cause. Generated-by: OpenAI Codex
|
Codex follow-up to the remaining canonical-startup P3: fixed in The existing canonical recovery regression now obtains the real stale-marker error from Dependency audit follow-up: the previous head's tarball job failed its production dependency audit (two moderate/two high). I ran the exact release-script command, @Astro-Han could you investigate the shared production dependency audit failure? I have kept dependency changes out of this storage recovery fix. New-head CI is still running; I do not describe CI as all green. |
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review at d5ad485663085ed0e372b13ef3f9cf6468c66838, following 356ed7b2 (two files, +22/-1). The preceding two commits are unchanged. The remaining canonical-startup diagnostic P3 is fixed; I found no new confirmed P0–P2 issue.
packages/cli/src/runtime-host-root-repair-command.ts:225–228 now recognizes both root_identity_collision and root_identity_changed. This adds the explicit recovery hint to the canonical deployment's existing-root failure while preserving the original error as its cause and retaining the root-ID selector. The repair algorithm, deployment revalidation, locks and terminal confirmation are unchanged.
I invoked the actual compiled CLI against temporary canonical and legacy deployments with stale markers and paths containing spaces. Canonical startup still refuses the stale marker, now prints service repair-root --managed-root-id <id>, and following that command in a real pseudoterminal succeeds after confirmation while preserving rootId. Piped confirmation remains refused. Loading the previous head's production modules reproduces the missing hint; explicit manual root-ID recovery works in that control. Legacy custom-client and default-client guidance continue to work. Replacing the marker's root ID is still rejected without changing the marker, even though startup now offers the diagnostic. All eight head/control CLI scenarios passed their respective expectations.
Validation on this head:
- Fresh dependencies/patches, the normal clean root test build, CLI type checking, Biome on both changed files and
git diff --checkpassed. - The four relevant original suites completed with 74 passed, three platform-condition skips, no failures. The extended canonical regression obtains the real stale-root error and verifies the recovery selector, original cause and unchanged marker. Loading the previous production modules with those same eight command tests produces seven passes and one failure, in that extended regression.
Exact-head CI test and Windows recovery pass. CLI package validation still fails at the production dependency audit (two moderate/two high findings); installed-package qualification is skipped. The manifests, lockfile and release/audit implementation match current main, so this is not a demonstrated dependency regression introduced by this PR. Overall checks are not all green.
The static merge with main 0d19f242336a31e8aeb300372d3d3c896b1e99ef is clean, with no protocol/epoch change. I did not build a separate merged tree, test a real remount/reboot, or recover an installed macOS LaunchAgent. The execution checks use isolated account paths and temporary Linux deployments.
Merge upstream main 15f8eba, including the production dependency audit remediation from apache#6229. Keep the managed root recovery implementation unchanged. Generated-by: OpenAI Codex
|
Codex CI follow-up: upstream has repaired the production dependency audit in #6229. I merged main at Before updating, I ran the release script's exact After syncing dependencies and applying the repository's patches, the merged branch passes that same production audit with zero findings, lint, formatting, full build, typecheck, both documented knip checks, and the existing root-repair suite (8/8). The PR still changes the same eight recovery files relative to main. I have not tested a real reboot or an installed LaunchAgent. The new remote CI results remain to be checked; this does not claim that all remote checks are green. |
Summary
Add
maka runtime-host service repair-rootso a managed Runtime Host can recover after its storage-root marker becomes stale across a remount. The command displays the exact canonical path and root ID and requires interactive confirmation before rebinding that prepared marker/directory snapshot. It rechecks the managed deployment generation or legacy service configuration under the existing lifecycle locks, and preservesrootId.Normal startup retains strict
dev + inovalidation. Startup and service diagnostics point to explicit recovery; no repair permission is persisted in the service definition.Fixes #6224
Verification
Node 24.15.0 / npm 11.19.0 on macOS:
npm run lint,npm run format:check,npm run build,npm run typecheck— passed. Rebuilt the project-reference graph to clear stale declarations in the reused checkout before the final build/typecheck.npx --no-install knip --workspace apps/desktopandnpx --no-install knip --workspace packages/ui— passed.EPERMrestriction on the real account owner/cache directories; they are not reported as passing.Not run: a real macOS reboot or recovery of the reporter's installed LaunchAgent. The regression coverage uses stale markers in repository tests.
AI use
Tool(s) and scope: OpenAI Codex implemented the recovery command, diagnostics, regression coverage, and automated validation under user direction. The commit includes a
Generated-by: OpenAI Codextrailer.Checklist
Does this PR entail a change in behavior?