Skip to content

fix(kanban): log git's returncode and stderr when survivor capture fails - #856

Merged
Kyzcreig merged 1 commit into
mainfrom
daedalus/t_169d6e46-survivor-git-diagnostic
Sep 22, 2026
Merged

Kyzcreig merged 1 commit into
mainfrom
daedalus/t_169d6e46-survivor-git-diagnostic

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Card: t_169d6e46. Test-hermeticity investigation; this is the impl half (item 2 of the card).

What the "flake" actually was

NOT a survivor defect, and NOT a RAM-disk / racy-clean-index race. Root cause is outside this repo:
~/.hermes/fleet/ramscratch-env.sh exported a fixed --basetemp=/Volumes/ramscratch/pytest
into every daedalus-family worker shell (terminal.shell_init_files). pytest rm_rf()s an explicit
--basetemp at session start — no numbered subdir, no lock
(_pytest/tmpdir.py::TempPathFactory.getbasetemp, read in 9.1.1: if basetemp.exists(): rm_rf(basetemp)).
Two concurrent worker pytest sessions therefore delete each other's live tmp_path trees mid-run.

Instrumented capture of the swallowed stderr (subprocess spy over subprocess.run):

argv=['git','-C','/Volumes/ramscratch/pytest/test_reaper_preserves_before_r0/home/kanban/workspaces/t_721c9203','status','--porcelain','--untracked-files=all']
rc=128
stderr="fatal: cannot change to '...': No such file or directory"

Measured, one variable (basetemp fixed vs per-session), same host, same RAM disk:

arm sessions red
fixed --basetemp (status quo), N=2 concurrent 12 10
fixed --basetemp, N=3 concurrent 6 6
unique --basetemp per session, N=2 10 0
no --basetemp, TMPDIR on the RAM disk (the fix), N=3 18 0

This also answers the card's review note that a green streak could not distinguish
"fixed" from "trigger absent": the red is now reproducible on demand
(repro.sh fixed 2 → red first try), so the green arm is a real control.

The env fix lands separately in hermes-home (fleet/ramscratch-env.sh + a detector,
fleet/tests/test-ramscratch-env.sh, born-red against the pre-fix file).

What this PR changes

Only the thing that is this repo's problem: _git raised a constant
survivor_unavailable: git inspection failed and threw away the returncode and stderr, so an
environmental fault and a real capture defect are indistinguishable at the call site. That is
why attributing it cost a full pass (t_ac0e595c), and kanban_survivor.py has five open PRs whose
reviewers each pay that tax.

  • The raised message is unchanged — it is persisted to held_reason, the event log and stderr,
    and open PRs key on that string.
  • The detail goes to the log, redacted through kanban_external_survivor.redact, the same
    helper that guards an echoed claim. Git stderr can carry a credential-bearing remote URL.

Verification

  • 2 new tests, born-red 2/2 against unmodified fork/main
    (assert 'rc=128' in '', IndexError on zero log records).
  • Teeth: mutating the log call to unredacted stderr kills only the redaction test
    (1 failed, 8 passed) — the pair is not a vacuous green.
  • 74/74 ×3 across all four survivor test files.
  • ruff check clean on both changed files.

Hotspot / coordination

hermes_cli/kanban_survivor.py is a multi-way hotspot (#848, #846, #842, #839, #796). Checked each
open PR's diff: only #796 touches this helper, and only its signature line
(check=True → check=True, timeout=30), which does not overlap the body edited here. Diff is
+66/−1 across 2 files, one impl hunk.

Upstream

kanban_survivor.py and its tests are 404 at NousResearch/hermes-agent — no upstream surface, so
no upstream PR is owed for this half.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

`_git` raised a constant `survivor_unavailable: git inspection failed` and
discarded the returncode and stderr, so a real capture defect and a purely
environmental fault were indistinguishable at the call site. Establishing
which cost a full attribution pass per occurrence (t_ac0e595c), and
kanban_survivor.py has five open PRs whose reviewers each pay that tax.

Root cause of the occurrences that prompted this (card t_169d6e46): NOT a
survivor defect and not a RAM-disk race. fleet/ramscratch-env.sh exported a
FIXED `--basetemp=/Volumes/ramscratch/pytest` into every daedalus-family
worker shell; pytest rm_rf()s an explicit --basetemp at session start with no
numbered subdir and no lock, so concurrent workers deleted each other's live
tmp_path and git exited 128 "cannot change to '<path>': No such file or
directory". Measured with 2 concurrent sessions of the survivor suite:
10/12 sessions red with the fixed basetemp, 0/18 red without it. The env fix
lands separately in hermes-home; this commit is what makes the next
occurrence diagnosable in one log line instead of an attribution pass.

The raised message is unchanged (it is persisted to held_reason, the event
log and stderr, and open PRs key on it). Detail goes to the log, redacted
through kanban_external_survivor.redact, the same helper that guards an
echoed claim.

Verified:
  - 2 new tests in test_kanban_survivor_authority.py, born-red 2/2 against
    unmodified fork/main (assert 'rc=128' in '' / IndexError on no records)
  - teeth: mutating the log to unredacted stderr kills ONLY the redaction
    test (1 failed, 8 passed), so it is not a vacuous green
  - 74/74 x3 across all four survivor test files
  - ruff clean on both changed files
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

FleetReview

Reviewed with 2 of 3 model families — openai unavailable.

Confidence: 4/5

Findings

  • P1 hermes_cli/kanban_survivor.py:47 — _ext.redact is a URL-claim sanitizer, not a general log scrubber; credential shapes in git stderr can still reach the logfile
  • P2 tests/hermes_cli/test_kanban_survivor_authority.py:232 — New test asserts on locale-dependent git output and an unpinned git message

FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $5.71 · duration: 18m 37s · rounds: 1 · files examined: 2

@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 22, 2026
Merged via the queue into main with commit 8cc6d83 Sep 22, 2026
54 checks passed
@Kyzcreig
Kyzcreig deleted the daedalus/t_169d6e46-survivor-git-diagnostic branch September 22, 2026 08:23
Kyzcreig added a commit that referenced this pull request Sep 23, 2026
fork/main advanced 83 commits since 0dc715c, including the survivor
stack (#837 #842 #848 #856 #872 #879 #886 #888) which rewrote
kanban_survivor.py from 475 to 1516 lines. Four conflicts, all in
kanban_survivor.py, resolved toward main's shapes:

  _git         union: main's input= (needed by _present_commits/_rev_list)
               plus this PR's timeout= (needed by _content_advisory's
               120s fetch).
  _capture     kept main's extraction; folded this PR's canonical-tree
               fallback + mirror_hint advisory INTO it, so the
               _explain_broken_object_store classifier still wraps every
               object-reading step.
  ref arm      kept main's 'not bundles and len(refs) == len(repos) +
               len(carried)' (the #842/#848 carried-survivor accounting)
               and this PR's canonical sidecar.

The pre-#848 unbound-claim path is NOT reintroduced: _verified_explicit,
_unbound_keys, _reusable and _bound are main's, untouched. The landed
arm in preserve() and _verify_landed/_landed_contains_history merged
without conflict.

Verified:
  11 survivor files (4 from this PR + 7 landed since): 255 passed, 0 failed
  the PR's own 4 files:                                  98 passed, 0 failed
  mutation, dirty-tree guard neutered:    16 passed, 2 FAILED
  mutation, history binding neutered:     15 passed, 3 FAILED
  ruff on kanban_survivor.py + kanban_db.py: clean
  git diff --check: clean
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