Repository navigation
Conversation
✅ SSOT Configuration Compliance: Passing🎉 No hardcoded values detected that have SSOT config equivalents! |
Self-correction: the marker list was too narrowThe first version enumerated four component directories ( A fleet node deploys only the components its roles carry. A node running just Now derived from the The negative case matters as much: Why this is urgent rather than tidy
So an update delivers the raise and triggers the restart that trips it, together. Only 2 of 13 unit templates set On the host where this was found, the other services are alive only because they have not restarted since the new shared code landed: There is a sequencing consequence worth stating: #14514 and #14555 repaired fleet updates, which had been failing on a node before reaching Play 2's restarts. Once those deploy, updates will start succeeding — and delivering this. Landing the gating fixes without this one would turn a single broken host into several. |
Applied Black, isort, and autoflake to match code-quality checks. Triggered by workflow auto-fix-formatting.yml.
Review — APPROVE, merging now. One follow-up needed on a docstring that still asserts the disproven premise.This is an outage fix for a regression I merged, so let me start there. #14575 (#14544) replaced a silent That answer was incomplete, and this is what it missed: a host whose systemd unit predates the template line. The template renders The lesson is one this repo already has a name for: host behaviour needs host evidence, never merge evidence. I verified those five declarations present in base and treated that as the runtime question answered. Base is not the fleet. The fix is right, and narrow
Verifying against real directory trees for all five layouts, including the two that must still raise, is the correct evidence for a resolver. The one thing to fix, in a follow-up rather than blocking this
That parenthetical is the exact false premise that caused this outage, still asserted in the file that fixes it. And the numbered resolution order lists three arms while the summary line says four — the install arm is missing from the list it belongs in. I am not blocking on a comment while a recovery path is down. But please correct it promptly: a docstring that contradicts the code is how someone later "simplifies" the install arm away on the grounds it cannot be reachable. That is not hypothetical here — the premise is what made the original resolver look complete. Filed separately, correctlyWhy the host's unit was two months stale is tracked on #14624 and kept out of this PR. Right call — it is an independent gap, it probably affects other deployed units, and it wants its own investigation rather than being absorbed into a hotfix. Merging. |
|
Approved (review above), but I cannot merge this from the API and it is an outage fix — flagging rather than sitting on it. This PR is position 1 of GitHub stack #14658 (size 2), with #14643 stacked on its branch. That closes every path I have:
The documented asynchronous endpoint is not reachable in this API build, and the stack refuses both to be merged synchronously and to be dismantled by retargeting. It is otherwise ready: 10/10 required contexts green, 12/12 shards, no base drift (6 commits since the merge base, zero overlap with the two files touched). Two ways forward, both needing someone with the UI:
I have not done either — closing another session's PR is not mine to do, and I will not Worth stating plainly: this fixes a crash-loop I caused. #14575 (#14544) replaced a silent |
|
Closing and reopening this branch as a non-stacked PR. GitHub marks stack membership permanently on the base of a stack: the sync merge endpoint, auto-merge, base retargeting and the GraphQL mutation all refuse it, and the marker survived closing the child PR (#14643) and deleting its branch. The only remaining path was the async merge API, which enforces an approving-review requirement the author cannot satisfy. Same branch, same head, same green CI — reopening restores the ordinary merge path. |
…4624) (#14640) resolve_project_root() raises rather than guessing (#14544), which is right, but a deployed install satisfied none of its arms: /opt/autobot has no top-level .env (per-component files live one level down) and no .git, so is_checkout_root is correctly False. The SLM backend crash-looped on import for ~11 hours - 5658 restarts, API unreachable, including the code-sync and self-update endpoints that would have deployed a fix. The deployment does export AUTOBOT_PROJECT_ROOT from the unit template, but that made startup contingent on every host's systemd unit being current, and the affected host had one from two months earlier. is_install_root() recognises the layout on its own terms: autobot_shared beside any autobot-* component directory. Derived from the prefix, not enumerated - an earlier revision listed four component names and would have left a node carrying only autobot-npu-worker or autobot-tts-worker still crashing. CHECKOUT_MARKERS is unchanged; its both-markers rule exists for reasons that still hold. An OSError while inspecting a candidate is now named in the raise rather than swallowed, so a permissions failure on the correct directory does not read as an unrecognised layout. Refs #14624
Thinking Path
The SLM backend crash-looped on a deployed host — uvicorn exiting 1 during import, restart counter past 530, API unreachable. That included the code-sync and self-update endpoints, so the normal recovery path was inside the thing that was down.
#14544 replacing a silent
/opt/autobotguess with a raise is right. What made it fatal is that a deployed install satisfies none of the arms:/opt/autobot/.envdoes not exist. The per-component files live one level down (autobot-backend/.env,autobot-ai-stack/.env, ...). The resolver's docstring assumes the opposite — "an install has a.envand no.git, so it matches on the.envarm" — and that premise does not hold for this layout.is_checkout_rootrequires both.gitandautobot_shared; an install has only the second. Correctly so: it is not a checkout.AUTOBOT_PROJECT_ROOT.The deployment does normally export the override. But that makes startup contingent on every host's unit being current, with nothing checking it — and this host's was not. A resolution that only works where deployed state is fresh is not a resolution.
What Changed
is_install_root()recognises a deployed install:autobot_sharedsitting beside at least one deployed component directory.Deliberately not a relaxation of
CHECKOUT_MARKERS. Their both-markers rule exists for stated reasons that still hold —autobot_sharedalone matches a package parent,.gitalone matches an unrelated repo if vendored — and loosening it would be a backdoor around exactly that. The install shape is distinct: a package's own parent does not carry component directories, and neither does anything above a checkout or worktree root.Added to the same single-pass walk, so the #13149 reasoning is untouched: a worktree still matches at its own level and never climbs to the main tree's
.env.Verification
Not run from the checkout, by instruction — CI verifies correctness, the deployment verifies runtime.
The resolver executed against real directory trees mirroring each layout:
The last two matter as much as the first: #14544's guarantee is that nothing is guessed, and this must not become a silent fallback.
test_the_pre_fix_arms_really_did_all_failpins why it raised, so a future change cannot quietly reintroduce the gap while the fix appears to still work.Not in this PR
Why the host's systemd unit was two months stale — that gap is independent of this crash and likely affects other deployed units. Tracked on #14624.
Model Used
Claude Opus 5
Refs #14624