fix(bin): deliver session-start digest on fresh clones - #4076
Closed
GTiberi wants to merge 5 commits into
Closed
Conversation
…ier gate fm-sessionstart-run.sh (used by every "run tier" harness's session-open hook, including Claude via fm-claude-compat-hook.sh and Copilot via fm-copilot-hook.sh) stands down silently whenever fm_primary_scope_matches finds no state/ directory yet. On a genuinely fresh `git clone` there is no state/ directory at all, so the very first session-open ever hits this gate before fm-session-start.sh (the script that would create state/) ever runs. The digest never reaches the model on that first launch, and only a capable model noticing AGENTS.md's own "confirm the digest is present ... run it yourself when it is not" fallback recovers. Verified live: manually invoking fm-sessionstart-run.sh --copilot with source=new against a fresh clone with no state/ directory produced zero output before this change, and the full 141-line digest after it. Add fm_primary_scope_shape_matches, the same shape check minus the state-directory requirement, and have fm-sessionstart-run.sh create state/ before the gate only when that shape already proves this is a genuine primary root (plain top-level checkout or marked secondmate home, with AGENTS.md and bin/ present) -- never for an unrelated repo that merely happens to share that file layout. Every other caller of fm_primary_scope_matches (turnend-guard, subagent-pretool-check, claude-stop-autoarm, the nudge wrapper) is unchanged, since the existing tests for all of them still pass unmodified. Add a regression test: a fresh clone with no state/ directory at all gets the full digest on its first run-tier session start. Not specific to GitHub Copilot CLI: bin/fm-primary-scope-lib.sh and bin/fm-sessionstart-run.sh predate PR kunchenguid#3588 and are unchanged by it; this fix surfaced only because testing Copilot required standing up a genuinely fresh scratch firstmate home rather than reusing an already-initialized one. Suitable to offer upstream as-is.
 The PR is not yet safe to merge because genuine shallow Firstmate clones still silently miss their first session-start digest. Findings
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| # Return 0 when $1 descends from the Firstmate history anchor above. | ||
| fm_primary_scope_is_firstmate_checkout() { | ||
| git -C "$1" merge-base --is-ancestor "$FM_PRIMARY_SCOPE_HISTORY_ANCHOR" HEAD 2>/dev/null |
There was a problem hiding this comment.
When a genuine fresh Firstmate checkout is created with shallow history that omits 40c50ea8843c5b6a5351db8352675537252b653e, git merge-base --is-ancestor rejects it, so the wrapper leaves state/ absent and silently exits without delivering the first session-start digest.
Context Used: If there is a VISION.md file at the root of the re... (source)
Author
|
Withdrawing this for now; keeping the change on my fork rather than pursuing it upstream. Thanks for the review. |
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
Offer a fresh-clone session-start digest fix to upstream firstmate as its own pull request, through this repo's validation pipeline (which is configured to push to a fork and open the PR against upstream main).
The fix (commit 60a635c on a maintained fork) is cherry-picked cleanly onto the current upstream main tip with no drift, touching bin/fm-primary-scope-lib.sh, bin/fm-sessionstart-run.sh, and tests/fm-sessionstart-nudge.test.sh (one new regression test).
Root cause this fixes: fm-sessionstart-run.sh is the shared entry point every "run tier" harness's session-open hook calls (Claude via fm-claude-compat-hook.sh, and others). It stands down silently whenever fm_primary_scope_matches finds no state/ directory yet. A genuinely fresh git clone has no state/ directory at all, so the very first session-open ever hits this gate before fm-session-start.sh (the script that would create state/) ever runs, and the digest never reaches the model on that first launch. This is not specific to any one harness: fm-primary-scope-lib.sh and fm-sessionstart-run.sh predate and are unrelated to any recent harness-specific work; the bug was found incidentally while verifying a different harness integration on a fork, because that testing required a genuinely fresh scratch firstmate home rather than reusing an already-initialized one. Verified live in that fork work: manually invoking fm-sessionstart-run.sh against a fresh clone with no state/ directory produced zero output before the fix and the full digest after it.
The fix adds fm_primary_scope_shape_matches (the same shape check minus the state-directory requirement) and has fm-sessionstart-run.sh create state/ before the gate, but only when that shape already proves the root is a genuine primary root (plain top-level checkout or marked secondmate home, with AGENTS.md and bin/ present) -- never for an unrelated repo that happens to share that file layout. Every other caller of fm_primary_scope_matches (turnend-guard, subagent-pretool-check, claude-stop-autoarm, the nudge wrapper) is unchanged, and their existing tests still pass unmodified.
PR requirements: the PR body must explain the fresh-clone failure mode, state plainly that it is harness-independent (affects every run-tier harness routing through fm-sessionstart-run.sh, this session's own harness included), summarize the live-evidence verification (zero output before the fix, full digest after, on a fresh clone with no state/ directory), and note it was found while verifying a different harness's support on a fork -- without naming any private task records, paths, or the other harness by name beyond what is already in this description. Do not mention or link PR #3588, and do not open any pull request other than this one.
Validation already performed before this run: bin/fm-lint.sh is clean (pinned ShellCheck 0.11.0 and actionlint 1.7.12, both installed locally since absent from this sandbox by default). tests/fm-sessionstart-nudge.test.sh passes in full, including the new regression test. bin/fm-test-run.sh --changed --base origin/main reports 4 failures, and every one of them was independently reproduced on a clean, unmodified origin/main checkout in a separate worktree, proving each is a pre-existing sandbox/environment or pre-existing-bug condition unrelated to this change, not something this change caused: tests/fm-test-run.test.sh fails because ruby is not installed in this sandbox (needed to parse a workflow YAML file); tests/fm-calm-pi-extension.test.sh fails because Chrome/Chromium is not installed in this sandbox; tests/fm-pi-primary-types.test.sh fails on pre-existing TypeScript type errors in files this change does not touch (fm-branch-supervision.ts, fm-primary-pi-watch.ts, lib/fm-async-exec.ts); tests/fm-session-start.test.sh fails on a pre-existing bug unrelated to this change (a locked session start not publishing the home-summary ledger). These four should be reported as pre-existing/environment gaps rather than fixed as part of this task, since fixing them is out of scope for this focused upstream offer.
What Changed
state/only for a verified Firstmate root before applying the normal scope gate; unrelatedAGENTS.md/bincheckouts remain untouched.bin/fm-sessionstart-run.sh, including this session's own harness: a fresh clone with nostate/previously produced zero output beforefm-session-start.shcould create it, while live verification produced the full digest after the fix.Risk Assessment
✅ Low: The change is a small, idempotent state-directory initialization gated by primary-root checks plus the session-start marker, with behavioral regression coverage for both the fresh-clone and unrelated-repository paths.
Testing
From a clean worktree, focused hook-consumer suites exercised the restored shared scope contract. A real fresh local clone with no state directory reproduced the user-visible behavior end-to-end: the base emitted zero stdout/stderr and created no state, while the target emitted the complete run-tier session-start digest and created
state/.lock; the direct CLI transcript is retained as evidence.Evidence: Fresh-clone run-tier session-start before/after CLI transcript
Source: Fresh-clone run-tier session-start before/after CLI transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
bin/fm-sessionstart-run.sh:97- The required criterion says creation must occur “only when that shape already proves the root is a genuine primary root ... -- never for an unrelated repo that happens to share that file layout.” That invariant is not met:fm_primary_scope_shape_matchesaccepts any plain Git checkout withAGENTS.mdandbin/. A normal invocation withFM_ROOT_OVERRIDE=/tmp/otherandFM_HOME=/tmp/other, where/tmp/otherhas that layout but nostate/, reaches this hunk, creates/tmp/other/state, and runs the digest. Establish a verifiable Firstmate-root identity at the shared scope boundary before permitting this new mutation.commit 515786e55b7dc64d0df490f5c2f06a6f9c31a864 message:32- The intent explicitly says “Do not mention or link PR feat(harness): add GitHub Copilot CLI support #3588,” but the new commit message adds the hunk “bin/fm-sessionstart-run.sh predate PR feat(harness): add GitHub Copilot CLI support #3588.” The commit message will be visible on the proposed pull request, so amend it to remove the forbidden reference before delivery.🔧 Fix: Harden fresh-clone state creation with Firstmate marker
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed ✅
bin/fm-primary-scope-lib.sh:38-fm_primary_scope_matchesnow additionally requiresbin/fm-session-start.sh, changing the established scope contract for non-session-start callers. On the target,tests/fm-subagent-pretool-check.test.shallows the delegation toolTask(exit 0) instead of denying it (exit 2); the identical test succeeds from base commit40c50ea. Keep the session-start executable check specific to the run wrapper's state-creation decision so existing callers retain their prior predicate behavior.bash tests/fm-sessionstart-nudge.test.shManual differential session-open reproduction using separate actualgit clone --no-localfixtures at base40c50ea8843c5b6a5351db8352675537252b653eand target:fm-sessionstart-run.sh --source startupunder a codex-named harness parent, with no initialstate/directory.bash tests/fm-subagent-pretool-check.test.shon the target (fails:Taskis allowed rather than denied).Baseline comparison:git clone --no-local . <baseline> && git -C <baseline> checkout 40c50ea8843c5b6a5351db8352675537252b653e && bash <baseline>/tests/fm-subagent-pretool-check.test.sh.🔧 Fix: Restored primary scope compatibility
✅ Re-checked - no issues remain.
tests/fm-sessionstart-nudge.test.shtests/fm-subagent-pretool-check.test.shtests/fm-turnend-guard.test.shtests/fm-claude-stop-autoarm.test.shFresh-clone end-to-end comparison:git clone --quiet --no-hardlinks "$PWD" <temp>/firstmate; checked out40c50ea8843c5b6a5351db8352675537252b653eand4f49a34d668969414bb0390abd95668cbb35f083in turn; invokedbin/fm-sessionstart-run.sh --source startupfrom acodex-named harness with nostate/directory.git status --shortandtest ! -d stateafter validation✅ **Document** - passed
✅ No issues found.
⏭️ **Lint** - skipped
✅ **Push** - passed
✅ No issues found.