Repository navigation
fix: merge upstream main and reconcile fm-lint memory fallback with per-root source views - #16
Merged
Merged
Conversation
…--external-sources (kunchenguid#6443) * fix(lint): retry memory-bound roots without external sources * no-mistakes(review): Make fallback tests portable and correct source-following telemetry * no-mistakes(review): Remove committed parity fixtures and use disposable test roots * no-mistakes(document): Document ShellCheck memory fallback and telemetry * no-mistakes(review): Cover bounded and unbounded fallback RSS behavior * no-mistakes(document): Correct stale lint fallback documentation * no-mistakes(document): Correct stale lint test documentation * no-mistakes(ci): The memory fallback (the retry without --external-sources) now gets only the time left in its root's original deadline, so it can no longer outlast the CI job. Invariant: one root's first attempt plus its fallback must fit inside a single FM_LINT_ROOT_SECONDS deadline, plus the cleanup grace. Only one site started a new deadline: the fallback call in fm_lint_run_root. The deadline is the only budget involved, because the memory limit already applies to each process separately. Changes in bin/fm-lint.sh: - fm_lint_exec_root now takes a <seconds> argument instead of always reading FM_LINT_INTERNAL_ROOT_SECS. - The first attempt passes the full deadline. - The fallback passes floor((start + deadline - now) / 1000) seconds. - When bounds are enforced and less than 1 second is left, no retry starts. fm_exec_timed rejects 0 seconds, so the retry cannot run with no time. The root keeps reason=memory, and the shard output says "no time left in its Ns deadline to retry without it". - Unbounded local runs have no deadline and behave as before. - The header comment now describes the shared deadline. Changes in tests/fm-lint.test.sh: a new test, test_memory_fallback_spends_only_the_remaining_root_deadline, runs only on hosts that can enforce bounds. It uses a 6 s deadline and 1 s grace. - Case 1: the first attempt runs 3 s and then fails with memory status 251. The test asserts one fallback ran, reported reason=timeout, and the root's recorded duration is under 7000 ms. - Case 2: the first attempt runs 5.2 s. The test asserts no fallback starts, the skip is explained, and the sidecar records memory with source-following 1. Verification: - Full `nice -n 10 bash tests/fm-lint.test.sh` passed, including the new test, in about 5 minutes. - Case 1 run against the HEAD script: the root took 9168 ms, so the under-7000 ms check fails before the fix. - `bin/fm-lint.sh bin/fm-lint.sh tests/fm-lint.test.sh` reported no findings. - The CI workflow is unchanged, so the Test step still runs only tests/fm-lint.test.sh with nice -n 10 and the 12 GiB ShellCheck limit
…tension log opt-in (kunchenguid#5489) * Fix Pi watcher successor-gap confirmations and add extension log Accept an already-acknowledged handling confirmation as a no-op when the generation matches, confirm the restoration's own recovery token with a superseded (not rejected) outcome on generation mismatch, retire an arm on confirm failure only when the failed token names that exact pid, and record restore attempts, readiness timeouts, and confirm results in the bounded state/.watch-extension.log. Regression tests: already-acked no-op plus mismatch/dead-pid/lock-mismatch rejections and the manual-restart churn contract in fm-watch-arm.test.sh, and a mid-restore marker advance with no rejection appendix in fm-pi-watch-extension.test.sh. * Treat a dead arm child as an empty slot so repair and retry recover startArm and scheduleRetry answered unchanged while holding a ChildProcess whose OS process was already gone but whose close had not fired, so neither the repair tool nor the retry timer started anything until that close fired. Gate slot occupancy on a liveness check (exit/signal codes plus pid probe) and start a fresh arm instead, with a regression test driving the repair tool against a dead-but-unclosed child. * no-mistakes(document): Document new Pi extension log knob * no-mistakes(review): Fix confirm-failure retire token match, add distinct-pid test * no-mistakes(document): Clarify retire guard needs pid and generation * Make the Pi extension diagnostic log opt-in and default-off Only a positive FM_WATCH_EXTENSION_LOG_KEEP_LINES enables state/.watch-extension.log. Unset, empty, non-numeric, zero, and negative values disable logging entirely, so the default run writes nothing and never creates the file. The shared positiveInteger fallback semantics stay untouched for the retry and timeout knobs. docs/configuration.md owns the knob contract and docs/watcher-continuity.md points at it. Tests: the superseded-delivery case runs opted in, and a new case proves unset, zero, and non-numeric values create no log file while delivery still succeeds. * no-mistakes(document): Qualify extension-log coverage bullet as opt-in * no-mistakes(ci): The two reported checks (CI run 36372002913, Require no-mistakes run 36372069208) show conclusion action_required with 0 jobs and no logs because this is a fork PR (RibatTRW/firstmate) and GitHub is holding the workflow runs awaiting maintainer approval; that gate is external to the code and needs a maintainer to approve the runs. While verifying the change locally I found a real defect the approved CI run would hit: the PR's new test test_handling_delivered_rejects_a_superseded_generation failed deterministically. Invariant violated: reopen-announced (a non-successor/manual arm start) mints a fresh recovery generation only when the durable wake queue holds unrecovered work; an announced episode with an empty queue must be left untouched so idle arm starts never churn generations (the [ -s queue ] guard from kunchenguid#4819, relied on by bin/fm-watch.sh:2413 and covered by the append-reopens and announcement-bound sibling tests). The test called reopen with an empty queue and expected churn, so the fix establishes the queued-work precondition (append one wake, re-announce, re-read the generation) before asserting the reopen mints and the old confirmation mismatches. Test-only change, 14 lines in tests/fm-watch-arm.test.sh. Verified: fm-watch-arm.test.sh 25/25 ok on two consecutive runs, fm-pi-watch-extension.test.sh 55/55 ok, and shellcheck reports only one pre-existing warning outside the edited region * no-mistakes(document): Restore blank line in watcher-continuity docs * Route superseded Pi deliveries like confirmed ones and cover the retire guard A superseded handling confirmation now falls through to the normal delivery path, so an accepting supervision branch owns the wake instead of main. Scope the watcher-continuity token-pinned confirmation and narrowed retire rule to Pi, since omp and OpenCode still confirm against the current successor. Add tests that fail when the retire guard, the scheduled-retry gate, or the deferred-close gate is reverted, relabel the churned-generation characterization test, and use a reaped pid for the dead-pid rejection.
…-lint # Conflicts: # bin/fm-lint.sh # docs/fm-test-portable-shards.md
…tures for old/current resolver APIs, extended only Herdr recovery lock waits while preserving ordinary fallback timing, and covered all sibling renderer consumers. Herdr presentation E2E, Pi branch tests, lint, syntax, and diff checks pass. Calm renderer regression now passes; later local DOM checks were blocked only by unavailable Chrome
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
3 participants
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
The scheduled fork sync ("scheduled job for every 2 hours fork and pull firstmate from original") has stopped: since 2026-10-02 23:00 UTC every run reports "upstream sync CONFLICT: upstream/main does not merge cleanly into the fork's main; a human decision is needed". The merge of upstream/main (e31bc6e at the time of writing) into the fork's main (9601909) conflicts in bin/fm-lint.sh and docs/fm-test-portable-shards.md. The fork changed both in its own PR #6 (commit 590af2c: per-root lint source views and up to four workers, to cut lint wall time), and upstream has since changed the same files.
What Changed
bin/fm-lint.shanddocs/fm-test-portable-shards.md. The fork's per-root source views and up-to-four-worker layout are kept, and upstream's memory fallback is layered on top: a source-following root that hits the memory ceiling is retried once without--external-sources(cross-file codes SC1091, SC2034, SC2153, SC2329 excluded) inside the time left in that root's original deadline. The sharedfm_lint_exec_roothelper now takes the root's source view, so the first attempt runs from its private view while the no-source retry runs in the repository itself. A clean retry is reported asmemory-fallback, and the roots log and telemetry record whether the final attempt followed sources..pi/extensions/fm-primary-pi-watch.tsrestores watcher continuity across successor gaps, its diagnostic log becomes opt-in viaFM_WATCH_EXTENSION_LOG_KEEP_LINES(documented indocs/configuration.md), andbin/fm-wake-lib.shnow treats begin-handling on an already-acked recovery episode as a no-op when the caller names its generation.tests/fm-lint.test.sh,tests/fm-pi-watch-extension.test.sh,tests/fm-watch-arm.test.sh,docs/watcher-continuity.md, and the lint-partition section ofdocs/fm-test-portable-shards.md.Risk Assessment
✅ Low: The merge resolves both conflicted files by keeping the fork's source views and 1-4 workers alongside upstream's memory fallback, with no behavior dropped from either side and no defect found in the hand-written join; the remaining six changed files are clean auto-merges of upstream content that I did not read line by line.
Testing
Reproduced the reported sync conflict on the base commit with git merge-tree and confirmed the target merges upstream/main cleanly and contains it. Drove the real bin/fm-lint.sh with the pinned ShellCheck 0.11.0 on real repository roots: a 512 MiB cap forced a genuine memory failure on a relative root, and in both bounded and unbounded modes the first attempt ran in the private source view, the retry ran in the repository without source following, and the run passed with the fallback disclosed. A fixture root with a real defect still failed lint under the fallback, diagnostics were byte-identical with the view on or off and at 4 or 1 workers, --jobs 5 and 0 were refused, and the default changed-file lint passed. tests/fm-lint.test.sh passes except one upstream RSS assertion that needs /usr/bin/time, which this host lacks; the tests after it were run from a temporary copy with only that assertion skipped and all passed. The two merged upstream watcher test files also pass. These automated test runs are not live scenarios and are recorded as untested. No visual evidence applies - this is a CLI-only change.
FM_LINT_REQUIRE_BOUNDS=1 bin/fm-lint.sh --partition 1of2and2of2in CI.Evidence: Sync merge before and after
Source: Sync merge before and after
$ git merge-tree --write-tree --name-only 9601909 e31bc6e # BEFORE CONFLICT (content): Merge conflict in bin/fm-lint.sh CONFLICT (content): Merge conflict in docs/fm-test-portable-shards.md exit=1 $ git merge-tree --write-tree --name-only 2909183 e31bc6e # AFTER 86d338a8db7d12c8c6b1ff084e04e37ac62c9ba9 exit=0 $ git rev-list --count 2909183..e31bc6e (upstream commits still missing) 0Evidence: Source view plus memory fallback, real ShellCheck
Source: Source view plus memory fallback, real ShellCheck
$ FM_LINT_REQUIRE_BOUNDS=1 FM_LINT_ROOT_MEMORY_KIB=524288 bin/fm-lint.sh --jobs 1 --telemetry lint.tsv bin/fm-wake-lib.sh fm-lint: bin/fm-wake-lib.sh hit the memory ceiling with --external-sources (reason=memory rc=251); retried without it; fallback passed with cross-file codes excluded (SC1091,SC2034,SC2153,SC2329) exit=0 --- ShellCheck invocations (cwd + flags), in order: cwd=/tmp/fm-lint.TwnNEN/output/view.0 flags=--norc --external-sources -- bin/fm-wake-lib.sh cwd=<worktree> flags=--norc --exclude=SC1091,SC2034,SC2153,SC2329 -- bin/fm-wake-lib.shEvidence: Adversarial defect under fallback, view parity, worker bound
Source: Adversarial defect under fallback, view parity, worker bound
fm-lint: bin/zz-defect.sh hit the memory ceiling with --external-sources (reason=memory rc=251); retried without it; fallback reason=findings rc=1 In bin/zz-defect.sh line 5: rm -r $target ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting. exit=1 diagnostics byte-identical with and without the source view, and at 4 vs 1 workers $ bin/fm-lint.sh --jobs 5 bin/fm-lint.sh fm-lint.sh: jobs must be between 1 and 4, got 5. exit=2Evidence: Default changed-file lint on this branch
Source: Default changed-file lint on this branch
Evidence: tests/fm-lint.test.sh transcript (host RSS assertion failure and remaining tests)
Source: tests/fm-lint.test.sh transcript (host RSS assertion failure and remaining tests)
Follow-up
The Test step was approved with a host-limitation exception for
tests/fm-lint.test.sh:1649(test_memory_failure_retries_without_external_sources).That assertion comes unchanged from upstream commit d719ef3 and requires GNU
/usr/bin/time, which the validating host lacks; it is unrelated to this merge, and CI runners have GNU time.No fork-only guard was added, so the fork does not gain another divergence from upstream.
Guarding the assertion on
[ -x /usr/bin/time ]is filed as a follow-up for the upstream template repository.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-lint.sh:339- The only logic this merge authored itself is the join of the fork's per-root source view with upstream's memory fallback: fm_lint_exec_root gained a <view-or-empty> argument, the first attempt runs in the view, the view is removed, and the no-source retry runs with an empty view in the repository. Traced for a relative root (bin/foo.sh, view built, first attempt exits 251, retry runs from ROOT with FM_LINT_INTERNAL_VIEW empty in bounded mode and no cd in unbounded mode) and for an absolute root (no view on either attempt); both are correct, and diagnostics keep the same relative paths. No existing test exercises the combined path, because upstream's fallback tests use absolute fixture paths (never a view) and the fork's view test never triggers a memory failure. Nothing to fix; noted only as the one path covered by reading rather than by a test.tests/fm-lint.test.sh:1649- test_memory_failure_retries_without_external_sources fails on any host without /usr/bin/time: it requires a numeric per-root RSS in bounded mode, but bin/fm-lint.sh reports 'unavailable' when GNU time is absent, and the file stops there so the 23 tests after it never run. The assertion came in unchanged from upstream commit d719ef3 and is not caused by the merge resolution; CI runners have /usr/bin/time. I did not patch it: a fork-only edit to this upstream test adds another divergence that could stall the sync again. Options: guard the assertion on[ -x /usr/bin/time ]in the fork, send that guard upstream, or install GNU time on this host.FM_LINT_REQUIRE_BOUNDS=1 bin/fm-lint.sh --partition 1of2and2of2in CI.git merge-tree --write-tree --name-only 9601909 e31bc6e(base: conflicts in bin/fm-lint.sh and docs/fm-test-portable-shards.md, exit 1)git merge-tree --write-tree --name-only 2909183 e31bc6e(target: clean, exit 0),git merge-base --is-ancestorfor both parents,git rev-list --count 2909183..e31bc6e= 0FM_LINT_REQUIRE_BOUNDS=1 FM_LINT_ROOT_MEMORY_KIB=524288 bin/fm-lint.sh --jobs 1 --telemetry <tsv> bin/fm-wake-lib.shwith a pass-through shim logging ShellCheck cwd and flagsbin/fm-lint.sh --jobs 1 --telemetry <tsv> bin/fm-wake-lib.shunbounded, real ShellCheck process capped at 512 MiB by the shimFM_LINT_REQUIRE_BOUNDS=1 bin/fm-lint.sh --jobs 1 --telemetry <tsv> bin/fm-wake-lib.shcontrol at the default 12 GiB capFixture copy of bin/ plus bin/zz-defect.sh (SC2086):FM_LINT_REQUIRE_BOUNDS=1 FM_LINT_ROOT_MEMORY_KIB=524288 bin/fm-lint.sh --jobs 1 bin/zz-defect.shbin/fm-lint.sh --jobs 4 bin/zz-defect.shvsFM_LINT_DEDUPE_SOURCES=0 bin/fm-lint.sh --jobs 1 bin/zz-defect.shcompared withcmpbin/fm-lint.sh --jobs 4|5|0 bin/fm-lint.shandbin/fm-lint.sh --helpbin/fm-lint.sh --list-filesandbin/fm-lint.sh(default changed-file mode plus workflow validation)bash tests/fm-lint.test.sh(26 pass, then stops at the RSS assertion in test_memory_failure_retries_without_external_sources)Remaining 24 tests of tests/fm-lint.test.sh from a temporary copy with only the RSS-digits assertion skipped (all pass; copy removed)bash tests/fm-watch-arm.test.sh(30 pass)bash tests/fm-pi-watch-extension.test.sh(60 pass)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.