Skip to content

fix(teardown): allow teardown when work is on any remote (incl. fork), not just local main - #31

Merged
kunchenguid merged 2 commits into
kunchenguid:mainfrom
e-jung:fix/teardown-local-only-fork-remote
Jun 22, 2026
Merged

fix(teardown): allow teardown when work is on any remote (incl. fork), not just local main#31
kunchenguid merged 2 commits into
kunchenguid:mainfrom
e-jung:fix/teardown-local-only-fork-remote

Conversation

@e-jung

@e-jung e-jung commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What

Fixes bin/fm-teardown.sh's local-only false-refuse: when a project is registered [local-only] but a task pushes its work to a fork (upstream-contribution PRs), teardown refused with "work not on main" even though the work IS safely on the fork remote. The local-only branch short-circuited to a strict local-main check before the git log HEAD --not --remotes check ran, so legitimate fork-pushed work looked unlanded.

Motivation

This bug bit us 3× this session:

  • no-mistakes teardown false-refused twice during this session's 8-PR upstream-contribution batch (projects registered local-only whose work was shipped via fork PRs to upstream).
  • A treehouse orphan got stuck behind the same false refuse.

In each case the work was safely on a fork remote, but teardown demanded a local-main merge that does not exist for upstream-contribution workflows.

The fix

Unify the safety check across modes. git log HEAD --not --remotes (empty == HEAD reachable from any remote-tracking branch) becomes the primary gate for every mode — and a fork IS a remote. The local-main check now runs only as a fallback for local-only projects whose work is genuinely on no remote at all.

Safety properties preserved:

  • Truly unpushed work (on no remote at all) is still REFUSED.
  • --force escape hatch is unchanged (still requires the captain's explicit discard word).
  • no-mistakes and direct-PR mode behavior is unchanged (they always used the --remotes check).

Files:

  • bin/fm-teardown.sh — the fix; also teaches the script to honor FM_ROOT_OVERRIDE / FM_STATE_OVERRIDE like its siblings (fm-guard.sh, fm-watch.sh), which is what makes it testable in isolation.
  • tests/fm-teardown.test.sh — new. Covers all six cases (fork-remote allow, truly-unpushed refuse, merged-to-main allow, no-mistakes origin allow, no-mistakes unpushed refuse, --force override). Wired into .github/workflows/ci.yml.
  • .github/workflows/ci.yml — runs tests/fm-teardown.test.sh in the Behavior tests job alongside tests/fm-wake-queue.test.sh.
  • README.md — lists tests/fm-teardown.test.sh in the behavior-test index.
  • AGENTS.md — rule docs: align tmux and harness guidance #3 and the local-only delivery-mode section both reflect that a fork counts as a remote in any mode.

Verification

  • bash tests/fm-teardown.test.sh — 6/6 ok
  • bash tests/fm-wake-queue.test.sh — 26/26 ok (no regression)
  • shellcheck bin/fm-teardown.sh tests/fm-teardown.test.sh — clean

Pipeline

Updates from git push no-mistakes

Note: the no-mistakes pipeline ran the full intent/rebase/review/test/document/lint gates on this change (all green; one review fix round applied by the no-mistakes agents is commit 79a8d44, which wired the new test into CI and README after captain approval). The no-mistakes push/pr steps could not complete in this environment because no-mistakes is hard-bound to its recorded remote kunchenguid/firstmate (403 for the e-jung contributor) and cannot drive a fork→upstream cross-repo PR, so the branch was pushed to the e-jung/firstmate fork and this PR opened manually. The change is no-mistakes-validated; the CI gates below (Require no-mistakes, CI lint/tests/invariants) re-verify it on the PR.

AI disclosure: Human-reviewed. The fix design, the safety-property analysis, the test matrix, and the AGENTS.md wording were authored by the captain's crewmate; the CI/README wiring was applied by the no-mistakes pipeline under explicit captain approval.

@kunchenguid

Copy link
Copy Markdown
Owner

Now that #30 and #37 have merged, this conflicts with main - #37's secondmate work substantially rewrote bin/fm-teardown.sh, the file this patches, so it's showing as not mergeable.

The fix is still relevant: current main's fm-teardown.sh still short-circuits a local-only project to a strict "merged into the local default branch" check (around line 377) and never consults the remotes, so a local-only project whose work was pushed to a fork would still be falsely refused. So this is worth carrying forward.

Could you rebase onto the latest main and re-resolve against the new teardown structure? Once it's rebased and green, we'll review it for merge. Thanks again.

e-jung added 2 commits June 22, 2026 22:43
…, not just local main

A local-only-registered project whose task pushes its work to a fork
(upstream-contribution PRs) was falsely refused by fm-teardown.sh: the
local-only branch short-circuited to a strict local-main check before
the 'on a remote' check ran, so fork-pushed work that IS safely on a
remote looked unlanded. This bit us 3x this session (no-mistakes x2,
treehouse orphan).

The fix unifies the safety check across modes: HEAD --not --remotes
(empty == on any remote, fork included) is the primary gate for every
mode. The local-main check now runs only as a fallback for local-only
projects whose work is genuinely on no remote at all. The real safety
property - refuse truly unpushed work - is preserved, and --force is
unchanged.

Re-applied onto the kunchenguid#37 teardown rewrite, which already added
FM_ROOT_OVERRIDE / FM_STATE_OVERRIDE support (so the new tests can drive
the script in isolation); this change is now purely the local-only
fork-remote eligibility fix.

Motivating incidents:
- no-mistakes teardown false-refused twice during this session's 8-PR
  upstream-contribution batch
- a treehouse orphan stuck behind the same false refuse

Tests (tests/fm-teardown.test.sh):
- local-only + HEAD on a fork remote-tracking branch -> ALLOW (the fix)
- local-only + truly unpushed work (no remote)      -> REFUSE (safety)
- local-only + work merged into local main          -> ALLOW  (no regression)
- no-mistakes + HEAD on origin                       -> ALLOW  (no regression)
- no-mistakes + truly unpushed                       -> REFUSE (no regression)
- local-only + unpushed + --force                    -> ALLOW  (escape hatch)

AI disclosure: Human-reviewed.
@e-jung
e-jung force-pushed the fix/teardown-local-only-fork-remote branch from 79a8d44 to c534b41 Compare June 22, 2026 22:43
@kunchenguid
kunchenguid merged commit 6f5800e into kunchenguid:main Jun 22, 2026
4 checks passed
@kunchenguid

Copy link
Copy Markdown
Owner

Thanks @e-jung, merged. The rebase onto the rewritten fm-teardown.sh structure was clean, and the dedicated tests/fm-teardown.test.sh - asserting the truly-unpushed REFUSE path in every mode, not just the fork-allow happy path - made the safety guard easy to verify. The fork-pushed-work false-refuse was a real recurring bite this session, so this is a genuinely useful fix. Appreciated.

leo1oel referenced this pull request in leo1oel/nemo Jun 23, 2026
Bring the upstream secondmate feature (PRs #37/#42/#43/#31/kunchenguid#45) to
herdr-backend, rewriting every treehouse/tmux mechanism into the herdr
equivalents this branch already uses. A secondmate is a crewmate whose
workspace is an isolated firstmate home (its own FM_HOME) and whose brief
is a charter; it runs the same spawn/brief/status/watch/teardown/recovery
lifecycle and is idle by default.

treehouse -> herdr mapping:
- Home leasing -> a herdr worktree of the firstmate repo. fm-home-seed.sh
  `-` runs `herdr worktree create --cwd $FM_ROOT --branch secondmate-<id>`
  and records the worktree path + open_workspace_id; herdr never recycles a
  worktree, so persistence is automatic and all lease/return bookkeeping is
  dropped. The workspace id is stored beside the home marker
  (.fm-secondmate-home.workspace) so retirement can find it.
- Retirement / seed rollback -> `herdr worktree remove --workspace <ws>
  --force`. An explicit <home> path stays a plain directory home (a git
  clone, no herdr worktree); spawn opens a workspace for it on the fly via
  `herdr workspace create` and teardown closes it with `herdr workspace
  close` then deletes the dir.
- firstmate_home_has_treehouse_slot -> "does the home record a workspace
  marker"; if yes remove via herdr, else plain rm.

New: bin/fm-home-seed.sh (charter fill, project cloning, no-mistakes init,
registry edits, transactional rollback, validate subcommand all ported
as-is), bin/fm-backlog-handoff.sh (no treehouse/tmux; near-verbatim).

Edits (re-implemented on the diverged herdr-backend files, not applied as
upstream hunks): fm-spawn.sh (--secondmate path, FM_HOME indirection,
registry/home resolution, launch in home, meta records
home=/home_workspace=/projects=), fm-teardown.sh (kind=secondmate
retirement via herdr, child-work refusal + --force discard, plus #31:
allow teardown when work is on any remote incl. a fork), fm-brief.sh
(--secondmate charter scaffold), fm-watch.sh (skip stale-pane wakes for
kind=secondmate), fm-project-mode.sh (honor FM_HOME/FM_DATA_OVERRIDE so a
secondmate resolves its own projects.md). kunchenguid#45: dropped the ':' from the afk
skill description. Skipped fm-bootstrap.sh / fm-harness.sh (removed on this
branch).

Docs: AGENTS.md + README.md gain herdr-flavored secondmate sections
(concept, secondmates.md routing table, fm-home-seed usage, idle-by-default
contract, backlog handoff, kind=secondmate recovery, watch idle) with no
treehouse/tmux wording.

Tests: new herdr-stubbed tests/fm-secondmate.test.sh drives the real
scripts against a stub `herdr` over the full lifecycle (seed `-`, register,
validate dup ids/homes/overlap, spawn, backlog handoff, retire);
tests/fm-teardown.test.sh ports the #31 any-remote/fork matrix to herdr.
shellcheck bin/*.sh tests/*.sh clean; 5/5 test scripts pass.
augustomklee added a commit to augustomklee/firstmate that referenced this pull request Jun 24, 2026
Port upstream batch 1: teardown fork-remote eligibility (kunchenguid#31) + afk skill desc (kunchenguid#45)
vipentti pushed a commit to vipentti/firstmate that referenced this pull request Aug 5, 2026
…, not just local main (kunchenguid#31)

* fix(teardown): allow teardown when work is on any remote (incl. fork), not just local main

A local-only-registered project whose task pushes its work to a fork
(upstream-contribution PRs) was falsely refused by fm-teardown.sh: the
local-only branch short-circuited to a strict local-main check before
the 'on a remote' check ran, so fork-pushed work that IS safely on a
remote looked unlanded. This bit us 3x this session (no-mistakes x2,
treehouse orphan).

The fix unifies the safety check across modes: HEAD --not --remotes
(empty == on any remote, fork included) is the primary gate for every
mode. The local-main check now runs only as a fallback for local-only
projects whose work is genuinely on no remote at all. The real safety
property - refuse truly unpushed work - is preserved, and --force is
unchanged.

Re-applied onto the kunchenguid#37 teardown rewrite, which already added
FM_ROOT_OVERRIDE / FM_STATE_OVERRIDE support (so the new tests can drive
the script in isolation); this change is now purely the local-only
fork-remote eligibility fix.

Motivating incidents:
- no-mistakes teardown false-refused twice during this session's 8-PR
  upstream-contribution batch
- a treehouse orphan stuck behind the same false refuse

Tests (tests/fm-teardown.test.sh):
- local-only + HEAD on a fork remote-tracking branch -> ALLOW (the fix)
- local-only + truly unpushed work (no remote)      -> REFUSE (safety)
- local-only + work merged into local main          -> ALLOW  (no regression)
- no-mistakes + HEAD on origin                       -> ALLOW  (no regression)
- no-mistakes + truly unpushed                       -> REFUSE (no regression)
- local-only + unpushed + --force                    -> ALLOW  (escape hatch)

AI disclosure: Human-reviewed.

* no-mistakes(test): Wire fm-teardown.test.sh into CI and README

---------

Co-authored-by: e-jung <8334081+e-jung@users.noreply.github.com>
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