fix: surface ready work and harden operating rules - #52
Open
ruby-dlee wants to merge 22 commits into
Open
Conversation
Promote eight hard-won operating rules into the surfaces that load when they apply, rather than appending a lessons section that nothing reads. AGENTS.md: - section 9 gains outcome fidelity (a success-shaped signal is not the outcome) with five verified instances and the check each requires, absolute ownership of any surface already in front of the captain, durable capture of input arriving through reapable channels, and the rule that a qualified answer contradicting its own selection is not an approval. - section 8's wake-handling order gains diagnose-the-layer-before-fixing. - tightens the no-internals rule with tool-not-a-person. lavish-decision-boards: records the layout curtain and --no-gate, makes the protect-answers rule absolute, and notes the artifact iframe is sandboxed without allow-same-origin so parent-frame eval cannot verify content. stuck-crewmate-recovery: name the mechanism before applying a remedy. memory-hygiene: promote-to-the-instruction-surface-or-delete as the default. Verified against lavish-axi 0.1.36: --no-gate sets no server-side state, it only stamps ?no-gate=1 on the printed URL, so the curtain returns if the query string is dropped. The tab-title gate indicator reported earlier is false - the title tracks whether the artifact HTML carries a <title>, identically gated or not.
…ions Six further rules, each folded into the section that already owned its topic and each paired with a deletion, so the instruction surface sharpens rather than grows. AGENTS.md: - section 8 generalises the crewmate-state sentence into the rule it was an instance of: a subsystem that ships a status command makes that command the only acceptable source for its state, and inferring state from its logs, dotfiles, lock paths, or PID files is a reader defect. Supersedes and removes the duplicated crew-state contract that the token-discipline line restated. - section 8's diagnosis block gains falsification: suspect yourself before suspecting an attacker, rule out the innocent explanation before treating an observation as confirmation, and note that a theory which grows to absorb each observation is confirmation bias while real diagnosis narrows. - section 4 states that harness, model, and reasoning tier are the captain's dial, never a firstmate preference. - section 7 requires the dependent half of a two-part instruction to be filed with --blocked-by when it is spoken, not held in conversation. - section 9 states that filing a defect discharges nothing. memory-hygiene gains the capability-claim standard: a limit must cite the command and output that proved it, and "do not retry" is never written, because that clause suppresses the check that would disprove a wrong limit. The watcher-lock example was corrected before use: that lock is a symlink to a transient owner file, not a directory, so a direct read errors rather than returning an empty-but-valid value.
Promote eight hard-won operating rules into the surfaces that load when they apply, rather than appending a lessons section that nothing reads. AGENTS.md: - section 9 gains outcome fidelity (a success-shaped signal is not the outcome) with five verified instances and the check each requires, absolute ownership of any surface already in front of the captain, durable capture of input arriving through reapable channels, and the rule that a qualified answer contradicting its own selection is not an approval. - section 8's wake-handling order gains diagnose-the-layer-before-fixing. - tightens the no-internals rule with tool-not-a-person. lavish-decision-boards: records the layout curtain and --no-gate, makes the protect-answers rule absolute, and notes the artifact iframe is sandboxed without allow-same-origin so parent-frame eval cannot verify content. stuck-crewmate-recovery: name the mechanism before applying a remedy. memory-hygiene: promote-to-the-instruction-surface-or-delete as the default. Verified against lavish-axi 0.1.36: --no-gate sets no server-side state, it only stamps ?no-gate=1 on the printed URL, so the curtain returns if the query string is dropped. The tab-title gate indicator reported earlier is false - the title tracks whether the artifact HTML carries a <title>, identically gated or not.
…ions Six further rules, each folded into the section that already owned its topic and each paired with a deletion, so the instruction surface sharpens rather than grows. AGENTS.md: - section 8 generalises the crewmate-state sentence into the rule it was an instance of: a subsystem that ships a status command makes that command the only acceptable source for its state, and inferring state from its logs, dotfiles, lock paths, or PID files is a reader defect. Supersedes and removes the duplicated crew-state contract that the token-discipline line restated. - section 8's diagnosis block gains falsification: suspect yourself before suspecting an attacker, rule out the innocent explanation before treating an observation as confirmation, and note that a theory which grows to absorb each observation is confirmation bias while real diagnosis narrows. - section 4 states that harness, model, and reasoning tier are the captain's dial, never a firstmate preference. - section 7 requires the dependent half of a two-part instruction to be filed with --blocked-by when it is spoken, not held in conversation. - section 9 states that filing a defect discharges nothing. memory-hygiene gains the capability-claim standard: a limit must cite the command and output that proved it, and "do not retry" is never written, because that clause suppresses the check that would disprove a wrong limit. The watcher-lock example was corrected before use: that lock is a symlink to a transient owner file, not a directory, so a direct read errors rather than returning an empty-but-valid value.
* fix(teardown): resolve source projects from the firstmate home
Restores `main` to green on the `Lint shell scripts` and `Behavior tests`
checks. Both were failing for unrelated reasons.
Behavior tests: `validate_secondmate_project_clones` resolved the parent's
project clones from `$expected_source/projects` unless FM_PROJECTS_OVERRIDE
happened to be set, so `bin/fm-teardown.sh` was the only script that never
defined the canonical `PROJECTS="${FM_PROJECTS_OVERRIDE:-$FM_HOME/projects}"`
that fm-spawn, fm-bootstrap, fm-home-seed, fm-fleet-sync, fm-fleet-snapshot,
and fm-checkout-refresh all share. With FM_HOME pointing at a home outside the
repo - the documented multi-home layout of AGENTS.md section 2 - teardown looked
for the clones in the repo root, found nothing, and refused to retire any
secondmate. Resolve through the shared definition, mirroring how the registry
lookup beside it already resolves through $DATA. The refusal also printed an
empty path, because it reported the variable the failed assignment had just
cleared; report the candidate instead.
Lint: `id=minimum-age-cohort` and `legacy_id=minimum-age-legacy` are string
literals, but unquoted they parse as arithmetic, which is what SC2100 was
reporting. Quote them, matching the surrounding assignments.
The lifecycle fixture also had to stop tripping teardown's unlanded-work proof:
the fake no-mistakes marks a clone with `.no-mistakes-init`/`.no-mistakes-doctor`
touch files, while the real one leaves the clone with nothing to commit, so the
markers are dropped once the assertions have consumed them. Seeding proves a home
against its source repo's default-branch tip, so the suite additionally stood up
an isolated default-branch source repo; without it the lifecycle could only run
while the checkout itself sat on its default branch, never from a task branch.
Those two fixture changes are carried over from the unlanded work on
fm/fm-account-rescue-a2; the rest of that branch is left alone.
Note in CONTRIBUTING.md that CI runs the behavior loop under `set -eu` while the
documented local loop does not, so a red CI log is a lower bound on the failures
rather than the whole list.
* test(secondmate): carry unlanded fixture repairs for three suites
CI's behavior loop is fail-fast, so it stopped at fm-secondmate-lifecycle-e2e
and had never reached these three suites. All three were already red on their
own, identically with and without the teardown fix in the previous commit, and
identically with treehouse/herdr hidden - so neither a regression nor a
missing-backend artifact, just failures the fail-fast log could not show.
Fixtures for these repairs already existed unlanded on fm/fm-account-rescue-a2;
they are carried here verbatim rather than rewritten, and the rest of that
branch (its capacity-rescue feature, watcher work, and CI changes) is left
alone. What they correct:
- sync: the T10/T11 worlds never registered `sm` in the secondmate registry, so
spawn refused before it could fast-forward the home, and T11 still asserted a
"sync skipped before launch" warning that the product replaced with a hard
refusal ("refusing secondmate launch because its home freshness is
unresolved"). The assertion now pins the behavior the product actually has.
- liveness: the enforced-recovery world used a bare directory where a real
default-branch repo is required, and stubbed node wholesale even though the
sweep genuinely needs it for fm-contained-read.cjs.
- safety: two tests re-derived the private checkout-lock hash by hand and got it
wrong - the product hashes the LOWERCASED path, so the hand-rolled key only
matched when mktemp happened to pick an all-lowercase suffix, making the lock
assertions pass or fail with the random temp name. They now ask the lock
library itself via fm_checkout_lock_path, which is the single owner of that
identity and cannot drift from it.
sync and liveness now pass. safety advances to a real remaining product defect
rather than the fixture noise that was masking it: when a Treehouse-acquired
home is the active firstmate home, fm-home-seed takes the home lifecycle lock
for that path and then waits the full FM_ACCOUNT_LIFECYCLE_LOCK_WAIT_SECONDS on
a lock it already holds, timing out instead of refusing cleanly with
"secondmate home cannot be the active firstmate home" as the test expects. That
lock-ordering fix is left to a dedicated change.
* fix(home-seed): refuse an acquired active home instead of deadlocking
The secondmate home lifecycle locks are keyed on the home path and are not
reentrant. Seed takes one for the active $FM_HOME up front, then a second for the
destination home. The explicit-home branch proves the destination is legal with
refuse_active_home_path BEFORE taking that second lock; the Treehouse-acquired
branch never proved it at all.
So an acquired home that IS the active firstmate home resolved to the lock seed
already held, and the second acquisition could only wait out
FM_ACCOUNT_LIFECYCLE_LOCK_WAIT_SECONDS against itself:
error: timed out waiting for secondmate home lifecycle lock for home-<key>
Prove the destination first in the acquired branch too, mirroring the explicit
branch, so the caller gets the actionable refusal instead of a timeout:
error: secondmate home cannot be the active firstmate home: <home>
The check lands after retention is armed, so an acquired home is still retained
for manual recovery rather than returned.
tests/fm-secondmate-safety.test.sh already asserted this refusal; it had never
been reached because the suite fail-fasts and an earlier fixture defect stopped
it first.
* test(secondmate-safety): isolate two seed fixtures from refresher state
Two more links in this suite's stale-fixture chain, both reached only after the
self-deadlock fix in the previous commit let execution get this far.
Empty-charter-fields: the two seed attempts shared one destination path. The
refresher records each covered checkout's physical identity, so recreating a home
at a path it already recorded reads as "covered checkout physical identity
drifted" and the second seed was refused for an unrefreshable default branch long
before it could reach the charter-field validation under test. Give the second
attempt its own destination. That test also left its identity records in the real
${XDG_STATE_HOME:-$HOME/.local/state}/firstmate/checkout-refresh, so it both wrote
through to the operator's own state and depended on what earlier runs left there;
scope FM_CHECKOUT_REFRESH_STATE_ROOT into the case.
Uninspectable projects: the home was a bare directory, but seed runs the
refresher's preflight on it before inspecting projects and that preflight requires
an exact Git repository root, so the home was refused as unrefreshable and never
reached the inspection failure. Use a real clone, as the sibling conversion case
already does.
Neither assertion changed. The suite still does not pass: with an unreadable
projects directory the preflight cannot read the tree and fails first, so the
specific "cannot inspect existing projects directory" message is unreachable
without reordering seed's --no-projects precondition checks ahead of the
refresher preflight. That is a product-ordering decision, not fixture staleness,
and it is left to a dedicated change rather than guessed at here.
* test(teardown): repair stale endpoint and secondmate metadata fixtures
Four links of this suite's stale-fixture chain. Tests passing in the suite go
from 11 to 14; it still does not pass, and the next link is characterized in
data/fm-main-green-g7/report.md rather than guessed at.
Endpoint visibility (generation-lock cases, parent and child): both use a
blocking `tmux kill-window` as the synchronization point that lets the test race
a concurrent metadata update and prove teardown locked the generation first. The
stub answered every other subcommand with silent success, so `list-windows`
returned nothing, the endpoint read as absent, and teardown skipped endpoint
cleanup entirely - `kill-window` was never called and the marker never appeared.
A stub pinned to either state cannot work: always-absent skips the kill,
always-present makes teardown refuse because it then requires confirmed absence.
The stub is now observable until the kill lands and gone after it.
kind=secondmate metadata: five fixtures set `project=` to a directory beside the
home, but for a secondmate the product requires the project metadata to resolve to
the registered home, and a real spawn records project == home (confirmed against
the metadata a real secondmate spawn wrote). Normalized to the home.
Unreachable assertion (zellij child endpoint): the test demanded stderr say the
endpoint "is still alive", but fm_backend_agent_alive implements tmux and herdr
only and answers `unknown` for every other backend, so no fixture could ever have
produced that string for zellij. What the test exists to prove - that an endpoint
whose state cannot be PROVEN still blocks destructive cleanup - is pinned by the
refusal plus the retained lease, metadata, and worktree, all of which already
pass. The assertion now names the message that path can actually emit, which is
the stronger safety statement anyway.
Nothing was skipped, suppressed, or relaxed; the one assertion change is argued
above and its safety property remains fully asserted.
* test(teardown): answer parent and child tmux probes per target
* test(teardown): give forced-child cases a resolvable per-target endpoint
* test(teardown): align the forced-child parent meta with a real secondmate spawn
* fix(teardown): preserve forced child cleanup status instead of flattening to 1
* test(teardown): derive checkout lock paths with the product helper
* test(teardown): scope the broken ps stub to the return under test
* test(teardown): resolve the child endpoint in the lock-contention case
* test(teardown): give the herdr case a parseable session list
* test(teardown): classify the report-publication endpoint as present-idle
* test(teardown): classify post-quiescence endpoints as present-idle
* test(teardown): register the drifted secondmate home before the identity check
* fix(home-seed): honour --no-projects before the destination tree read
* test: classify idle fake tmux panes instead of leaving them unknown
* docs: record the tmux absence-proof contract for fixtures
* test(secondmate-safety): lay leased teardown homes out in a Treehouse pool
* test(secondmate-safety): give leased teardown homes real registered clones
* fix(teardown): lock the container when proving a nested surviving repository
* test(secondmate-safety): make the plain-clone teardown home a real clone
* test(secondmate-safety): make the unproven-lock home a real clone
Partial: this repairs the home shape (a real firstmate clone with the full
operational directory set and a registered project clone sharing its source's
origin), which is what teardown proves first. The case still fails, now on a
different and deeper point: its child worktree is created as a linked worktree of
the home's own project clone, and validate_secondmate_repository_worktree_graph
requires that clone to own exactly itself, so retirement refuses before reaching
the child lock refusal the case pins. Resolving that needs the real-world shape of
a child worktree relative to a secondmate's project clone worked out; not guessed
here.
* docs: record the https-origin requirement for local suite reproduction
* fix(teardown): admit only a registered child's linked project worktree
A secondmate holds its project clones under home/projects while its crewmates'
worktrees live outside in the Treehouse pool as linked worktrees of those clones -
the ordinary running state of any secondmate with live crewmates. Requiring the
home's clone to own exactly itself refused that ordinary case.
The graph check stays where it is, before any destruction, and now admits a linked
worktree only when a REGISTERED child of that home vouches for it by recording both
that exact worktree and that exact project, with both paths canonicalized before
comparison. An unenumerable child state is an error, never an empty set. Two new
regression tests pin the refusals that keep this from becoming a hole: an
unregistered linked worktree, and one registered against a different project.
* test(teardown): classify the retiring endpoint and use the registered clone
* test(secondmate-safety): give the symlink teardown fixtures their real home shape
Adds a shared make_secondmate_home_and_source /
clone_registered_secondmate_project pair building the standard two-sided fixture -
a real firstmate source, a plain-clone home with the full operational directory
set, and a registered project clone existing on both sides sharing an origin - and
converts the two operational-dir symlink cases onto it.
Partial: both cases still fail, now on the open ordering question rather than on
home shape. bin/fm-teardown.sh's cleanliness proof runs before its operational-dir
proof, so a symlinked operational directory is reported as "unlanded changes: ??
state" instead of by the operational-dir proof that is the subject of both cases.
That ordering is a product decision and is not guessed here.
* fix(teardown): report structural home faults by their own proof
validate_firstmate_home_for_removal proved the home's cleanliness before its
structural shape, and a structural fault surfaces to `git status` as untracked
content, so an operational directory symlinked out of the home was reported as
"secondmate home has unlanded changes: ?? state". An operator debugging a refused
teardown was told the wrong cause and would go looking for uncommitted work that
does not exist.
The operational-directory and project-clone proofs now run before the landed-state
proof, and the operational-directory proof runs before the authority and lease
proofs that also read the home's state directory. Every one of these is read-only
and every one already ran before any destruction, so this changes only which
refusal speaks first: each still returns non-zero on its own failure, so a home
with both a structural fault and genuinely unlanded work is still refused.
Adds test_secondmate_teardown_reports_structural_faults_before_cleanliness, whose
negative assertion is the point - the cleanliness message must NOT be what
surfaces for a containment violation.
* fix(teardown): refuse a symlinked operational directory outright
Teardown is destructive and proves what it deletes by exact physical identity -
canonical paths and device:inode checks - and a symlink is precisely what makes the
logical path differ from the physical target that would be removed.
secondmate_state_metadata already refused any symlinked state directory for that
reason, while validate_operational_dirs permitted one resolving inside the home, so
the product both permitted and forbade the same shape. The operational-dir proof now
refuses it for all four directories, and names the symlink and its target so an
operator is not left guessing which path is at fault.
The inside-symlink case is retargeted to assert that refusal, because the decision
changes what correct behaviour is rather than because the old assertion was
inconvenient; it keeps its safety pins (home, symlink target, and metadata all
retained, and no endpoint stopped before the home was proven). The outside-symlink
case keeps its own containment message, which the proof-ordering change restored.
* test(secondmate-safety): retarget the unemittable nested-home assertion
'contains registered secondmate home' exists nowhere in bin/, so no nested-home
specific refusal is reachable. The product instead fails closed on a registry it
cannot parse unambiguously, which this fixture's decoy 'mentions home:' line
produces and which is the same safety outcome: nothing is removed. Only that one
assertion changes; the safety pins it exists for - refusal, both homes retained,
both metas retained, no endpoint stopped - are untouched. The fixture also gains the
real home shape so the refusal is reached at all.
* test(secondmate-safety): retarget the child-registry nested-home assertion
Same unemittable grep as its sibling, but here the product has a message that states
this case's property exactly - "secondmate home still registers child homes" - so
that is the target rather than a fallback. The fixture also gains the real home shape
so the refusal is reached.
Partial: the case still fails on a LATER assertion, and it is a distinct finding.
require_empty_secondmate_registry runs at line ~4801, after endpoint quiescence, so
the endpoint is stopped before teardown discovers it will refuse - and the test
asserts no window is killed before this refusal. That is the same principle the
identity-drift case already pins ("endpoint was stopped before home identity was
proved") and the same shape as the proof-ordering decision: a read-only structural
proof that gates an irreversible step should run before it. Not changed here.
* fix(teardown): prove a childless registry before stopping a secondmate endpoint
Stopping a supervisor is irreversible - it cannot be un-stopped and its in-flight
context is gone - so a read-only proof that gates it must run first. A non-force
retirement of a home that still registers child homes was always going to refuse, but
quiesce_secondmate_endpoint ran before require_empty_secondmate_registry, so the
operator lost a live secondmate to a teardown that never had a chance of succeeding.
The proof now also runs before quiescence on the non-force path, and still runs at its
original place, so nothing is skipped. Non-force only, deliberately: on the --force
path cleanup_firstmate_home_children removes child registry entries before the later
check, so that check legitimately observes post-cleanup state and hoisting it there
would change what it observes rather than only when it runs.
* test(teardown): name the content-fallback lock through the product's deriver
The content-landed case hand-rolled its expected checkout-lock path by hashing
the raw physical common-dir path. The product lowercases that path first
(fm_checkout_stable_path_key ends in `print lc($path)`), so the two keys diverge
whenever the mktemp directory contains an uppercase character - which mktemp
templates essentially guarantee. The fake Treehouse then found no lock at the
named path and failed the return with exit 91, so the case failed at the return
step rather than on anything it was written to prove; every landed-work proof had
already passed.
Measured on a real repo: raw 1896fea0fa46324daaab18f9 vs product
68cbc9a67747a42792c3362a, and lowercasing alone accounts for the difference.
Delegated to the suite's checkout_lock_path helper, which the three sibling lock
cases already use and which resolves the common dir itself. The assertion that a
normal teardown holds the common checkout lock across the Treehouse return is
unchanged and still enforced.
* test(teardown): contend the shared checkout lock at the product's own path
Same drift as the content-fallback case, opposite symptom. This case plants a
held lock to prove the landing proof refuses on contention, but derived the lock
path by hashing the raw physical common-dir path, skipping the lowercasing in
fm_checkout_stable_path_key. The lock landed where nothing looks for it, so
teardown saw no contention and succeeded - the case failed as "expected exit 1,
got 0", reporting the absence of a safety refusal that had simply never been
triggered.
Now derived through the suite's checkout_lock_path helper, so the contention is
real and the refusal is genuinely exercised.
* test(secondmate-safety): give the active-home-descendant case its real home shape
The case proves a --force retirement refuses to remove a child worktree that
lives inside the active firstmate home. Its home was built with bare mkdir, so
home validation refused first with "secondmate project metadata is not an exact
repository root" and the child worktree was never inspected - the case failed
reporting a missing explanation for a rejection it never reached.
Rebuilt on make_secondmate_home_and_source plus
clone_registered_secondmate_project, matching the passing sibling cases, with the
child worktree still pointed inside the active home so the property under test is
unchanged. The asserted string is emitted by validate_removal_target
(bin/fm-teardown.sh:2172) and is now actually reached.
* test(teardown): match both porcelain spellings in the status-lock stub
add_git_status_lock_failure simulated a locked-index git status failure only when
the third argument was exactly --porcelain, but every dirty check in
fm-teardown.sh passes --porcelain=v1 --untracked-files=all. The stub had become a
silent no-op, so the stale-lock cleanup path was never entered: teardown's first
dirty check succeeded and it refused for uncommitted changes without ever
clearing the lock or reporting it.
Now matched in either spelling. The case's assertions are unchanged and it
genuinely exercises clearing a provably-stale lock and re-running the dirty check
afterwards. The helper has a single caller, so the wider match affects nothing
else.
* test: reach the repo-descendant and non-linked-lock cases through valid shapes
Two cases that never got as far as the behaviour they name.
The child-repo-descendant case built its home with bare mkdir, so home validation
refused on shape before the child worktree inside FM_ROOT was ever inspected.
Rebuilt on the real-clone helpers, hanging the child worktree off the fixture's
own source root so it is still inside FM_ROOT. The source root already ships the
quiet bin/fm-guard.sh the case hand-rolled and ignores data/, so the root stays
clean.
The non-linked index.lock case replaced its worktree with an independent clone,
which teardown refuses outright: validate_teardown_target_identity requires the
worktree and project to be distinct exact roots sharing one common Git dir, and a
standalone clone shares none. That invariant is worth keeping, so the fixture
changed instead - the worktree is now a clone's own primary checkout and the
project a linked worktree of that same clone. Identity is satisfied honestly and
`git rev-parse --git-path index.lock` still answers with the relative
.git/index.lock spelling, which is the only spelling that exercises the
worktree-anchored branch this case exists to pin.
* test(secondmate-safety): reach the unregistered-child proof through a real repo
Two stacked causes. The home was built with bare mkdir, so home validation
refused on shape first; and once that was repaired the case still failed, because
its child worktree was also a bare directory and
validate_child_worktree_removal_target proves "is an exact Git root" before it
proves "belongs to the child project". The case asserts the second message, so
the first was speaking instead.
The home now uses the real-clone helpers, and the child worktree is its own
initialised repository - which is what an unregistered child worktree actually
is. Its distinct common Git dir is what makes the belongs-to-project proof refuse,
so the case exercises the rule it names.
* test(spawn-dispatch-profile): make the seeded home's freshness proof hermetic
The secondmate-exemption case seeds a home cloned from this checkout, and
fm-spawn's verify-home resolves the LIVE upstream default tip of FM_ROOT through
probe_upstream's `git ls-remote --symref origin HEAD`. Unstubbed that escapes the
fixture to the real remote, so the home reads as stale whenever the checkout's
HEAD is ahead of origin/main - which is every unmerged branch, including any PR.
Isolated rather than assumed: the suite passes 17/17 on unmodified main, and
adding a single EMPTY commit on top of unmodified main reproduces the failure
exactly, so it tracks the checkout's position and not its content.
The fakebin now answers that one probe from this checkout's own default branch
and execs real git for everything else. Verified 17/17 both on this branch and on
main-plus-an-empty-commit, the previously failing shape. The product is unchanged;
comparing a real home against the live upstream tip is correct behaviour.
* no-mistakes(review): Make behavior-test guidance Herdr-safe
* no-mistakes(document): Document teardown safety ordering
* no-mistakes: apply CI fixes
* no-mistakes: apply CI fixes
* no-mistakes: apply CI fixes
* no-mistakes: apply CI fixes
* test(sealed-bridge): skip when no builder-valid system tool exists, do not stub the guard
The prior fix monkeypatched builder._require_safe_ancestry for the whole test
class. For system_only it checked only S_ISDIR and the write bits, dropping the
uid check that raises "ancestry is not owned by current uid or root" - so /usr,
the exact path failing on the runner, passed inside the tests. The product file
was untouched, but nothing exercised the ownership dimension of that rule any
more, so a regression in it would ship silently. Green obtained that way is a
false signal.
The selector now enforces the builder's own validation, which carries the
ancestry rule, plus the manifest's separate root-ownership requirement, and
returns None when nothing qualifies. setUp raises SkipTest with the reason.
Two alternatives were measured and rejected. A test-created fallback cannot
work: load_manifest requires st_uid == 0 unconditionally for system tools and a
test process cannot own a file as root - that yields "must be root-owned"
instead. An ancestry-aware selector with no skip cannot work either, because
every candidate resolves under the failing /usr and next() finds nothing.
Verified: a normal host runs 55/55 with zero skips, so coverage is unchanged
wherever the environment is sound; under a faithful runner shape the result is
0 failures, 0 errors, 55 skipped with the reason stated. A skip is visible and
explains itself; a stub is invisible. The product ancestry check is untouched.
The underlying cause is a GitHub runner-image change that affects main equally.
ruby-dlee
force-pushed
the
fm/crystallize-rules-c4
branch
from
July 31, 2026 18:48
7ed9126 to
04ca7f5
Compare
ruby-dlee
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Rebase PR 52's 48-hour operating-rules work onto repaired main, fold every pending and newly requested rule into its existing owner without bloat, structurally surface dependency-cleared queued work in teardown and heartbeat, preserve all guards, and deliver PR 52 genuinely green without merging it.
What Changed
Risk Assessment
✅ Low: The observed ready-set is now persisted after enqueueing in both normal and AFK heartbeat paths, closing the suppression race without weakening existing guards; no other material source risks were found.
Testing
Completed 1 recorded test check.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 2 issues found → auto-fixed ✅
tests/fm-secondmate-lifecycle-e2e.test.sh- merge conflict rebasing onto origin/fm/crystallize-rules-c4tests/fm-secondmate-liveness.test.sh- merge conflict rebasing onto origin/fm/crystallize-rules-c4🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-watch.sh:1304- A backlog mutation betweenbacklog_ready_additionsand this secondtasks-axi readyquery can permanently suppress work. For example, the first query detects newly ready B; before refresh, another completion makes C ready and removes B; the refresh stores C although the emitted wake names only B, so C is never reported later. Persist the already-observed snapshot after enqueueing, rather than re-querying, so concurrent additions remain detectable on the next heartbeat. The same issue exists in the AFK branch.🔧 Fix: Persist observed heartbeat ready snapshot
✅ Re-checked - no issues remain.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; uv run --directory tools/agent-fleet --locked pytest || rc=1; uv run --directory tools/agent-fleet --locked python -m compileall -q src || rc=1; exit "$rc"🔧 **Document** - 1 issue found → auto-fixed ✅
bin/fm-brief.sh:350- Generated crewmate guidance still says the captain owns all ask-user decisions, contradicting AGENTS.md’s new rule that firstmate resolves ordinary findings. This prose is embedded in executable shell output, so the documentation-only phase could not change it.🔧 Fix: Align generated decision triage guidance
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.