fix(bin): gate trace-context resolve on the frozen session decision - #2898
Open
harmeet-1337 wants to merge 2 commits into
Open
fix(bin): gate trace-context resolve on the frozen session decision#2898harmeet-1337 wants to merge 2 commits into
harmeet-1337 wants to merge 2 commits into
Conversation
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.
…n-decision resolve
Confidence Score: 5/5The 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 |
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.
Summary
Fixes accepted simplification-audit finding F-S28-1 (P0):
fm_trace_context_resolvegated on the livefm_trace_context_enabledcheck, which contradicted its header contract promising the frozen per-session decision and forced every production caller to defeat it with aFM_TRACE_CONTEXT=onenv override — leaving theconfig_dirargument dead on every real path.Change
fm_trace_context_resolve <state-dir> <meta-file>: it returns empty unless the frozenstate/.trace-context-effectivedecision (bound to the session lock) for that state dir ison, then reuses the recorded carrier or mints a fresh root exactly as before.fm_trace_context_enabledis intentionally left unchanged as the session-start input resolver.bin/fm-spawn.shcall sites updated to the new shape with no env forcing. The local path keepsSPAWN_TRACE_EFFECTIVEfromsession_effectiveso 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--traceparentdelivery branch bypassesresolveand is untouched.fm_trace_context_enabledfunction comment updated to match.tests/fm-trace-context-lib.test.shresolve cases rewritten to seed the session lock +.trace-context-effectiverecord instead ofFM_TRACE_CONTEXT=on, and add the previously inexpressible negative:FM_TRACE_CONTEXT=onin 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, andfm-control-relaunchpass;bin/fm-lint.shis clean with pinned ShellCheck 0.11.0 + actionlint 1.7.12. (The pre-existingfm-remote-secondmate-trace-contextsuite fails on an unrelatedPROJECT_NAMES[@]unbound-variable error infm-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.