Skip to content

fix(bootstrap): enforce safe Treehouse compatibility - #1247

Open
Moshik21 wants to merge 1 commit into
kunchenguid:mainfrom
Moshik21:fm/teardown-treehouse-json-scout-sol
Open

fix(bootstrap): enforce safe Treehouse compatibility#1247
Moshik21 wants to merge 1 commit into
kunchenguid:mainfrom
Moshik21:fm/teardown-treehouse-json-scout-sol

Conversation

@Moshik21

Copy link
Copy Markdown

Intent

Prevent Firstmate from accepting unsafe Treehouse versions after v2.0.0's non-atomic state persistence corrupted a shared worktree-pool record. Require Treehouse 2.0.1 or newer plus durable lease support only for backends that use Treehouse, reject malformed output including any treehouse --version output with an extra line, preserve the exact existing MISSING upgrade diagnostic and captain-consent install path, and retain the accepted focused regression. Preserve fm-teardown.sh's generic-error refusal and do not touch recovered project pools or task cleanup. This reconciled history intentionally preserves original accepted commits 6c245b2 and 0b3e604 together with every prior pipeline-generated commit through 85311e3; publish only through the authorized Moshik21/firstmate fork while keeping kunchenguid/firstmate as the official upstream and PR base.

What Changed

  • Require Treehouse 2.0.1+ with durable lease support for Treehouse-backed runtimes while preserving the existing MISSING upgrade diagnostic and Orca exemption.
  • Reject unparseable, prerelease, multiline, and trailing-blank-line version output.
  • Document the compatibility requirements and add focused bootstrap regression coverage for version boundaries, lease support, malformed output, and backend scoping.

Risk Assessment

✅ Low: The sentinel-based capture now preserves and rejects trailing extra lines while retaining the Treehouse version floor, lease check, backend scoping, exact diagnostic, install-consent path, and required history.

Testing

The focused bootstrap and teardown suites passed; manual CLI evidence shows safe Treehouse is accepted, unsafe/malformed/lease-less versions receive the exact existing MISSING diagnostic, and Orca ignores unused Treehouse, while diff and ancestry checks confirmed teardown/data boundaries and required history were preserved.

Evidence: Treehouse bootstrap compatibility CLI transcript
CASE: minimum-safe
treehouse --version: v2.0.1
lease support: 1
backend: -
bootstrap output: <silent: accepted>

CASE: corruption-prone-v2.0.0
treehouse --version: v2.0.0
lease support: 1
backend: -
bootstrap output: MISSING: treehouse (install: curl -fsSL https://kunchenguid.github.io/treehouse/install.sh | sh)

CASE: extra-diagnostic-line
treehouse --version: $'v2.0.1\nunexpected diagnostic line'
lease support: 1
backend: -
bootstrap output: MISSING: treehouse (install: curl -fsSL https://kunchenguid.github.io/treehouse/install.sh | sh)

CASE: extra-blank-line
treehouse --version: $'v2.0.1\n'
lease support: 1
backend: -
bootstrap output: MISSING: treehouse (install: curl -fsSL https://kunchenguid.github.io/treehouse/install.sh | sh)

CASE: missing-durable-lease
treehouse --version: v2.1.0
lease support: 0
backend: -
bootstrap output: MISSING: treehouse (install: curl -fsSL https://kunchenguid.github.io/treehouse/install.sh | sh)

CASE: orca-does-not-use-treehouse
treehouse --version: $'v2.0.1\nunexpected diagnostic line'
lease support: 0
backend: orca
bootstrap output: <silent: accepted>

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed ✅
  • 🚨 bin/fm-bootstrap.sh:542 - The required criterion says to “reject malformed output including any treehouse --version output with an extra line,” but command substitution strips all trailing newlines before the newline check. Consequently, output such as v2.0.1\n\n becomes v2.0.1 and is accepted. Capture with a sentinel or another mechanism that preserves trailing newlines, then validate exactly one line.

🔧 Fix: Reject trailing lines in Treehouse version output
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-bootstrap.test.sh
  • bash tests/fm-teardown-endpoint-safety.test.sh
  • Manual bootstrap CLI matrix covering Treehouse v2.0.1, v2.0.0, extra diagnostic/blank lines, missing lease support, and the Orca exemption
  • git diff --quiet 99533c5d7d3702050e6084429dddff6ea4fe1aa0..bb774dc1ed51483d070802e9d87b6229a850bb76 -- bin/fm-teardown.sh
  • git merge-base --is-ancestor checks for preserved commits 6c245b2, 0b3e604, and 85311e39
  • Diff inspection confirming no changes under projects, data, or state
✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning
  • ⚠️ linter found issues (exit code 127)

🔧 Fix: Confirm clean lint with pinned ShellCheck 0.11.0
1 warning still open:

  • ⚠️ linter found issues (exit code 127)
✅ **Push** - passed

✅ No issues found.

@kunchenguid

kunchenguid commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch.

When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again.

Noted for firstmate#1247 at 7b90bf24.

@Moshik21

Copy link
Copy Markdown
Author

Rebased onto current main (66b0f77) to clear the conflict. No implementation was redone; the change is the rebase plus one genuine conflict resolution.

The conflict

main has since replaced the per-tool version parsers in bin/fm-bootstrap.sh with a single shared helper, tool_version_at_least <tool> <min-version>, and dropped no_mistakes_version_parts / no_mistakes_compatible. This branch had added a parallel per-tool pair, treehouse_version_parts / treehouse_version_compatible, in the pattern main just removed.

Resolved by folding this branch's stricter parsing into the shared helper rather than reintroducing a second parser:

  • tool_version_at_least takes an optional third argument, exact-line. Without it the existing lenient behaviour is unchanged, so no-mistakes version v1.31.2 (fake) still parses as main expects.
  • With exact-line the whole --version output must be one bare version line. This keeps the sentinel-based capture from the earlier review fix, so a trailing blank line or an extra diagnostic line is still rejected rather than silently stripped by command substitution.
  • treehouse_compatible() is now tool_version_at_least treehouse "$TREEHOUSE_MIN" exact-line && treehouse_supports_lease, and the floor is a single TREEHOUSE_MIN=2.0.1 matching main's NO_MISTAKES_MIN=1.31.2 style.

The test-side diff carried through the rebase byte-for-byte; every added regression case still passes, including the version boundaries, prerelease rejection, malformed and multiline output, lease support, and the Orca exemption.

Note on the history-preservation line in the description

The Intent section says this branch preserves commits 6c245b2, 0b3e604, and 85311e39 as ancestors. A rebase rewrites those commits regardless of how it is done, so that ancestry claim can no longer hold on any rebased head. The pipeline-generated history was squashed into one commit; the resulting tree and diff are unchanged apart from the conflict resolution described above.

Verification

  • bin/fm-lint.sh clean (pinned ShellCheck 0.11.0).
  • bin/fm-doc-audience-check.sh clean.
  • bin/fm-test-run.sh --changed --base origin/main: green apart from tests/fm-calm-pi-extension.test.sh, which fails the same way on a clean origin/main checkout in this environment and is unrelated to these files.

@kunchenguid kunchenguid removed the wheelhouse:pending-contributor-action Managed by Wheelhouse label Jul 31, 2026
Bootstrap only checked that treehouse supported `get --lease`, so a
build older than 2.0.1 - the release that adds atomic state persistence
and conservative corrupt-state recovery - passed the check and left
guarded cleanup exposed to malformed Treehouse state.

Add a version gate alongside the lease probe: parse `treehouse
--version`, reject unparseable, multiline, or trailing-line output, and
report treehouse as MISSING below 2.0.1.
@Moshik21
Moshik21 force-pushed the fm/teardown-treehouse-json-scout-sol branch from 84f6c25 to 7cb3950 Compare July 31, 2026 21:48
@Moshik21

Copy link
Copy Markdown
Author

Fixed the portable-serial failure (default materialization should stay quiet). Pushed as 7cb3950; the delta over the previous head is 4 added lines in one test fixture.

Diagnosis

The gate is not firing unconditionally, and the default backend is not being mistreated. Running identical fixtures against origin/main and this branch:

fixture (backend) origin/main this branch
treehouse stub with no --version, tmux quiet MISSING: treehouse
treehouse stub answering v2.0.1, tmux quiet quiet
treehouse stub with no --version, orca quiet quiet
treehouse absent, tmux MISSING: treehouse MISSING: treehouse

Row 3 shows the check stays scoped to Treehouse-using backends: an incompatible Treehouse under backend=orca produces no diagnostic. Row 4 shows the Treehouse-absent path is unchanged from main; that line comes from the pre-existing generic missing-tool loop, not from this change.

The failing test is not on a default or unset backend either. tests/fm-startup-memory-budget.test.sh sets FM_BACKEND=tmux explicitly in run_bootstrap, and tmux genuinely requires Treehouse.

The actual cause is that test's fake Treehouse stub, which implements only get --help and not --version. Under the version floor a Treehouse that prints nothing for --version is unparseable, so it is reported as an upgrade request - the same behaviour the accepted unparseable Treehouse version reports an upgrade case pins. That test file landed on main after this branch was cut, which is why the branch updated the fixtures in fm-bootstrap, fm-session-start, and fm-secondmate-sync but not this one.

Fix

The stub now answers --version with v2.0.1, identical to the pattern already used in the three other bootstrap-invoking fixtures. The default materialization should stay quiet assertion is untouched and still fails if bootstrap emits anything; the fixture simply once again satisfies its own stated contract of a toolchain where every required tool is present.

I also audited every fake Treehouse stub in the suite by extracting each stub body rather than grepping the file. Thirteen test files define one and nine omit --version, but only four invoke bootstrap or session-start, and of those only this one failed. The three passing ones are left untouched.

Verification

Full portable CI lanes on 7cb3950:

  • portable-parallel-1: 11 tests, 0 failed.
  • portable-parallel-2: 13 tests, 0 failed.
  • portable-serial: 67 tests, 2 failed - fm-backend-tmux-smoke and fm-calm-pi-extension, both of which fail identically on a clean origin/main checkout in this environment and touch none of these files.

bin/fm-lint.sh clean (pinned ShellCheck 0.11.0).

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.

2 participants