feat: support guarded local merge targets#718
Open
waleedarafa wants to merge 20 commits into
Open
Conversation
Co-authored-by: Waleed Arafa <waleedarafa@AELT-MAC885.local>
waleedarafa
force-pushed
the
fm/fm-merge-target-support-guarded-local-merge-into-an-appr-f5
branch
2 times, most recently
from
July 19, 2026 08:41
75308dd to
c9fc942
Compare
… targets accurately
waleedarafa
force-pushed
the
fm/fm-merge-target-support-guarded-local-merge-into-an-appr-f5
branch
from
July 19, 2026 08:50
e5dff3c to
5baf84f
Compare
waleedarafa
force-pushed
the
fm/fm-merge-target-support-guarded-local-merge-into-an-appr-f5
branch
from
July 19, 2026 10:52
5baf84f to
6b44bc3
Compare
4 tasks
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
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 developer wanted Firstmate’s guarded local-only merge helper to accept an explicit existing local target branch, while leaving its default behavior of fast-forwarding the project’s default branch unchanged and preserving all existing safety checks. The implementation had to validate the target and task branches rigorously, require the main local checkout to be clean and on the exact target, use only fully guarded ff-only local Git operations, reject malformed or injection-shaped arguments, and add isolated regression coverage proving refusals never mutate repositories. Follow-up review requirements expanded the change to persist the selected target only after a successful fast-forward, make teardown validate containment against that recorded target, emit accurate completion history for non-default targets while retaining legacy “local main” behavior, and safely update validated metadata atomically. The developer also required focused and repository-wide validation, ShellCheck cleanliness, no changes to the real Secondbrain repository, no agent co-author, and delivery through the no-mistakes PR workflow without merging; after push and pipeline recovery issues, they explicitly required verifying and fast-forwarding the local branch to the known pipeline commit before rerunning validation without --yes.
What Changed
--targetsupport to guarded local-only merges while preserving default-branch behavior and enforcing clean, checked-out, fast-forward-only landing.Risk Assessment
✅ Low: Captain, the prior command-injection path is fully addressed by validating derived targets and using the project’s established shell-quoting helper; the remaining change is well-bounded and guarded.
Testing
The supplied repository-wide baseline and focused merge/teardown suites passed; isolated end-to-end CLI fixtures demonstrated explicit-target landing, unchanged default-main behavior, atomic metadata persistence, teardown history against the recorded target, and non-mutating safety refusals. The retained transcript provides reviewer-visible evidence; no screenshot was applicable because this is a shell CLI change.
Evidence: End-to-end local merge CLI transcript
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-teardown.sh:418-LOCAL_MERGE_TARGETis interpolated into a shell command without shell quoting. Explicit targets reject metacharacters, but default targets bypass that guard; a valid default branch such asrelease$(touch${IFS}/tmp/p)can therefore be recorded and execute command substitution when this suggested command is run. Apply identical validation to derived targets and shell-quote the note argument.🔧 Fix: Harden local merge target validation and command quoting
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
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; exit "$rc"Configured repository-wide baseline: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; exit "$rc"Focused regression suites:bash tests/fm-merge-local.test.sh && bash tests/fm-teardown.test.shEvidence harness:chmod +x /var/folders/kl/9kq9q45504qd48wr0y8h1tnw0000gp/T/no-mistakes-evidence/01KXVSAN2B5CE3E18N2S5W6BYC/run-e2e-evidence.sh && /var/folders/kl/9kq9q45504qd48wr0y8h1tnw0000gp/T/no-mistakes-evidence/01KXVSAN2B5CE3E18N2S5W6BYC/run-e2e-evidence.sh > /var/folders/kl/9kq9q45504qd48wr0y8h1tnw0000gp/T/no-mistakes-evidence/01KXVSAN2B5CE3E18N2S5W6BYC/local-merge-e2e-transcript.txt 2>&1Manual fixture invocations exercisedbin/fm-merge-local.sh task-x1 --target feature/for-you-feed, its no-target default, injection-shaped input, wrong-checkout and dirty-target refusals, followed bybin/fm-teardown.sh task-x1with the repository’s documentedFM_GATE_REFUSE_BYPASS=1test exemption.🔧 **Document** - 1 issue found → auto-fixed (2) ✅
tests/fm-brief.test.sh:88- Captain, this assertion still requires the obsolete local-main wording; updating it is necessary but prohibited by this documentation-only task.🔧 Fix: Verify local merge target documentation
1 warning still open:
tests/fm-brief.test.sh:88- Captain, this assertion still expects obsoletelocal mainwording; fixing it requires changing a test, which this documentation-only task explicitly prohibits.🔧 Fix: Update target-aware local merge assertion
✅ Re-checked - no issues remain.
🔧 Fix: Confirm pinned ShellCheck lint passes cleanly
1 warning still open:
✅ **Push** - passed
✅ No issues found.