Skip to content

fix: Bugbot findings from promotion PR #383 (summary copy + backfill hook docs) - #394

Merged
shujaatTracebloc merged 4 commits into
developfrom
fix/bugbot-summary-backfill
Jul 24, 2026
Merged

fix: Bugbot findings from promotion PR #383 (summary copy + backfill hook docs)#394
shujaatTracebloc merged 4 commits into
developfrom
fix/bugbot-summary-backfill

Conversation

@shujaatTracebloc

@shujaatTracebloc shujaatTracebloc commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Resolves the second batch of Cursor Bugbot findings on the client promotion PR (#383, develop → main).

Fixes:

  • Tier 0 reboot promise is false_reboot_note unconditionally told Linux users tracebloc auto-restarts after a reboot, but the zero-privilege Tier 0 path in ensure_cluster_autostart never enables docker.service. It now keys off a TB_DOCKER_AUTOSTART signal (set only when systemctl enable docker succeeds) and, when autostart wasn't enabled, honestly tells the user to start Docker. (Release develop → main #383 (comment))
  • Node-local summary path wrong — the connected summary always printed Data /tracebloc/<ns>, but under TB_STORAGE_MODE=node-local (RFC-0003 Option C) datasets live in-node on k3s local-path with no host /tracebloc. The Data line now branches on the storage mode. (Release develop → main #383 (comment))
  • Backfill docs claim post-installvalues.yaml and values.schema.json described the metadata-backfill hook as post-upgrade/post-install, but the Job is deliberately post-upgrade only (a fresh install has no pre-cutover data; a cold image pull could fail the install). Corrected both to match the template's helm.sh/hook: post-upgrade annotation. (Release develop → main #383 (comment))

Goldens are byte-stable: the copy catalog pins the canonical full-install path (autostart enabled, hostpath storage), mirroring the existing TB_CLI_USABLE_NOW pin; the new honest / node-local branches are covered by summary.bats unit tests. scripts/manifest.sha256 refreshed for the summary.sh + cluster.sh changes.

Lands on develop; the promotion PR head picks these up on the next sync and Bugbot re-reviews.

🤖 Generated with Claude Code


Note

Low Risk
User-facing copy and documentation corrections plus a read-only systemctl is-enabled check; no change to cluster provisioning or Helm hook behavior.

Overview
Addresses Bugbot findings from promotion PR #383: installer messaging and Helm values docs now match actual behavior.

Reboot footer (summary.sh + cluster.sh) — Linux no longer always promises automatic restart after reboot. ensure_cluster_autostart sets TB_DOCKER_AUTOSTART when docker.service is persistently enabled (including Tier 0 installs where Docker was already enabled on boot, without running systemctl enable), and when a privileged systemctl enable docker succeeds. Transient enabled-runtime does not set the flag. _reboot_note uses three paths: macOS/Windows → open Docker Desktop; Linux with autostart → restarts automatically; Linux without → start Docker first.

Connected summary data line — When TB_STORAGE_MODE=node-local, the footer shows Data in-node (k3s local-path) instead of Data /tracebloc/<ns>, since there is no host bind mount.

Helm docsmetadataBackfill in values.yaml and values.schema.json now states the hook is post-upgrade only (not post-install), aligned with the template.

Tests and copy-catalog pins cover the new branches; manifest.sha256 updated for cluster.sh and summary.sh.

Reviewed by Cursor Bugbot for commit 9c9d740. Bugbot is set up for automated code reviews on this repo. Configure here.

…kfill docs)

- Reboot note: only promise auto-restart when docker.service autostart was
  actually enabled; Tier 0 (zero-privilege) leaves it to the user, so be honest
- Connected summary Data path: branch on TB_STORAGE_MODE — node-local keeps
  datasets in-node on k3s local-path, so don't point at a host /tracebloc
- Backfill docs: correct values.yaml + values.schema.json to say post-upgrade
  ONLY (not post-install) to match the Job's helm.sh/hook annotation
- Refresh scripts/manifest.sha256 for the summary.sh + cluster.sh changes

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTracebloc shujaatTracebloc self-assigned this Jul 24, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4c9875d. Configure here.

Comment thread scripts/lib/cluster.sh
shujaat hasan and others added 3 commits July 24, 2026 16:04
… state

Bugbot: on Tier 0, ensure_cluster_autostart deliberately never runs
`systemctl enable docker`, and TB_DOCKER_AUTOSTART was set only when *this*
run enabled the service. A host where docker.service is already enabled on
boot (a normal package install) therefore kept the flag unset, so _reboot_note
told the user to start Docker manually even though the cluster returns on its
own after a reboot.

Seed TB_DOCKER_AUTOSTART from the current on-boot state via an unprivileged
`systemctl is-enabled docker` read (matching only the persistent "enabled"
state, not the transient "enabled-runtime"). Add cluster.bats coverage for the
pre-enabled, disabled, and enabled-runtime cases; refresh manifest.sha256.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-backfill

# Conflicts:
#	scripts/manifest.sha256
…-backfill

# Conflicts:
#	scripts/manifest.sha256

@aptracebloc aptracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shujaatTraceblocApproved. Clean set of honest-messaging fixes; the through-line (don't let the summary claim something untrue) is exactly right. I verified the load-bearing bits:

  • ✅ Manifest hashes match for cluster.sh and summary.sh (recomputed) — R8 trust root intact; both parse (bash -n).
  • ✅ The TB_DOCKER_AUTOSTART seed is correct. systemctl is-enabled docker is an unprivileged read (no sudo prompt), the flag is set only on the persistent "enabled" state and before the tier logic, so it captures the common case (a normal Docker package install already enabled the service → Tier 0 still honestly promises auto-restart) and the sudo-enable path also sets it. It correctly excludes the transient "enabled-runtime" — and that edge is explicitly tested.
  • ✅ Tests are thorough and precise — the three new cluster.bats cases pin the seed exactly (already-enabled → set; disabled → not set; enabled-runtime → not set), summary.bats covers both reboot-note branches + both storage modes, and copy-catalog.bats pins TB_DOCKER_AUTOSTART=1 for deterministic rendering (mirroring TB_CLI_USABLE_NOW). (bats isn't installed locally — parse-checked + logic-reviewed; CI runs the suite.)

⚪ Nits (non-blocking)

  • Socket-activation blind spot. The seed recognizes only is-enabled docker == "enabled". A setup where docker.service is "static" but docker.socket is enabled (on-demand activation) falls to the conservative "start Docker" message even though Docker may come up on first connection after boot. That's the safe direction (under-promise — the whole point of this fix), but a one-line comment noting it, or also checking docker.socket, would tighten it.
  • TB_STORAGE_MODE=node-local is a flag-gated prototype (RFC-0003 Option C, not GA); the branch is correct and defaults to hostpath — just flagging it quietly adds summary surface for an unshipped path.

What's good

Every change makes a claim honest (post-upgrade-not-install scope, conditional auto-restart, real data location). The standout is the test coverage — every branch pinned, including the transient-enable edge that's easy to get wrong — and the manifest is kept in lockstep.

Touches @saadqbal's code-owned scripts/lib/ + manifest.sha256 (R8), so his code-owner review is the intended backstop; this is the second set of eyes. No blocking issues.

🤖 Generated with Claude Code

@shujaatTracebloc
shujaatTracebloc merged commit 9c03bc9 into develop Jul 24, 2026
39 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the fix/bugbot-summary-backfill branch July 24, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants