fix(onboarding, deploy): checklist updates without a reload; 1-Click first boot serves again

[obasilakis]

Found while render-checking #2854 on a local stack with the enterprise submodule (release work-order M4).

The bug

stores/onboarding.js::fetchChecklist returns early once loaded is set, and ActivationChecklist called it without force. So the checklist was read once per page load. Create an agent from the dashboard and it still says 0/4 until a browser refresh; the server already had it right (refresh shows 1/4).

This predates #2854 — the inline placement from #2278 had the same fetch-once behaviour.

The fix

In ActivationChecklist.vue only:

• Re-read on every mount. Chat, schedules and channels are reached on other pages, so returning to the dashboard is when those milestones land. The Dashboard isn't in the KeepAlive include list, so the component remounts on return.
• Re-read when agentsStore.agents.length changes. Agents are created and deleted from the dashboard itself, and the WS agent_created / agent_deleted handlers already keep that list current. Deletion matters too: milestones untick honestly.

Completion stays server-derived; nothing is ticked optimistically.

Verification

Playwright against the local stack (enterprise submodule mounted), checklist response patched to control completion:

Check	Before	After
Initial render 0/4	PASS	PASS
Agent added on dashboard → 1/4 without reload	FAIL (1 GET total)	PASS (2nd GET fires)
Leave dashboard via router, return → 2/4 without reload	FAIL	PASS
Checklist GETs on initial load	1	1

npx vitest run tests/unit/firstRunSteps.spec.js tests/unit/rawColorRatchet.spec.js tests/unit/loadingGateRatchet.spec.js — 76 passed.

No new unit test: vitest runs environment: node without a mount harness (the ent#392 constraint), so the specs can only assert source structure, and the behaviour above is what matters.

Not in this PR

The checklist's "Create an agent" action routes to /, which is the dashboard it is shown on, so on the dashboard that button does nothing. The route comes from the server side, so it's a separate fix.

Also in this PR: the 1-Click first boot never served (Caddyfile permissions)

A droplet created from the v0.9.5-rc4 Marketplace snapshot never answered on 443. First boot stopped at:

❌ --provision: could not write the Caddy configuration.
Error: reading config from file: open /etc/caddy/Caddyfile: permission denied

firstboot.sh runs start.sh under umask 077. Since #2773, provision_caddyfile renders to Caddyfile.new and moves it over the live file, so the new file came out 0600 root. The Caddy unit runs as User=caddy, so the restart failed and provisioning stopped before Trinity started. The old in-place overwrite kept the package's 0644, which is why rc3 booted.

• 4f93d4e37 runs chmod 0644 before the move (the file holds no secrets). New test test_the_installed_caddyfile_is_readable_by_the_caddy_user_under_a_strict_umask runs the real function under umask 077: red before the fix (0o600), green after.
• The rc5 bump that briefly rode here (ac1c2a2ad) is reverted in ea0649aad and moved to its own PR, chore: set version to 0.9.5-rc5 #2866, per review finding I1.

Verified on a real droplet: a snapshot built from rc4 with this start.sh came up on HTTPS with a valid Let's Encrypt IP certificate 184s after create. /etc/caddy/Caddyfile was 644, Caddy active, first boot complete, all containers healthy. pytest tests/unit/test_2380_*.py tests/unit/test_2692_private_network_access.py tests/unit/test_2281_*.py: 200 passed.

Refs abilityai/trinity-enterprise#238

🤖 Generated with Claude Code

[dolho]

/review Report

Branch: fix/activation-checklist-refetch → dev (merge-base 3a47f74)
Files Changed: 1 (+10/−2) — components/onboarding/ActivationChecklist.vue
Scope: CLEAN — intent: the checklist re-reads without a reload (M4 render-check finding, refs ent#238). Delivered: fetchChecklist(true) on mount + a watch on agentsStore.agents.length. No store or backend change.
Plan Completion: no tracked plan (drive-by fix); the PR's own two claims are both in the diff — mount refetch ActivationChecklist.vue:151, fleet-size watch :153.

Critical Findings (block merge)

None.

Informational Findings (review required)

[I1] Product bar / perf (4.15, 4.11): on OSS and unentitled builds the fix turns one swallowed 404/403 per session into one per dashboard mount, per sidebar expand and per agent create/delete (Confidence: 8/10)
File: src/frontend/src/components/onboarding/ActivationChecklist.vue:151-153; store stores/onboarding.js:56-57
Evidence: store.fetchChecklist(true) bypasses the store's only guard — if (this.loaded && !force) return — and _apply is never reached on 404/403, so available stays false but loaded is true, and every forced call re-issues GET /api/enterprise/onboarding/checklist. The component is mounted by SystemViewsSidebar.vue:111 under v-if="!isCollapsed", so each sidebar expand is a remount too; and agentsStore.agents.length changes on every WS agent_created/agent_deleted (utils/websocket.js:127-132).
Issue: the checklist is deliberately "silent" on OSS (available=false, render nothing) — the store comment says so — but silent now costs a request + a red 404 line in the browser console on every one of those events, for a component that will never render. Not a correctness bug; it is the OSS bundle paying for an enterprise surface it doesn't have, which the store's design note explicitly tried to avoid.
Suggestion: keep the fix, add the one guard that preserves the OSS behaviour — the entitlement verdict doesn't change mid-session, so a prior 404/403 can be terminal for the session: in fetchChecklist, if (this.loaded && (!force || !this.available)) return (or a separate unavailable flag so a genuine 5xx — also available=false today — can still be retried). One line in the store, covered by extending onboardingChecklistStore.spec.js's existing "fetches once unless forced" case with "…and never re-forces after a 404".

[I2] Concurrency (4.2): overlapping forced fetches have no ordering; the last response to arrive wins (Confidence: 5/10 → appendix)
File: ActivationChecklist.vue:153, stores/onboarding.js:56-72
Evidence: the watch fires per length change; a burst (deploy a 3-agent system manifest → three agent_created events) issues three GETs with no sequence check; _apply applies whichever resolves last. Same endpoint, monotone data, next event re-syncs — so a stale intermediate state is at worst momentary. Noting it because the store has a loading flag it doesn't use for coalescing; a if (this.loading) return would collapse a burst to one in-flight read.

[I3] Test gap (4.9): the behaviour is verified by hand only; a structure spec is possible and is this repo's precedent (Confidence: 6/10)
Files: src/frontend/tests/unit/onboardingChecklistStore.spec.js (exists, node env), firstRunFrontDesk.spec.js
Issue: the PR says vitest can't mount, which is true, but the sibling specs assert source structure in exactly this situation (the ent#392 shape the PR cites). Two cheap pins: the component source contains fetchChecklist(true) and a watch(() => agentsStore.agents.length — so a future "optimise away the force" doesn't silently reintroduce the reload-only checklist. The store-level force path is already covered ("fetches once unless forced").

[I4] Behaviour note (4.5): the length trigger misses same-size changes (Confidence: 4/10 → appendix)
A rename, or a delete immediately followed by a create, leaves agents.length unchanged and fires no refetch; the next mount catches it. Deliberate simplicity — mentioned so it's a known edge, not a surprise.

Clean Categories

• 4.1 / 4.3 / 4.4 — no backend, no auth surface, no credentials; the read is the existing entitlement-gated endpoint.
• 4.10 frontend — no v-html, no new user content; the watcher is created in <script setup> scope so it stops on unmount (no leak on the remount path).
• 4.7 — watch import used; no dead code.
• 4.12 enums — none.
• 4.13 docs — architecture.md's ent#238 paragraph says "derivation-only on the client… renders nothing when the surface is absent" — still true; no stale section.
• Design invariant — completion stays server-derived; nothing optimistic. Verified: the only state writes are in _apply from the response.

Summary

• Critical: 0 — none found
• Informational: 2 (+2 appendix) — I1 is the one I'd address before merge (one-line store guard keeps OSS silent and cheap); I3 is a cheap pin
• Scope: clean

Review complete. Next: /validate-pr 2862 for the docs/process pass.

[obasilakis]

Thanks @dolho. Your review was posted before 4f93d4e37 (Caddyfile permission fix) and ac1c2a2ad (rc5 bump) landed on this branch, so it covers the checklist commit only. The two deploy commits still need eyes.

I1: fixed in e153f6c08. I didn't use the one-line guard as suggested: !this.available is also true after a genuine 5xx, so it would stop retries after a real failure too. A 404/403 now sets a separate absent flag, and fetchChecklist returns early on it, forced or not. A 5xx stays retryable. Store tests: never re-forces after a 404 / …403, which were red before the change, and still retries a forced read after a real failure.

I2: not changing. if (this.loading) return drops the NEWER request. If the in-flight GET was issued before the agent create committed, it returns the old count and the read that would have fixed it never happens, so the checklist stays stale until the next event. Today, whichever response arrives last is applied: at worst stale for a moment on a monotone read, and the next event corrects it. Collapsing a burst correctly needs a "re-read after the current one" flag, which isn't worth it for three events.

I3: not adding the component-source pin. The #2773 merge-train review rejected exactly this shape ("a toContain pin over the SFC text would only restate the source"). The behaviour that matters, forcing without re-asking an absent surface, is now covered at the store.

I4: agreed, a known edge. No milestone depends on the agent name, and the next mount catches it.

Frontend unit suite: 3078 passed, 1 failed. The failure is portalAgentsAtCentre (date-locale), which fails the same way with this change stashed.

[dolho]

/review Report — re-review at e153f6c (4 commits)

Branch: fix/activation-checklist-refetch → dev (merge-base 3a47f74, unchanged)
Files Changed: 7 (+82/−6)
Scope: DRIFT DETECTED (declared) — intent: the checklist re-reads without a reload. Delivered: that (aa9028a), the store guard from the first review (e153f6c), plus two release-work items with no relation to the checklist: the 1-Click Caddyfile permission fix (4f93d4e) and the 0.9.5-rc5 version bump (ac1c2a2). The PR body declares both ("Also in this PR"), so this is a bundling call, not a surprise — but the rc5 bump is a release-cut action riding a UI fix; see I1.
Plan Completion: first-review items — I1 (store guard) DONE stores/onboarding.js:56-59 + spec onboardingChecklistStore.spec.js:113-135; I3 (structure pin on the component) NOT DONE, low impact; I2/I4 appendix, unchanged.

Critical Findings (block merge)

None.

Informational Findings (review required)

[I1] Scope (2.2): the rc5 bump couples this merge to the release cut (Confidence: 8/10)
Files: VERSION, scripts/deploy/trinity-do-create.sh:24
Evidence: TRINITY_IMAGE_TAG="${TRINITY_IMAGE_TAG:-v0.9.5-rc5}"; git ls-remote --tags shows v0.9.5-rc1…rc4 — no rc5 exists. Between this merge and the tag, dev's installer defaults to an image tag that isn't published (override via env exists). Precedent is identical (#2838 did rc4 the same way), so this is how the repo does it — the point is ordering: the work order says the final tag comes after M1 (#2861) lands, and merging this first puts VERSION=rc5 on a dev that is still red. Suggestion: merge #2861 first, then this; or split ac1c2a2 into its own chore: PR as #2838 was. Either is fine; just don't tag rc5 off a red dev.

[I2] Security check on the Caddy fix (4.4) — verified, recorded (Confidence: 9/10)
File: scripts/deploy/start.sh:396-399
Evidence: chmod 0644 /etc/caddy/Caddyfile.new runs after caddy validate succeeds and before mv. The rendered file interpolates only ${_do_header} and ${_private_block} (a remote_ip CIDR matcher) — no token, password or key — so world-readable is correct and the comment's "holds no secrets" is true. The test runs the real function under umask 077 and asserts 0o644; it was red before the fix per the body. Clean. One adjacent thought, not a finding: firstboot.sh's umask 077 applies to every other file start.sh creates on first boot too — worth one pass over anything else a non-root service reads (nothing found in this function; not audited repo-wide here).

[I3] Store guard (4.5/4.15) — verified, one documented limit (Confidence: 8/10)
File: src/frontend/src/stores/onboarding.js:26,56-59,69
Evidence: absent is set only on 403/404, checked first in fetchChecklist, never reset — a real failure (5xx/network) leaves it false and stays retryable (spec.js:124-135 proves both). Limit: an entitlement that flips mid-session (license activated while the tab is open) needs a reload; the code comment says "does not change within a session", which is the right trade for ambient guidance. Clean.

[I4] Test gap (4.9) — carried from the first review (Confidence: 5/10 → appendix)
ActivationChecklist.vue's fetchChecklist(true) + watch(agentsStore.agents.length) are still unpinned (the store side is). A two-line source-structure spec (the ent#392 shape) would stop a future "optimise away the force" from silently reverting to reload-only.

Clean Categories

• 4.1 / 4.3 — no backend, no auth surface.
• 4.2 — the overlapping-fetch note from the first review stands as appendix; unchanged risk, momentary at worst.
• 4.8 — the Caddy path still return 1s on a failed validate before touching the live file (start.sh:392-394).
• 4.10 — no v-html; watcher scoped to <script setup>.
• 4.13 — no architecture/flow surface changed; VERSION is release metadata.
• Tests: onboardingChecklistStore.spec.js covers the guard (3 new cases); test_2692_private_network_access.py covers the perms fix under the real umask.

Summary

• Critical: 0
• Informational: 3 (+1 appendix) — I2/I3 are verifications on the record; I1 is the one to decide: merge order vs fix(tests): evict the whole git_service package family — 13 leaked failures in test_ent615 (0.9.5 M1) #2861, or split the rc5 commit
• Scope: declared drift (Caddy fix + rc5 bump bundled)

Review complete. Next: /validate-pr 2862; sequence the merge after #2861 so v0.9.5-rc5 is tagged off a green dev.

[obasilakis]

@dolho re-review:

I1: split. ea0649aad reverts the rc5 bump here, and it now lives in #2866 on its own. That PR says to merge it only after #2861 (the 13 leaked test_ent615 failures behind dev's red backend-unit-test) and after this PR (the Caddyfile fix rc5 exists to carry). With the bump out, this PR can merge whenever it's approved, independent of the release cut. Net diff against dev is now 5 files, with no VERSION change.

I2 (umask 077 beyond the Caddyfile): checked every file start.sh --provision creates in the site phase that first boot runs. .env is 0600 by design and read by compose as root. /etc/trinity/public-ip and /etc/trinity/tls-status live under a 0700 root dir and are read by root (MOTD, installer). The systemd unit is written in the machine phase at build time, not under first boot's umask. On a droplet booted from the patched snapshot, all seven containers came up healthy, so the data paths the non-root containers write were fine. Nothing else found.

I3: agreed, and noted in the comment.

I4: leaving as is, for the reason in my previous reply (the merge train rejected this pin shape on #2773). Happy to add it if you and vybe agree it's wanted.

[obasilakis]

Restored the rc5 bump here (9ddae0195), undoing the split, as agreed: #2861 has merged (f31ed2da4), so this merges next, and v0.9.5-rc5 gets tagged once the dev push unit run is green. #2866 is closed as superseded.