Skip to content

fix(bin): gate trace-context resolve on the frozen session decision - #2898

Open
harmeet-1337 wants to merge 2 commits into
kunchenguid:mainfrom
harmeet-1337:fm/fm-fix-tracegate
Open

fix(bin): gate trace-context resolve on the frozen session decision#2898
harmeet-1337 wants to merge 2 commits into
kunchenguid:mainfrom
harmeet-1337:fm/fm-fix-tracegate

Conversation

@harmeet-1337

Copy link
Copy Markdown

Summary

Fixes accepted simplification-audit finding F-S28-1 (P0): fm_trace_context_resolve gated on the live fm_trace_context_enabled check, which contradicted its header contract promising the frozen per-session decision and forced every production caller to defeat it with a FM_TRACE_CONTEXT=on env override — leaving the config_dir argument dead on every real path.

Change

  • Re-shape fm_trace_context_resolve <state-dir> <meta-file>: it returns empty unless the frozen state/.trace-context-effective decision (bound to the session lock) for that state dir is on, then reuses the recorded carrier or mints a fresh root exactly as before.
  • fm_trace_context_enabled is intentionally left unchanged as the session-start input resolver.
  • Both bin/fm-spawn.sh call sites updated to the new shape with no env forcing. The local path keeps SPAWN_TRACE_EFFECTIVE from session_effective so the child still freezes the correct on/off decision even when entropy fails and no carrier is minted; the remote secondmate path deliberately keeps resolving under the parent's frozen state dir, since the parent owns the task meta. The --traceparent delivery branch bypasses resolve and is untouched.
  • Header entry-point and enablement paragraphs plus the fm_trace_context_enabled function comment updated to match.
  • tests/fm-trace-context-lib.test.sh resolve cases rewritten to seed the session lock + .trace-context-effective record instead of FM_TRACE_CONTEXT=on, and add the previously inexpressible negative: FM_TRACE_CONTEXT=on in the environment but frozen off / unlocked / stale-lock resolves to empty.

Scope kept minimal: no behavior change beyond moving the gate from a live check to the frozen decision; all existing error messages, output formats, and exit codes preserved.

Validation

Ran through the no-mistakes pipeline: intent, rebase, review, test, document, and lint steps all completed. Gating suites fm-trace-context-lib, fm-trace-context-spawn, and fm-control-relaunch pass; bin/fm-lint.sh is clean with pinned ShellCheck 0.11.0 + actionlint 1.7.12. (The pre-existing fm-remote-secondmate-trace-context suite fails on an unrelated PROJECT_NAMES[@] unbound-variable error in fm-remote-home-seed.sh, confirmed identical on a clean tree.)

Delivery note

The upstream account has read-only access, so this PR is delivered from a fork branch. It carries only the in-scope commits: the fix and the document-step commit that syncs docs/verification/trace-context.md. No out-of-scope pipeline-injected commits (e.g. lint-tooling PATH fallbacks) were produced or included.

fm_trace_context_resolve gated on the LIVE fm_trace_context_enabled check,
which every production caller had to defeat with FM_TRACE_CONTEXT=on, making
the config_dir argument dead on every real path and contradicting the header
contract that promised the frozen per-session decision.

Re-shape resolve to <state-dir> <meta-file>: it returns empty unless the
frozen state/.trace-context-effective decision for that state dir is on, then
reuses or mints as before. fm_trace_context_enabled stays the session-start
input resolver. Update both fm-spawn call sites to the new shape with no env
forcing; the remote secondmate path keeps resolving under the parent's frozen
state dir. Rewrite the resolve tests to seed the session lock plus effective
record and add the previously inexpressible negative: FM_TRACE_CONTEXT=on in
the environment but frozen off, unlocked, or stale-lock resolves to empty.
@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge; no concrete changed-code-triggered failure was identified.

The updated callers pass the correct session state directory, and the resolver consults the same lock-bound decision that previously guarded those calls while eliminating the redundant live enablement check.

Reviews (1): Last reviewed commit: "no-mistakes(document): sync trace-contex..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant