From ef48247f9780754f52fdbf666c7a6073d09d6dcb Mon Sep 17 00:00:00 2001 From: Charles Brooker Date: Thu, 8 Oct 2026 16:40:41 -0500 Subject: [PATCH 1/3] fix(bin): keep a recorded PR parseable after relaunch Relaunch appended control_relaunch_tx after the preserved lines, so a task with pr= recorded ended with a key after its PR identity block. fm_pr_metadata_identity_parse rejects any such key and PR monitoring stopped silently. Write the transaction marker before the preserved lines so the pr= block stays last, and cover it with a relaunch test that parses the result. --- bin/fm-spawn.sh | 9 ++++++--- tests/fm-control-relaunch.test.sh | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 40c07e05887..6c6b6f6eb34 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -5509,12 +5509,15 @@ preserve_relaunch_meta() { echo "home=$PROJ_ABS" echo "projects=$SECONDMATE_PROJECTS" fi - if [ "$RELAUNCH" -eq 1 ]; then - preserve_relaunch_meta - fi + # Written before the preserved lines, never after: a pr= line's identity block + # must stay LAST in the record, because fm_pr_metadata_identity_parse rejects + # any other key following pr= and PR monitoring would silently stop. if [ "$SPAWN_CONTROL_PARENT" = 1 ] && [ -n "${FM_CONTROL_RELAUNCH_TX:-}" ]; then echo "control_relaunch_tx=$FM_CONTROL_RELAUNCH_TX" fi + if [ "$RELAUNCH" -eq 1 ]; then + preserve_relaunch_meta + fi } >"$SPAWN_META_PATH" || { echo "error: task record for $ID could not be prepared at $SPAWN_META_PATH" >&2 exit 1 diff --git a/tests/fm-control-relaunch.test.sh b/tests/fm-control-relaunch.test.sh index 112bc0d1d86..329b2b478ca 100755 --- a/tests/fm-control-relaunch.test.sh +++ b/tests/fm-control-relaunch.test.sh @@ -729,6 +729,29 @@ test_relaunch_preserves_durable_task_metadata() { pass "fm-control relaunch: durable task metadata survives replacement launch publication" } +test_relaunch_keeps_a_recorded_pr_parseable_for_monitoring() { + local dir out rc parsed + dir=$(new_case pr-parse rl90) + add_ship_task "$dir" rl90 claude + { + printf '%s\n' 'pr=https://github.com/example/repo/pull/90' + printf '%s\n' 'pr_head=0123456789abcdef0123456789abcdef01234567' + printf '%s\n' 'x_request=request-90' + } >> "$dir/home/state/rl90.meta" + + out=$(run_control "$dir" rl90 relaunch --note "keep PR monitoring alive"); rc=$? + expect_code 0 "$rc" "relaunch of a task with a recorded PR should succeed"$'\n'"$out" + [ -n "$(meta_field "$dir" rl90 control_relaunch_tx)" ] \ + || fail "the published record should still identify its relaunch transaction" + parsed=$(bash -c ' + . "$1/bin/fm-pr-lib.sh" + fm_pr_metadata_identity_parse "$2" && printf "%s\n" "$FM_PR_META_URL" + ' _ "$ROOT" "$dir/home/state/rl90.meta") + [ "$parsed" = "https://github.com/example/repo/pull/90" ] \ + || fail "PR monitoring must still parse the relaunched task record (got: ${parsed:-rejected})" + pass "fm-control relaunch: a recorded PR stays parseable for PR monitoring after relaunch" +} + test_relaunch_serializes_concurrent_durable_metadata_publication() { local dir control_pid link_pid rc i=0 traceparent prepare launch_release waiting ready release dir=$(new_case metadata-race rl28) @@ -4140,6 +4163,7 @@ test_relaunch_refuses_before_exit_when_the_composer_holds_pending_text test_relaunch_refuses_before_exit_when_the_composer_state_is_unproven test_relaunch_from_linked_home_preserves_recorded_worktree test_relaunch_preserves_durable_task_metadata +test_relaunch_keeps_a_recorded_pr_parseable_for_monitoring test_relaunch_serializes_concurrent_durable_metadata_publication test_disabled_relaunch_clears_prior_trace_context test_relaunch_appends_the_progress_note_to_the_instructions From 5a94aad66c4bf40d15cc961d6b6ea0b2c6579425 Mon Sep 17 00:00:00 2001 From: Charles Brooker Date: Thu, 8 Oct 2026 21:19:39 -0500 Subject: [PATCH 2/3] no-mistakes(review): Preserve PR metadata ordering during traced relaunches --- bin/fm-spawn.sh | 8 +++-- docs/trace-context.md | 3 ++ tests/fm-control-relaunch.test.sh | 54 +++++++++++++++++++------------ 3 files changed, 43 insertions(+), 22 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 6c6b6f6eb34..11711864968 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -5866,8 +5866,12 @@ spawn_record_traceparent() { fi SPAWN_META_TMP="$STATE/.$ID.meta.trace.${BASHPID:-$$}" if [ ! -f "$meta" ] || [ ! -w "$meta" ] || - ! awk -F= '$1 != "traceparent"' "$meta" >"$SPAWN_META_TMP" || - ! printf 'traceparent=%s\n' "$SPAWN_TRACEPARENT" >>"$SPAWN_META_TMP" || + ! awk -F= -v carrier="$SPAWN_TRACEPARENT" ' + $1 == "traceparent" { next } + $1 == "pr" && !written { print "traceparent=" carrier; written = 1 } + { print } + END { if (!written) print "traceparent=" carrier } + ' "$meta" >"$SPAWN_META_TMP" || ! fm_backlog_atomic_transition publish "$SPAWN_META_TMP" "$meta" "task record" "$STATE"; then status=1 rm -f "$SPAWN_META_TMP" 2>/dev/null || true diff --git a/docs/trace-context.md b/docs/trace-context.md index f714d1a555e..f04dd1efaaf 100644 --- a/docs/trace-context.md +++ b/docs/trace-context.md @@ -52,6 +52,9 @@ Because ambient `TRACEPARENT` is never read, the environment a supervisor happen Disabling propagation is an intentional trace boundary: a disabled home injects no carrier into a newly launched or relaunched agent even when the task meta already contains a valid `traceparent=`. An actual disabled relaunch regenerates the task meta without `traceparent=`, so a later enabled relaunch roots a new trace instead of resuming the identity from before the boundary; reusing an already-alive remote endpoint is not a relaunch and preserves the carrier that agent already holds. +Carrier publication removes prior `traceparent=` lines and writes one replacement before the first `pr=` line, or at the end of a record without a PR. +This keeps the trailing PR identity block parseable for monitoring after a traced relaunch. + ### Enablement is home-session-scoped Each locked `bin/fm-session-start.sh` run resolves that home's `config/trace-context` plus `FM_TRACE_CONTEXT` exactly once into session-scoped effective state. diff --git a/tests/fm-control-relaunch.test.sh b/tests/fm-control-relaunch.test.sh index 329b2b478ca..dc5422f685f 100755 --- a/tests/fm-control-relaunch.test.sh +++ b/tests/fm-control-relaunch.test.sh @@ -730,26 +730,40 @@ test_relaunch_preserves_durable_task_metadata() { } test_relaunch_keeps_a_recorded_pr_parseable_for_monitoring() { - local dir out rc parsed - dir=$(new_case pr-parse rl90) - add_ship_task "$dir" rl90 claude - { - printf '%s\n' 'pr=https://github.com/example/repo/pull/90' - printf '%s\n' 'pr_head=0123456789abcdef0123456789abcdef01234567' - printf '%s\n' 'x_request=request-90' - } >> "$dir/home/state/rl90.meta" - - out=$(run_control "$dir" rl90 relaunch --note "keep PR monitoring alive"); rc=$? - expect_code 0 "$rc" "relaunch of a task with a recorded PR should succeed"$'\n'"$out" - [ -n "$(meta_field "$dir" rl90 control_relaunch_tx)" ] \ - || fail "the published record should still identify its relaunch transaction" - parsed=$(bash -c ' - . "$1/bin/fm-pr-lib.sh" - fm_pr_metadata_identity_parse "$2" && printf "%s\n" "$FM_PR_META_URL" - ' _ "$ROOT" "$dir/home/state/rl90.meta") - [ "$parsed" = "https://github.com/example/repo/pull/90" ] \ - || fail "PR monitoring must still parse the relaunched task record (got: ${parsed:-rejected})" - pass "fm-control relaunch: a recorded PR stays parseable for PR monitoring after relaunch" + local dir out rc parsed tracing + for tracing in off on; do + dir=$(new_case "pr-parse-$tracing" rl90) + add_ship_task "$dir" rl90 claude + printf '%s\n' "$$" > "$dir/home/state/.lock" + printf '%s %s\n' "$$" "$tracing" > "$dir/home/state/.trace-context-effective" + { + printf '%s\n' 'traceparent=00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-01' + printf '%s\n' 'pr=https://github.com/example/repo/pull/90' + printf '%s\n' 'pr_head=0123456789abcdef0123456789abcdef01234567' + printf '%s\n' 'x_request=request-90' + } >> "$dir/home/state/rl90.meta" + + out=$(run_control "$dir" rl90 relaunch --note "keep PR monitoring alive"); rc=$? + expect_code 0 "$rc" "relaunch of a task with a recorded PR and tracing $tracing should succeed"$'\n'"$out" + [ -n "$(meta_field "$dir" rl90 control_relaunch_tx)" ] \ + || fail "the published record should still identify its relaunch transaction" + parsed=$(bash -c ' + . "$1/bin/fm-pr-lib.sh" + fm_pr_metadata_identity_parse "$2" && printf "%s\n" "$FM_PR_META_URL" + ' _ "$ROOT" "$dir/home/state/rl90.meta") + [ "$parsed" = "https://github.com/example/repo/pull/90" ] \ + || fail "PR monitoring must still parse the relaunched task record with tracing $tracing (got: ${parsed:-rejected})" + if [ "$tracing" = on ]; then + [ "$(awk -F= '$1 == "traceparent" { count++ } END { print count+0 }' "$dir/home/state/rl90.meta")" = 1 ] \ + || fail "traced relaunch must publish exactly one trace carrier" + [ "$(meta_field "$dir" rl90 traceparent)" = "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-01" ] \ + || fail "traced relaunch must preserve the task's recorded carrier" + else + [ -z "$(meta_field "$dir" rl90 traceparent)" ] \ + || fail "untraced relaunch must remove the task's recorded carrier" + fi + pass "fm-control relaunch: a recorded PR stays parseable for PR monitoring with tracing $tracing" + done } test_relaunch_serializes_concurrent_durable_metadata_publication() { From b682789040d85ef96b4df9077ba6100af8cd0fbd Mon Sep 17 00:00:00 2001 From: Charles Brooker Date: Thu, 8 Oct 2026 22:12:40 -0500 Subject: [PATCH 3/3] no-mistakes(document): Clarify relaunch metadata ordering and regression coverage --- bin/fm-spawn.sh | 5 ++--- docs/trace-context.md | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 11711864968..7ddc846fc23 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -5509,9 +5509,8 @@ preserve_relaunch_meta() { echo "home=$PROJ_ABS" echo "projects=$SECONDMATE_PROJECTS" fi - # Written before the preserved lines, never after: a pr= line's identity block - # must stay LAST in the record, because fm_pr_metadata_identity_parse rejects - # any other key following pr= and PR monitoring would silently stop. + # Keep relaunch fields before the preserved PR block so monitoring can parse it; + # fm_pr_metadata_identity_parse owns the allowed fields following pr=. if [ "$SPAWN_CONTROL_PARENT" = 1 ] && [ -n "${FM_CONTROL_RELAUNCH_TX:-}" ]; then echo "control_relaunch_tx=$FM_CONTROL_RELAUNCH_TX" fi diff --git a/docs/trace-context.md b/docs/trace-context.md index f04dd1efaaf..0d4237e3ac1 100644 --- a/docs/trace-context.md +++ b/docs/trace-context.md @@ -53,7 +53,7 @@ Disabling propagation is an intentional trace boundary: a disabled home injects An actual disabled relaunch regenerates the task meta without `traceparent=`, so a later enabled relaunch roots a new trace instead of resuming the identity from before the boundary; reusing an already-alive remote endpoint is not a relaunch and preserves the carrier that agent already holds. Carrier publication removes prior `traceparent=` lines and writes one replacement before the first `pr=` line, or at the end of a record without a PR. -This keeps the trailing PR identity block parseable for monitoring after a traced relaunch. +PR monitoring can still parse the preserved identity after relaunch with tracing on or off; [`tests/fm-control-relaunch.test.sh`](../tests/fm-control-relaunch.test.sh)'s `test_relaunch_keeps_a_recorded_pr_parseable_for_monitoring` covers both paths. ### Enablement is home-session-scoped