Conversation
added 3 commits
August 10, 2026 16:56
The reconcile merge (f7b7cff) dropped the fork's --resolve-key flag initialization and parsing loop, causing fm-send to fail with 'RESOLVE_KEYS: unbound variable' error on any send. Restore: - RESOLVE_KEYS initialization - fm_send_add_resolve_key function - --resolve-key flag parsing loop This was a semantic merge conflict where fork logic depended on initialization code that was lost when the merge preferred fork code but not the initialization that precedes it. Fixes: tests/fm-send-settle.test.sh, tests/fm-send-strict.test.sh
…cision closure path
…ady documented in the authoritative script header.
📝 WalkthroughWalkthroughAdded Claude lifecycle hooks for task state updates, changed-file linting helpers, repeatable resolution-key handling in ChangesShell workflow updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeSession
participant LifecycleHooks
participant fmBusyEvent as fm-busy-event.sh
participant TurnMarker as turn-ended marker
ClaudeSession->>LifecycleHooks: Submit prompt
LifecycleHooks->>fmBusyEvent: Mark task busy
ClaudeSession->>LifecycleHooks: Stop, stop failure, or session end
LifecycleHooks->>fmBusyEvent: Mark task idle
LifecycleHooks->>TurnMarker: Create marker on stop
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
trillium
force-pushed
the
fm/fix-main-ci-fm-send
branch
from
August 11, 2026 00:24
b129e97 to
295c0db
Compare
The reconcile merge dropped two critical functions: - fm_lint_changed_base_ref: determines the base ref for diff - fm_lint_is_canonical_root: validates file membership Also initialize CHANGED_MODE=0 to prevent unbound variable errors with set -u. These functions are needed for the changed-file detection logic when linting branches before main.
trillium
force-pushed
the
fm/fix-main-ci-fm-send
branch
from
August 11, 2026 00:41
295c0db to
eb3454f
Compare
The reconcile merge left an unmatched fi statement on line 2199 that closed nothing. The case statement ends with esac on line 2198, and there is no corresponding if condition before it. Removing this orphaned fi resolves the shell syntax error.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/settings.local.json:
- Line 1: Update the Stop hook command in the UserPromptSubmit/Stop
configuration so the idle lifecycle update runs only when creating the
.turn-ended marker succeeds. Preserve the existing marker path and
fm-busy-event.sh invocation, but chain the commands with failure propagation
rather than allowing a failed touch to continue or be masked by || true.
- Line 1: Remove the tracked generated local hook file represented by
.claude/settings.local.json from version control, while preserving the existing
ignore rule so future local copies remain untracked.
In `@bin/fm-send.sh`:
- Around line 343-346: Update the --resolve-key option handling to reject
reserved option tokens --key and --resolve-key before calling
fm_send_add_resolve_key, while retaining existing argument validation and exit
behavior. Add coverage for both reserved-token cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ba27cea-fad7-497c-960e-d00359baa7f0
📒 Files selected for processing (4)
.claude/settings.local.jsonbin/fm-lint.shbin/fm-send.shbin/fm-spawn.sh
💤 Files with no reviewable changes (1)
- bin/fm-spawn.sh
| @@ -0,0 +1 @@ | |||
| {"hooks":{"UserPromptSubmit":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' busy --gen 'g1786408690.89755.6034' --source claude-hook --event user-prompt-submit 2>/dev/null || true"}]}],"Stop":[{"hooks":[{"type":"command","command":"touch '/Users/trilliumsmith/code/firstmate/state/fix-main-ci-fm-send.turn-ended'; '/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event stop 2>/dev/null || true"}]}],"StopFailure":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event stop-failure 2>/dev/null || true"}]}],"SessionEnd":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event session-end 2>/dev/null || true"}]}]}} | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve failure when creating the turn-ended marker.
The Stop hook uses touch ...; fm-busy-event.sh ... || true. If touch fails, the command still applies the idle event and returns success. The downstream classifier then receives no .turn-ended marker, so wake handling can be lost.
Make marker creation a prerequisite for the lifecycle update:
Proposed fix
-touch '/Users/trilliumsmith/code/firstmate/state/fix-main-ci-fm-send.turn-ended'; '/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply ... || true
+touch '/Users/trilliumsmith/code/firstmate/state/fix-main-ci-fm-send.turn-ended' && '/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply ... || true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {"hooks":{"UserPromptSubmit":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' busy --gen 'g1786408690.89755.6034' --source claude-hook --event user-prompt-submit 2>/dev/null || true"}]}],"Stop":[{"hooks":[{"type":"command","command":"touch '/Users/trilliumsmith/code/firstmate/state/fix-main-ci-fm-send.turn-ended'; '/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event stop 2>/dev/null || true"}]}],"StopFailure":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event stop-failure 2>/dev/null || true"}]}],"SessionEnd":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event session-end 2>/dev/null || true"}]}]}} | |
| {"hooks":{"UserPromptSubmit":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' busy --gen 'g1786408690.89755.6034' --source claude-hook --event user-prompt-submit 2>/dev/null || true"}]}],"Stop":[{"hooks":[{"type":"command","command":"touch '/Users/trilliumsmith/code/firstmate/state/fix-main-ci-fm-send.turn-ended' && '/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event stop 2>/dev/null || true"}]}],"StopFailure":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event stop-failure 2>/dev/null || true"}]}],"SessionEnd":[{"hooks":[{"type":"command","command":"'/Users/trilliumsmith/code/firstmate/bin/fm-busy-event.sh' apply '/Users/trilliumsmith/code/firstmate/state' 'fix-main-ci-fm-send' idle --gen 'g1786408690.89755.6034' --source claude-hook --event session-end 2>/dev/null || true"}]}]}} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/settings.local.json at line 1, Update the Stop hook command in the
UserPromptSubmit/Stop configuration so the idle lifecycle update runs only when
creating the .turn-ended marker succeeds. Preserve the existing marker path and
fm-busy-event.sh invocation, but chain the commands with failure propagation
rather than allowing a failed touch to continue or be masked by || true.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if git ls-files --error-unmatch .claude/settings.local.json >/dev/null 2>&1; then
echo "settings.local.json is tracked"
else
echo "settings.local.json is not tracked"
fi
rg -n --hidden --glob '!.git' \
'(/Users/trilliumsmith/code/firstmate|fix-main-ci-fm-send|g1786408690\.89755\.6034|fm-busy-event\.sh arm|settings\.local\.json)' .Repository: trillium/firstmate
Length of output: 5341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracked file and status ---'
git ls-files --stage -- .claude/settings.local.json
git status --short -- .claude/settings.local.json
cat -n .claude/settings.local.json
printf '%s\n' '--- generator ---'
sed -n '2465,2510p' bin/fm-spawn.sh
printf '%s\n' '--- hook-related tests ---'
sed -n '1,130p' tests/fm-spawn-reused-worktree-hooks.test.sh
sed -n '1,80p' tests/fm-hook-artifacts-not-tracked.test.sh
printf '%s\n' '--- busy-event generation semantics ---'
sed -n '1,150p' bin/fm-busy-event.sh
rg -n 'fm-busy-event\.sh (arm|apply)|--gen|turn-ended' bin tests | head -160Repository: trillium/firstmate
Length of output: 33916
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- stale-generation validation ---'
sed -n '140,205p' bin/fm-busy-event.sh
printf '%s\n' '--- ignore rules ---'
sed -n '15,32p' .gitignore
git check-ignore -v .claude/settings.local.json || true
printf '%s\n' '--- tracked hook artifacts ---'
git ls-files -- \
.claude/settings.local.json \
.opencode/plugins/fm-busy-state.js \
.opencode/plugins/fm-turn-end.js \
.fm-grok-turnend \
.fm-kimi-turnend
printf '%s\n' '--- read-only contract verifier ---'
python3 - <<'PY'
import json
from pathlib import Path
settings = json.loads(Path(".claude/settings.local.json").read_text())
commands = []
for event, entries in settings["hooks"].items():
for entry in entries:
commands.append((event, entry["hooks"][0]["command"]))
expected_literals = {
"/Users/trilliumsmith/code/firstmate",
"fix-main-ci-fm-send",
"g1786408690.89755.6034",
}
found = {literal for literal in expected_literals if any(literal in cmd for _, cmd in commands)}
print("tracked_settings_json:", True)
print("hardcoded_literals_found:", sorted(found))
print("all_hooks_use_literal_root:", all("/Users/trilliumsmith/code/firstmate" in cmd for _, cmd in commands))
print("all_hooks_use_literal_task:", all("fix-main-ci-fm-send" in cmd for _, cmd in commands))
print("all_hooks_use_literal_generation:", all("g1786408690.89755.6034" in cmd for _, cmd in commands))
source = Path("bin/fm-spawn.sh").read_text()
contract = {
"uses_dynamic_root": 'shell_quote("$FM_ROOT/bin/fm-busy-event.sh")' in source,
"uses_dynamic_state": 'shell_quote("$STATE_REAL")' in source,
"uses_dynamic_task": 'shell_quote("$ID")' in source,
"uses_dynamic_generation": 'shell_quote("$BUSY_GEN")' in source,
"rewrites_settings": 'cat > "$WT/.claude/settings.local.json"' in source,
"excludes_settings": "exclude_path '.claude/settings.local.json'" in source,
}
print("generator_contract:", contract)
PYRepository: trillium/firstmate
Length of output: 3206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
source = Path("bin/fm-spawn.sh").read_text()
patterns = {
"uses_dynamic_root": r'shell_quote "\$FM_ROOT/bin/fm-busy-event\.sh"',
"uses_dynamic_state": r'shell_quote "\$STATE_REAL"',
"uses_dynamic_task": r'shell_quote "\$ID"',
"uses_dynamic_generation": r'shell_quote "\$BUSY_GEN"',
"rewrites_settings": r'cat > "\$WT/\.claude/settings\.local\.json"',
"excludes_settings": r"exclude_path '\.claude/settings\.local\.json'",
}
for name, pattern in patterns.items():
print(f"{name}:", bool(re.search(pattern, source)))
PYRepository: trillium/firstmate
Length of output: 307
Stop tracking this generated local hook file.
.claude/settings.local.json is ignored and must not be committed. Its tracked contents bind hooks to one checkout, task, and generation. Remove it from the repository and retain the existing ignore rule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/settings.local.json at line 1, Remove the tracked generated local
hook file represented by .claude/settings.local.json from version control, while
preserving the existing ignore rule so future local copies remain untracked.
Comment on lines
+343
to
+346
| --resolve-key) | ||
| [ $# -ge 2 ] || { echo "error: --resolve-key requires a key" >&2; exit 1; } | ||
| fm_send_add_resolve_key "$2" || exit 1 | ||
| shift 2 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'Option definitions:'
rg -n -C 4 -- '--resolve-key|--key' bin/fm-send.sh || true
printf '%s\n' 'Existing coverage:'
rg -n -C 2 --glob '*test*' --glob '*.sh' \
-- '--resolve-key|duplicate|requires a key' . || trueRepository: trillium/firstmate
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'fm-send.sh header and parser:'
sed -n '1,110p' bin/fm-send.sh
sed -n '300,475p' bin/fm-send.sh
printf '%s\n' 'Resolve-key and close-flow definitions/usages:'
rg -n -C 6 'fm_send_close_resolved_keys|RESOLVE_KEYS|decision key|valid decision key' \
bin tests --glob '*.sh' --glob '*test*' | head -n 240
printf '%s\n' 'fm-send-specific tests:'
fd -i 'fm-send' tests binRepository: trillium/firstmate
Length of output: 32867
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'Resolve-key tests:'
sed -n '1,430p' tests/fm-send-resolve-key.test.sh
printf '%s\n' 'Decision-key parsing and validation:'
rg -n -C 8 'status_open_decisions|key=.*A-Za-z|validate.*key|decision.*key|pending-reply-' \
bin/fm-classify-lib.sh bin/fm-pending-reply-lib.sh tests/fm-watch-triage.test.sh tests/fm-remote-reply.test.sh | head -n 320Repository: trillium/firstmate
Length of output: 42345
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
allowed = re.compile(r'^[A-Za-z0-9._-]+$')
def parse(args):
keys = []
i = 0
while i < len(args) and args[i] == '--resolve-key':
if i + 1 >= len(args):
raise ValueError('requires a key')
key = args[i + 1]
if not allowed.fullmatch(key):
raise ValueError('invalid key')
keys.append(key)
i += 2
return keys, args[i:]
def close_lines(keys, answer):
return [f'resolved [key={key}]: answered: {answer}' for key in keys]
for case in (
['--resolve-key', '--key', 'Enter'],
['--resolve-key', '--resolve-key', 'answer'],
):
keys, rest = parse(case)
print(f'input={case!r}')
print(f'accepted_keys={keys!r}')
print(f'remaining_args={rest!r}')
print(f'close_lines={close_lines(keys, "answer")!r}')
print(f'reserved_token_accepted={any(k.startswith("--") for k in keys)}')
PYRepository: trillium/firstmate
Length of output: 532
Reject reserved option tokens as --resolve-key values.
--key and --resolve-key pass the current key validation and can be persisted as resolved decision keys. Reject them before fm_send_add_resolve_key, and add tests for both cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bin/fm-send.sh` around lines 343 - 346, Update the --resolve-key option
handling to reject reserved option tokens --key and --resolve-key before calling
fm_send_add_resolve_key, while retaining existing argument validation and exit
behavior. Add coverage for both reserved-token cases.
Owner
Author
|
Superseded by #113 (merged), which restored the same fm-send --resolve-key parsing plus the full fork-behavior set. This branch is now dirty against the fixed main and its content already landed. Closing as redundant. |
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
Restore origin/main's CI to green by fixing the broken fm-send.sh after the upstream reconcile merge. The merge dropped the fork's --resolve-key flag parsing logic, causing fm-send to fail with unbound variable errors on all sends. Restore the missing initialization and parsing code while keeping the merge intact.
What Changed
--resolve-keyflag parsing logic with validation that validates key format (alphanumeric, dot, underscore, hyphen) and prevents duplicatesfm_pending_reply_carrier_bodyto strip carrier markers before closing resolved keys, preventing "unbound variable" errors on sendsRisk Assessment
🚨 High: The change breaks the --resolve-key feature's durable ledger contract, violates user intent to restore missing initialization code, and causes test failures. The workaround using MESSAGE instead of RESOLVE_ANSWER_TEXT is semantically incorrect because MESSAGE gets modified with correlation IDs after initialization.
Testing
Comprehensive test validation of fm-send --resolve-key functionality. All 10 resolve-key tests pass, covering decision closure, secondmate marker handling, error cases, and transport failures. Additional fm-send test suites (7 total) confirm no regressions in related functionality.
Evidence: fm-send-resolve-key test results
Evidence: all fm-send test suite results
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-send.sh:613- Undefined RESOLVE_ANSWER_TEXT variable will cause unbound variable error when using --resolve-key flag. Line 613 calls fm_send_close_resolved_keys with $RESOLVE_ANSWER_TEXT which is never initialized. This breaks the decision-resolution feature and will crash whenever someone tries to close a decision.🔧 Fix: Fix undefined RESOLVE_ANSWER_TEXT variable in decision closure path
1 error still open:
bin/fm-send.sh:469- Missing RESOLVE_ANSWER_TEXT initialization causes corr tokens to leak into decision closure ledger. The variable should be initialized after MESSAGE=$* to capture plain answer before MESSAGE is modified with correlation IDs. The automated fix changed line 613 to use MESSAGE as a workaround, but MESSAGE gets modified later (line 520) to include corr tokens, violating the semantic contract that closing lines contain plain answer text without marker or corr bytes. fm-send-resolve-key test fails with 'the closing line leaked the corr token' proving this. Proper restoration requires adding: RESOLVE_ANSWER_TEXT=$MESSAGE after line 468.✅ **Test** - passed
✅ No issues found.
bash tests/fm-send-resolve-key.test.sh (10/10 tests)bash tests/fm-send-popup-settle.test.shbash tests/fm-send-raw.test.shbash tests/fm-send-secondmate-marker.test.shbash tests/fm-send-settle.test.shbash tests/fm-send-strict.test.shbash tests/fm-send-verify-transition.test.sh✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes