cwcli-apps-update

Imported from karotkriss/caffeinated-whale-cli (.claude/skills/cwcli-apps-update/SKILL.md). Install upstream with npx skills add karotkriss/caffeinated-whale-cli --skill cwcli-apps-update. Copyright stays with the author.

cwcli apps command group + update.py (sharp edges)

Each note guards a real shipped bug. Keep the root-cause "why" so a later change does not silently re-break the fix. The one-line contract lives in the always-loaded AGENTS.md "Important Components" section; this skill is the deep detail.

apps command group: multi-site fan-out, JSON purity, update deprecation (2026-07-11; onto the logic core 2026-07-15)

commands/apps.py is the first-class app manager (list/install/uninstall/update), registered as a Typer sub-app in main.py (add_typer(apps_cmd.app, name="apps")). As of batch 5 (migrate-apps-core) list/install/uninstall moved to core/apps.py (list_apps/install_apps/uninstall_apps, returning typed AppsListing/AppsReport); commands/apps.py is now a THIN RENDERER over the core for all four subcommands (update moved in batch 4, see below). It adds no new dependency or cache field. The original list/install/uninstall design was spec-driven via OpenSpec (openspec/changes/add-app-management/); the core migration's own artifacts live in openspec/changes/migrate-apps-core/.

Load-bearing decisions, each guarding a real constraint - the substance is unchanged from the original design (every flag, message, exit code and --json shape is preserved byte-for-byte by the migration), only the file/function each one lives in moved:

• Multi-site by DEFAULT for install/uninstall/update. No --site = fan out to ALL sites from the canonical bench_sites.list_sites (NOT get_default_site - that single-site model was rejected by the captain); --site is repeatable and narrows (core/apps.py:_target_sites). There is deliberately NO active/disabled-site distinction - one site set, the same list_sites everything else uses. The fan-out is continue-and-report-all: run every (app, site) step, collect a per-step AppResult(app, site, action, ok), and exit non-zero if ANY failed - never a success banner over a partial failure. commands/apps.py:_report_and_exit renders the aggregate and reads the exit code off report.ok/listing.ok, NOT the envelope's result.status (the closed Status set has no ERROR member, so a partial failure is WARNING-shaped and every other verb maps WARNING to exit 0 - copying that pattern here would report success for a half-failed uninstall). Stop-on-first-failure was explicitly rejected.
• EVERY verb that changes app code on disk routes through ONE resynchronise step (core.supervision.resync_after_code_change; core.apps._resync_after_mutation is its report adapter). bench get-app changes the bench environment after the interpreter started, so a successful install-app could return ok: true while site requests failed with ModuleNotFoundError; uninstall leaves the inverse stale-code risk; checkout leaves the bench serving the branch the developer just moved off (the QUIETEST form - nothing errors at all); and update's git pull is the same fault again. Four verbs each growing their own restart is the drift the install/uninstall fix already avoided between two, so the step lives in core.supervision - NOT core.apps - because core.update is a separate module that needs it and supervision already owns discovery, per-program restart and the site-routed probe. Callers pass only the thing they alone know: which sites they changed (install/uninstall: the sites the fan-out landed on; checkout: the sites with that app installed, via core.apps._sites_with_app_installed - "checkout has no target site" was true of its ARGUMENTS and false of its EFFECT; update: sites_to_migrate; the frappe bench-wide reset: every site on the bench). WHAT IS CYCLED: every code-bearing program, not just web (supervision.code_bearing_programs: web, schedule, and every worker spelling, keyed off the normalized label's base so worker_default/worker_short need no list). This closes the residual the first fix reported: workers and the scheduler import the same app code, so after an install they cannot import the new app, after an uninstall they act on tables that are gone, and after a checkout they silently run the old branch. socketio/watch (node) and redis_* are deliberately excluded - they import no Frappe app and cycling redis drops the cache and the job queue for nothing. The honest cost, disclosed rather than hidden: a worker restart interrupts a job in flight (RQ shuts down warm on SIGTERM inside supervisord's stop window); that is the same disturbance cwcli restart causes and the one the README already directed users to after a mutation, and a worker running code that no longer exists is not a safer state to leave behind. Three properties, each guarding something specific. (1) A stopped bench stays stopped: programs are cycled only when a required process read POSITIVELY establishes a live supervisord with a serving web, and only programs already RUNNING/STARTING are touched, so a deliberately-stopped worker is not started by an install; an unreadable read is a REPORTED failure, never "assume nothing is running" (core.where's fail-honest rule) - the required=True on BOTH discovery calls is what makes that true and a test pins its presence. A manager cwcli does not own (honcho, plain bench start) is probed but never restarted, and an unhealthy site there gets a manual-restart remedy. (2) It NEVER raises: the code change has already landed, so every failure comes back in ResyncOutcome; a raise would erase the record of a change that genuinely happened and an automation caller that retried would run the mutation twice. (3) Restarting is NOT proof - "supervisord reports RUNNING" is a bound-port-shaped claim, and serving stale code is exactly the class of defect such a claim misses - so every named site must answer /api/method/ping with HTTP 200 (bounded; each probe gets only the remaining budget, or a server that accepts and never answers hangs the loop past its own timeout) before the outcome is ok. A failed restart, unreadable process state or port, probe exception, or non-200 makes AppsReport.ok/UpdateReport.ok false and the command non-zero without erasing the landed mutation. The restart's exit code is read not in (0, None) like every other container read here - None means docker recorded no code, not failure, and the site probe is the real gate. The step is DISCLOSED: each program is ANNOUNCED as it is cycled (AppsAnnounce(phase="restart-processes", app=<program>) / UpdateStepStart(phase="resync")) and reported as its own row on the human, --json and axi surfaces - the defect is that a bench was mutated underneath running processes silently, and a fix that restarts silently repeats the habit on a command that can then sit in the probe wait for up to a minute. update's POSITION is load-bearing: the resync runs inside the finally AFTER the maintenance-mode disable, because a site in maintenance answers 503 and the proof could never be obtained; it is SKIPPED on an abort, where aborted: true already says the run is half-applied and a Ctrl-C should not gain a restart plus a bounded wait. UpdateReport grew restarted_processes/unserved_sites/resync_error and _build_report folds them into ok. No extra consent, unlike scale: the caller already chose an app mutation and this cycles the programs of the ONE bench they named, whose web may otherwise stay broken; a consent gate whose refusal leaves the site down would be the defect, not a guard. The failure is real but NOT reproducible on demand - do not read a non-reproduction as proof the guard is unnecessary. It was observed twice on real v16 benches (the operator's, and a 200 -> 500 on /api/method/ping straight after axi apps install), but a later deliberate attempt - raw bench install-app into a running bench, no restart - served 200 from the stale process on a freshly provisioned v16 bench. Whether the running interpreter trips depends on what the request path imports and whether the dev server's reloader happened to pick the change up, which is exactly why the resync is UNCONDITIONAL rather than gated on detecting the bad state: a condition that only sometimes appears is one a conditional guard will sometimes miss. The real-Docker guards are test_apps_resync_e2e.py (install -> checkout -> update off ONE install, asserting each code-bearing program got a NEW pid, that socketio/watch/redis_* kept theirs, and that the site serves 200 at every step - a statement about real supervisord pids that no mock can make) and test_axi_apps_install_permitted_then_refused_on_rerun, which proves HTTP 200 BEFORE checking installed-state assertions; the ordering is the point, because checking only that no error was raised is exactly what missed this.
• An absent webserver_port is a legitimate config, NOT a resync read failure (core/supervision.py:resync_after_code_change, _port_from_process). The resync's site probe reads the bench's serving port via resolvers.resolve_assigned_ports(fill_defaults=False), which SKIPS a bench whose common_site_config.json simply omits the key, indistinguishable at that call from a config that could not be read at all. A DNS-multitenant bench deliberately removes webserver_port so Frappe routes purely by Host header, and hitting that on a real staging bench used to report apps update/install/checkout as FAILED with "the bench's assigned port could not be read", even though the mutation, maintenance-mode lifecycle, and locks completed cleanly. The fix is EVIDENCE: when the config yields no port, resync_after_code_change reads the selected live web process's bound port from its argv (_port_from_process). The supervised path captures that evidence BEFORE restarting web, since the restart replaces its PID. If a bare bench serve carries no explicit port, cwcli uses Frappe's default 8000 only when /proc proves that this bench's selected web PID owns that listener and runs under this bench path. A sibling-owned listener, missing listener, or unreadable ownership check returns the honest "serving port could not be determined safely" failure without probing port 8000. Scoped deliberately to THIS caller: core.status/core.start/core.url remain unchanged, and cwcli never writes webserver_port back into the bench's config. Guarded by tests/test_core_supervision.py::TestResyncAfterCodeChange, the realistic FakeContainer-driven TestResyncPortlessBenchRegression, tests/test_core_apps.py, and tests/e2e/test_apps_resync_e2e.py::test_apps_checkout_confirms_serving_when_webserver_port_is_absent.
• --json stdout purity. console is stdout (see utils/console.py), so in JSON mode NOTHING but the final json.dumps may touch stdout. The core emits AppsOutput events tagged by stream; the frontend's _make_renderer picks the consumption mode - buffer-and-join to stderr-on-failure when json_output, direct sys.stdout.write when not (mirrors run.py) - because that choice is rendering, not logic. A destructive apps uninstall --json without --yes REFUSES (json implies non-interactive; a confirm_or_exit prompt would fight the JSON contract, and its --yes ack prints to stdout).
• Post-mutation refresh uses cache.recache_project, NOT partial_inspect_known_benches/core.partial_refresh. The partial pass is READ-ONLY (never persists) and cannot refresh per-site installed_apps - exactly what install/uninstall change. Only a full recache (which routes through cache_project_data -> _redact_config_for_cache, so no secret is ever written) makes where/open/inspect honest. Degrades to a warning; the mutation already succeeded. This refresh is deliberately NOT in core/apps.py - see the batch 5 section below for why it stays a frontend epilogue here rather than following core/update.py's example of a mid-fan-out call.
• <app> is a name OR a git URL passed straight to bench get-app; the per-site install-app name is derived from the actual apps/ before/after diff (the real dir bench get-app created), NOT parsed from the target string - core/apps.py:derive_app_name's URL-basename-minus-.git heuristic is only a FALLBACK for when that diff is ambiguous (zero or more than one new apps/ entry, e.g. the app was already present). uninstall-app is always invoked with bench's own --yes so a non-TTY exec never hangs on bench's internal confirm (cwcli's confirm_or_exit gate is the human confirmation). uninstall only removes the app from site(s); deleting its code from the bench is out of scope (that is bench remove-app, run manually).
• A dir under apps/ is NOT proof the app is a valid install (BUG-12, core/apps.py). A bench get-app that CLONED but failed at its pip install -e step (e.g. an app declaring frappe as a hard dep tripping uv/pypika on v16) leaves the apps/<app> dir behind. The idempotent-skip keyed on directory presence (if app_name in before) then rode that leftover on EVERY retry: it skipped the fetch, emitted a false get-app ok, and fell straight into install-app, which dumped a raw ModuleNotFoundError traceback - forever, until the user hand-rm'd the dir. Two guards, both in the core so axi/GUI share them: (1) on a FAILED bench get-app, _remove_app_dir deletes only the dirs THIS fetch created (set(after) - before), never a pre-existing app dir, so a retry genuinely re-fetches; (2) on the present-dir SKIP path, _app_registered reads sites/apps.txt (bench's own registry, written only AFTER a successful pip install) - a present dir NOT listed there is refused with an actionable app.present_not_installed warning (surfaced by the human install frontend and in the TOON/--json warnings) instead of a traceback, and is NOT installed. apps.txt UNREADABLE returns None -> proceed (fail-honest: never refuse a legit pre-warmed base on an unknown). Guarded by tests/test_core_apps.py (cleanup + pre-existing-dir-survives + unregistered-refused + registered-still-installs + unreadable-proceeds) and tests/test_apps.py (the human warning, not a traceback).

apps's list/install/uninstall on the logic core: what a reviewer would flag as a mistake and isn't (batch 5, 2026-07-15)

core/apps.py owns resolution (container + bench only - no default-site, no site-name validation, no bench-dir probe: apps uses none of those today, and reaching for them because they exist would ADD failures it does not have), the container I/O, and the fan-outs; it RETURNS AppsListing/AppsReport and prints nothing. Five decisions guard real behaviour, and each looks like an inconsistency if you read only the diff:

• The recache stays a FRONTEND epilogue (commands/apps.py:_refresh_cache, gated on any(r.ok for r in report.results)), NOT hoisted into the core the way core/update.py calls cache.recache_project mid-fan-out. That call exists in update only because its recache runs MID-fan-out and cannot be moved out; install/uninstall's recache is a post-mutation step gated on a condition already in the returned report, so an epilogue costs nothing. (Before migrate-inspect-core, update's mid-fan-out call was also the one core-module-imports-CLI-layer site in the codebase, since recache_project invoked the inspect Typer command; since that batch recache_project's body calls core.inspect instead, so this is now an ordinary core-to-core call, not a layering exception.)
• uninstall's fused --yes ("skip the destructive confirmation AND auto-start containers") keeps that exact CLI meaning - a migration does not get to change the contract - but core.uninstall_apps takes auto_start and consent as SEPARATE parameters and returns NEEDS_CHOICE/confirm_uninstall for the destructive gate. The fusion is one frontend's UX choice; a core that must also serve axi and a future GUI must not inherit it. commands/apps.py:_uninstall closes over consent and is called TWICE on the interactive path - once with yes, and again with True after confirm_or_exit returns - which is the re-invoke shape every NEEDS_CHOICE frontend uses (--yes short-circuits it on the first call; tests/test_apps_characterization.py's interactive-confirm test is what proves the second call actually runs the uninstall rather than just passing).
• AppsAnnounce and AppsCommand are two events, not one, because the pre-migration code interleaves them: it announces "Fetching x...", THEN reads apps/ (echoing that read under --verbose), THEN echoes the get-app command itself. Fusing the two into one event would reorder --verbose stderr.
• The warnings field carries notes an agent should act on; the command trace rides the event channel. The original batch-5 proposal said list_apps needs no callback ("nothing to report progress about") - right about progress, wrong about the trace: routing $ ls -1 apps through warnings would put a debug echo inside axi apps list's structured TOON document. list_apps takes on_event like the other two verbs (and like core.update); axi passes nothing and the trace is discarded for free (_noop).
• cwcli axi apps list ships; axi apps uninstall deliberately does NOT (captain-locked 2026-07-15). bench uninstall-app drops the app's tables, and letting an agent do that is a product decision on its own evidence, not a side effect of moving code onto the core. tests/test_axi_apps_list.py::test_uninstall_is_deliberately_not_a_verb asserts the absence so it cannot be misread as an oversight. Decision 4 above made the eventual verb cheap: the core already exposes destructive consent separately from auto-start. axi apps install SHIPPED 2026-07-20, reversing only the half of this deferral it never actually covered. Its user-facing contract lives in README.md, while the safety invariant and regression references live in the AGENTS.md apps entry. uninstall stays deferred under the unchanged rationale.
• cwcli axi apps checkout SHIPPED 2026-07-20, reversing its own absence assertion - and HOW it was reversed is the durable lesson. The old assertion sat beside the install/uninstall one and read "apps checkout mutates the in-instance checkout, so LIKE install/uninstall it is a HUMAN verb only". That "like" is where the reasoning slipped: install/uninstall were held for the ONE threat named above (dropping an app's tables), and checkout was grouped with them for merely being a mutation, so a rationale it was never covered by silently became a block. Two things settled it, both verified rather than asserted: (1) core.checkout_app runs git fetch + git checkout -B inside apps/<app> - no bench command, no site, no SQL, no table - and migrate-apps-core/tasks.md §7.2 had already pre-authorized a different answer for non-deleting mutations; (2) "the agent surface is read-only" is not a rule this repo follows - most live axi verbs mutate (see the AGENTS.md apps checkout entry for the current count and enumeration), axi init provisions an entire instance and axi apps update runs schema migrations across live sites, so no read-only principle can be what withholds a git fetch. Generalize this, do not just remember the outcome: when you find an absence pinned by a test, read the rationale the test INHERITED and check it actually reaches the thing it is blocking. axi init is the same story (deferred, pinned, then shipped on its own evidence). The verb itself shipped as a thin renderer with zero core/ delta; its guards and the deferred resulting-commit read are in the AGENTS.md entry and openspec/changes/add-axi-apps-checkout-verb/.
• The dirty-tree guard is cwcli's own, and how it got there is the lesson. axi apps checkout originally had NO dirty-tree pre-check by design, leaning on git's refusal of a checkout that would OVERWRITE a modified file, and the docs described that as "a dirty tree fails". CI on PR #129 proved the claim false: a dirty file the target ref does not touch has nothing to conflict with, so it rode through at exit 0. The obvious fix was to soften the docs; the captain ruled the opposite - fulfil the promise rather than scrub the doc, because a false SAFETY claim is acted on (a user leaves uncommitted work in an app dir believing it is protected), and these checkouts live in a SHARED dev instance where the work carried across may not be the runner's. core.apps._refuse_dirty_tree now refuses CONFLICT/app.dirty_tree before any fetch. Generalize this: when a doc and the code disagree about a SAFETY guarantee, the doc is not automatically the thing that is wrong. What counts as dirty is stated, not implied, and it was itself revised once - the first cut passed --untracked-files=no, reasoning that the guard should cover exactly what --reset destroys; the captain reversed that, so it is now plain git status --porcelain and untracked files refuse too, because a new module not yet git added is uncommitted work and the tool must not decide it is worthless. That widening is only safe because .gitignore keeps build residue out of git status entirely - a real-bench E2E asserts a freshly provisioned app is genuinely clean rather than assuming it. Keep the honest corollary attached: --reset does NOT delete untracked files (cwcli never runs git clean). A definition left vague here would recreate the very ambiguity the fix removed. Full rationale in the AGENTS.md apps checkout entry.
• _checkout_narrate forwards raw git output to stderr; _init_narrate deliberately does not forward raw bench output. Do not "unify" them. They answer different questions. Init's InitOutput is thousands of lines of bench-build noise an agent will not parse, and cwcli logs serves it afterwards, so narrating it is pure cost. A checkout runs two or three short git commands, a git step is NOT a supervised process so nothing is logged anywhere afterwards, and the entire reason a step failed lives in those bytes - git's own "couldn't find remote ref" or its auth failure is what tells an agent whether to fix the ref or the credentials. Drop it and those reach the agent as a bare ok: false with no cause. (The dirty-tree case no longer depends on this: it is refused before the fetch as its own typed error carrying the paths and --reset. The narration still matters for every failure that IS a git step.) Both narrators write only to stderr, so stdout stays one TOON document either way.

A standing recon item was judged and DECLINED on the code, not assumed: unifying core/update.py's _sites_with_app with core/apps.py's _installed_apps/list_apps installed-apps read. They are INVERSE questions (app->sites vs site->apps) with deliberately opposite failure semantics - _sites_with_app drops an unreadable site because it feeds a migration filter, while list_apps must distinguish "read failed" (None) from "no apps" ([]) or its exit code lies - reading different data shapes (the cache's raw bench list-apps lines vs a live first-token parse). The one genuinely shared line (the first-token parse) stays duplicated on purpose.

Judging that surfaced a real defect, REPORTED and deliberately kept dead: core/update.py:291's cache branch does exact list-membership (app in installed_apps) against get_cached_project_data's RAW bench list-apps lines, which on a real v16 bench are frappe 16.26.3 - so the cache branch is dead and _sites_with_app always falls through to its live query. Fail-SAFE (correct answer, slower); it survived because the tests seed the cache with bare names and take the cache branch, while the live fallback production always runs was 0% covered until TestSitesWithAppLiveFallback (tests/test_core_update.py) exercised it directly with realistic versioned cached data. Fixing the branch itself would still change update's behaviour on the input to a migration fan-out, so the captain chose to keep it dead-but-now-covered rather than fix it (openspec/changes/migrate-apps-core/tasks.md §8). core/update.py itself remains untouched.

Regression coverage for the core migration: tests/test_apps_characterization.py (the green-before net, committed before anything moved so refactor-under-green is auditable - commands/apps.py 91.28% -> 99.49%), tests/test_core_apps.py (every branch only axi/a future GUI can reach: confirm_start, select_bench, the no-cache default, confirm_uninstall, plus that the core prints nothing and returns plain serializable data), tests/test_axi_apps_list.py (TOON rendering, exit 0/1/2, the null-vs-empty-list distinction on a failed site read, and the install/uninstall non-registration assertion).

cwcli update deprecation + frappe special-case (folded into commands/update.py):

• cwcli update is now a DEPRECATED alias that prints a notice and delegates to the shared run_app_update, which both apps update and update call - one implementation, no drift. run_app_update validates >=1 app then calls _update_project.
• _update_project gained sites_filter (the repeatable --site narrowing, applied once to all_affected_sites via _apply_site_filter in the shared discovery pass) and an early frappe special-case: if any named app is frappe (case-insensitive), it runs _run_frappe_update_reset (bench update --reset, whole-bench) and returns - the per-app git pull loop is skipped, and --site does not apply (bench update is bench-wide).
• --site matching zero affected sites refuses, non-zero. _fail_if_site_filter_matched_nothing distinguishes "genuinely nothing to migrate" (no site has the app installed at all - exits 0, unchanged) from "--site named a site the app isn't actually on" (a typo/mismatch): when sites_filter is non-empty AND the unfiltered affected-site set is non-empty AND filtering narrows it to empty, the command errors naming both the requested and the actually-affected sites and exits 1, rather than silently completing having migrated nothing. Applies to both apps update and the deprecated update alias (shared _update_project).

update.py control flow + maintenance-mode safety (u4/b8, 2026-07-11/12)

_update_project used to mis-nest the verbose vs non-verbose logic: only the header was gated on if verbose, the "verbose" body ran ALWAYS, and the whole non-verbose progress-bar block was the else: of the trailing if all_affected_sites: (clear-locks) - so it only ran when the affected set was EMPTY, and re-did git pull + discovery there (double pull, dead progress UI). Now the pull + discovery happen in ONE shared pass (_pull_apps -> _recache_after_pull -> _discover_affected_sites), then a single top-level if verbose:/else: keyed on --verbose (NOT all_affected_sites) picks the migration presentation (_run_migrations_verbose streams; _run_migrations_quiet uses console.status). The old rich Progress/Live/Spinner machinery and its throwaway pre-count discovery pass are gone; non-verbose now uses the same console.status spinner pattern the rest of the file already uses.

Maintenance-mode safety (all in _update_project + helpers):

• _enable_maintenance turns maintenance ON per site, recording each success in maintenance_sites AS it is enabled - so a mid-loop exec crash still lets the finally disable exactly the sites that were actually enabled (the old bulk-then-record could lose already-enabled sites on an exception).
• sites_to_migrate is sorted(maintenance_sites) (or sorted(all_affected_sites) when --skip-maintenance), and migration, the optional cache/website-cache clears, and lock-clearing all run over that same set - a site that could not enter maintenance is never migrated, cache/lock-cleared, or otherwise touched as if the update had run there.
• That skip is honestly surfaced, not silent: failed_maintenance_enable (all_affected_sites - maintenance_sites, only computed when maintenance isn't skipped) is reported in the "Update completed with errors" summary ("could not enter maintenance mode - not migrated") and folds into has_errors -> non-zero exit, same as every other partial-failure phase.
• _disable_maintenance (called in finally) turns maintenance OFF per site, checks each result, warns on a failed disable, and records stuck sites in failed_maintenance_disable -> which feeds has_errors -> non-zero exit (a stuck site is never left silent).
• Every shell interpolation in update.py is shlex.quoted (site/app/path); docker-py shlex.splits string commands, so quoting yields valid tokens. _dir_exists passes its test -d check in LIST form (["sh", "-c", script]) so docker-py execs it directly rather than re-shlex.splitting a sh -c "..." string - that keeps shlex.quote(path) robust even for a path containing a single quote (a sh -c "..." string wrapper would break the outer quoting on such input).

--force also clears a git "dubious ownership" refusal at its ROOT, not by silencing the check (v3.1.2, fm/cwcli-apps-update-force-ownership). On a shared-mode/migrated instance an app source (often symlinked out of the bench to /workspace/.hdsrc/<app>) is owned by the pre-shared uid, so git run as frappe refuses it - and crucially BOTH git pull AND the git status the conflict/dirty-tree path relies on fail, so plain --force (which needs a readable git status) was powerless. So _update_apps calls core.docker.reown_app_repo_to_frappe(container, app_path) under --force BEFORE the pull: it resolves app_path through any symlink (so the real repo is chowned, not the link) and chown -Rs it to the frappe user's CURRENT uid (whoever git will run as, in any mode) ONLY when a different uid owns it - a stat-gated cheap no-op otherwise. A re-own emits an app.force_reown warning; a failed one emits app.force_reown_failed and the pull still runs (degrades to the pull's own error, never raises). Chosen over git -c safe.directory=<p> because a stray safe.directory hides a real ownership problem the captain wants fixed. This is the per-op twin of the align-time self-heal (AGENTS.md REGRESSION GUARD entry / _external_app_source_dirs): a cwcli start/restart re-owns the app repos so a PLAIN apps update works, while --force fixes it inline with no prior restart. Pinned by tests/test_core_docker.py (reown_app_repo_to_frappe unit cases) + tests/test_core_update.py::TestForceReownsADubiousOwnedAppRepo (wiring) + real-git E2E tests/e2e/test_shared_workspace_reown_e2e.py::test_force_reown_clears_dubious_ownership_on_a_git_repo.

Regression coverage: tests/test_apps.py (a FakeFrappeContainer recording every exec + streaming via a fake client.api; covers both modes, multi-site fan-out aggregation, git-URL derivation, the destructive/non-TTY refusals, cache-refresh, the frappe reset branch, and the deprecated-update warn+delegate). The u4/b8 control-flow/maintenance-mode fix above is covered by test_update_pulls_and_discovers_once and test_update_empty_affected_set_performs_neither_second_pass (one pull + one discovery pass, both presentations), test_update_migrate_skipped_for_site_not_in_maintenance (failed-enable skip + honesty: non-zero exit, summary line, no cache/lock-clear for the skipped site), test_update_stuck_site_warns_and_exits_nonzero (failed disable), and test_update_shell_interpolations_are_shlex_quoted. The apps commands are @handle_docker_errors-decorated, so tests patch docker_utils.shutil.which + docker_utils.docker.from_env (in the wired fixture) and pass every Typer param explicitly.

The summary MUST be reported from inside the finally (r2, 2026-07-15)

Reporting used to live AFTER the try/finally, so any raise jumped clean over it. Batch 3 (PR #80) re-pointed _stream_command onto core.exec_stream, which RAISES CwcliError when the exit code is unknowable (a dropped connection: exec_inspect -> {"ExitCode": None, "Running": True}); _stream_command turns that into typer.Exit(1), which unwound past the seven-way aggregation. It disclosed the hang-to-typed-error change but not this consequence: the finally held (maintenance always disabled-attempted - the invariant that matters most), but the summary never ran, so a genuinely stuck site lost its actionable bench --site X set-maintenance-mode off remediation and kept only a bare inline warn; failures accumulated before the raise were dropped; the abandoned fan-out went unreported. Net still an improvement over the unbounded poll it replaced (an infinite hang is strictly worse than a truncated report with an honest exit code) - a NEW gap on a path that previously could not be reached, not a regression against anything a user had.

The fix and the two things that pin its shape - do not "tidy" either back:

• Report from within the finally, AFTER _disable_maintenance - not from an except. The obvious alternative (catch, summarise, re-raise) is WRONG here: Python runs the except clause BEFORE the finally, and the remediation data (failed_maintenance_disable) is produced BY _disable_maintenance in that finally - so an except-based report prints while the list is still empty and omits the very line the fix exists to restore. Making it work needs nested trys and a duplicated report call. The finally is the only placement where the data already exists and every raise is covered structurally, without enumerating exception types.
• _report_summary is PRINT-ONLY; the caller owns the exit. Raising from a finally REPLACES the in-flight exception - it would swallow the real error and any traceback from an unexpected one. So it returns has_errors and the trailing if has_errors: raise typer.Exit(1) sits after the try (unreachable when aborted: that exception propagates out of the finally and already exits non-zero, carrying its own message).
• aborted is gated on bool(sites_to_migrate) (hence its [] init before the try): an abort is only worth reporting once there was a fan-out to abandon. Raising earlier (a --site typo, a failed pull) leaves nothing half-done, so the raise's own error stands alone instead of being dressed up as an interrupted update - while any failure already accumulated still reports. aborted also suppresses the success banner (no "Successfully updated N app(s)" over an update that stopped halfway) and is set in an except BaseException: that records it, ENDS the orphaned in-container migrate (bench_ops.end_migrate_on_interrupt, keyed on sites_to_migrate), and re-raises untouched - BaseException, not Exception, so a Ctrl-C is handled the same way as a stream loss: both end the orphan first, and the finally then clears maintenance LAST with a settle/re-check (clear_maintenance(recheck=aborted); under --skip-maintenance, _restore_pre_maintenance_on_abort restores each site's pre-migrate state instead), so neither strands the site at 503. See the "interrupt cleanup: end the orphaned migrate, THEN clear maintenance LAST" entry in AGENTS.md.

Regression coverage: test_update_stream_loss_mid_fanout_still_reports_stuck_site_remediation (parametrized over verbose) drives the exact dropped-connection shape into a 2-site fan-out with the disable failing too, and asserts the summary + BOTH remediation lines survive; test_update_site_filter_refusal_is_not_reported_as_an_interrupted_update pins the sites_to_migrate gate. Both fail against unfixed source - the pre-existing green did not cover this (tests/test_apps.py covered 69.95% of update.py and only 2 of the 7 aggregation branches), which is exactly how it shipped. Note rich hard-wraps to console width, so assertions on the long remediation line must collapse whitespace first.

Where this lives now (m5): the state machine moved to core/update.py and the report is a RETURNED UpdateReport, so an unwinding exception can no longer skip it at all - the placement discipline above became structural rather than a convention. The two pins still hold in their new form: _report_summary (now in commands/update.py, taking the report) is still PRINT-ONLY with the caller owning the exit, and aborted is still gated on bool(sites_to_migrate). The except BaseException: that records aborted, ends the orphaned migrate, and re-raises untouched is still BaseException, not Exception, for the same Ctrl-C reason. See the m5 section below for what a returned report could NOT do, and what UpdateAborted exists to fix.

update on the logic core: three decisions that must not be undone (m5, 2026-07-15)

The state machine now lives in core/update.py (core.update(...) -> Result[UpdateReport]); commands/update.py is a renderer over it, and both cwcli apps update and the deprecated cwcli update are thin frontends over that ONE implementation. cwcli apps update --json and cwcli axi apps update emit the same report. The full argument is openspec/changes/migrate-update-core/.

• core.update is NOT a generator, deliberately, and against locked decision 4's letter. Decision 4 says "streaming operations return typed event iterators"; a later agent reading it alone WILL reach for update_stream(plan) -> Iterator[UpdateEvent] and reopen a data-safety hole with an elegant-looking refactor. Do not. A try/finally inside a generator does NOT run when a consumer breaks early while holding a reference, or when the generator lands in a reference cycle - cleanup is deferred to the garbage collector. Probed, five consumer shapes: exhaust -> ran; break (refcount drops) -> ran; break holding a reference -> DID NOT RUN; reference cycle -> DID NOT RUN; contextlib.closing -> ran. A GUI pumping events from an event loop holds the iterator on self and lives in a widget cycle - exactly cases 3 and 4 - so a window closed mid-update strands a site in maintenance until GC happens to run, and enabling that GUI is the rework's own goal. The reading: decision 4 governs genuine STREAMING ops cwcli itself streams - raw exec output, which core.exec_stream is and remains (logs was once the other expected case; it settled as a deliberate NON-consumer instead - core/logs.py is a plan-only resolve and its --follow bytes never enter the Python process, so decision 4 does not govern it); update is a state machine that emits progress and whose terminal value is a report. contextlib.closing was rejected because it relocates a safety-critical guarantee into every frontend's hands, forever, including frontends not yet written. run_stream has the same exposure and is fine: an abandoned run_stream leaks a socket, not a stuck site.
• UpdateAborted is why the callback exists at all. A RETURNED report cannot survive an unwinding KeyboardInterrupt - there is no return. Since a Ctrl-C mid-update must still surface a stuck site's remediation (the property PR #81 restored, above), the finally builds the report either way and hands it to on_event(UpdateAborted(report=...)) when unwinding. Delete that and Ctrl-C silently loses the remediation again.
• A lost stream is UNKNOWN, never a failure, and the fan-out CONTINUES. Before m5, a migration that returned non-zero was recorded and the fan-out continued, while a migration whose stream was LOST aborted it: the same real-world event, two behaviours, decided by whether Docker happened to record an exit code. They are one behaviour now. But unknown is NOT folded into failed_*, and must never be: a lost stream means the exit code is unknowable and the command may still be running, so an agent branching on axi apps update will retry a "failure" and retrying a live migration does real harm. unknown_apps/unknown_migrations/unknown_builds sit alongside their failed_* counterparts (three, not seven: only pull/migrate/build stream inside the state machine; the frappe reset rides failed_apps/unknown_apps as the app frappe). Every exec-stream CwcliError is unknown, including exec.start_failed - it arguably means "it never ran", but that is a confident claim derived from an API call whose own outcome is uncertain, and the safe direction for a retry decision is unknown.
• The exit code reads report.ok, NOT result.status. Every other axi verb does 0 if result.status in (OK, WARNING) else 1. Copying that here ships a fail-open: a partial update failure is a WARNING-shaped envelope carrying ok=False, so it would report success for an update that half failed - on the exact surface this batch exists to make honest. The closed Status set has no ERROR member by design (hard failures raise; update's partial failures must not, because reporting them all IS the job), so the aggregate rides the DTO.
• cwcli axi apps update has no --yes, and must not grow one. A first cut threaded auto_start=yes straight into core.update with no start prologue: resolvers.resolve_container_state(auto_start=True, ...) only REPORTS ContainerState(start_requested=True) back to a caller that is supposed to perform the real, UI-coupled .start() itself (see its docstring) - core.update never did, so the exec against a still-stopped container raised a raw docker.errors.APIError instead of a clean TOON error. commands/update.py:run_app_update is safe ONLY because its ensure_containers_running(auto_start=yes) prologue actually starts the container BEFORE calling core.update; the agent verb has no such prologue and must not gain one (starting stays UI-coupled, off the core). The fix is not "add a start": it is dropping --yes entirely, so auto_start defaults to False, resolve_container_state returns NEEDS_CHOICE/confirm_start, and axi_apps_update renders that through the SAME emit_axi_choice_as_usage_error path every other stopped-project fork uses - a TOON usage error (exit 2) whose help: line names cwcli start, exactly like axi backup/axi unlock. An agent composes cwcli axi start then this verb. Do not re-add --yes/auto_start to the axi verb without also adding a real start prologue - and even then, that would reopen the "starting is UI-coupled" boundary this core migration exists to keep.
• _run_frappe_update_reset hardcoded verbose=True and wrote bench output to stdout whatever the caller asked - which is why apps update was the only apps subcommand with no --json: a structured surface cannot call it. Fixed; non-verbose now runs the reset under a spinner, mirroring _pull_apps. The three pre-existing frappe tests all pass verbose=True and the base fake returns "" for the reset, so with no bytes to stream, streaming and not streaming looked identical - that is why nothing caught it. Any new test of this property needs a fake that actually emits output. Its recache-before-exit-code-check is PRESERVED deliberately: a failed reset still recaches, then reports failure (a partially-applied reset genuinely changes the cache). Do not "fix" it without an argument of its own.
• update keeps its OWN two-directory bench probe (apps/ AND sites/) in core/update.py. resolvers.require_bench_dir probes sites/ ONLY: reusing it silently drops the apps/ check, and widening it changes backup/unlock. Reported as a near-miss, not resolved by bending the primitive. core.update also deliberately does NOT call validate_site_name/resolve_default_site - update uses neither, and adding them would add failures it does not have.
• LAYERING SIGNAL, SETTLED by migrate-inspect-core (batch 7). core/update.py was the first core module to call utils.cache.recache_project, which (until that batch) lazily imported the inspect COMMAND - a core module reaching into the CLI layer at runtime, the one site of its kind. It could not be hoisted to the frontend (it runs mid-fan-out, between the pull and the discovery that depends on it), so the fix needed inspect's own migration, not a local workaround. Since inspect moved onto the core, recache_project's body calls core.inspect(refresh="full", offer_choice=False) directly; core/update.py's call into it is now an ordinary core-to-core call, and no module under core/ imports commands/ at runtime any more. utils/auto_inspect.py's daemon fallback was re-pointed the same way in the same batch (it used to import and call the inspect Typer command directly).
• The two frontends are NOT interface-identical. apps update <proj> erpnext takes apps POSITIONALLY; the deprecated cwcli update <proj> --app erpnext takes them as a repeatable --app/-a OPTION. Unifying the signatures breaks every existing cwcli update ... --app x invocation. Both are pinned by an E2E leg against the real binary.

Regression coverage for the above: tests/test_core_update.py (the envelope, the probe, unknown-vs-failed, the maintenance lifecycle, the finally under a raise, UpdateAborted, the frappe fork, the callback), tests/test_axi_apps_update.py (both structured surfaces, stdout purity, exit 0/1/2), tests/test_update_characterization.py (the seven-way aggregation and every flag, written BEFORE the migration and passing unchanged either side of it), and tests/e2e/test_apps_update_e2e.py (both modes on a real instance; bench is shimmed for the output-purity legs, because an empty bench output cannot show a purity break).