PR #787 follow-ups: unpinned wrap-up payload keys, the dropped tree list, one fake still swallowing kwargs nf-xjif ← Beads

closed priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/790 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #787 (dev.py skeleton + --json plumbing, design 006 pieces 3 + 4). All are below the bar that cycles a PR — the review closed CONVERGED. Unclaimed and unlabelled.

1. tools/pr/wrap_up.pypayload() publishes six keys nothing reads and nothing pins.

worktree, clone, state_dir, trees, tracked, tracked_files are all in the object; none is in devcli.WRAP_UP_KEYS, none is read by _print_wrap, and the contract test only checks one direction (WRAP_UP_KEYS ⊆ producer), so none of them can be seen to drift or vanish.

This is the call #787 itself made on the sibling producer one round earlier — session_state.payload()'s docstring now states it as a rule: *"Nothing here is published that no consumer reads … Add a field when a caller wants it, with a *_KEYS entry in the same commit."* wrap_up.payload() does not follow it. Pin what a consumer wants (see item 2), drop the rest until piece 5 or 8 asks. DEV_PRINCIPLES B4.

2. tools/nflib/devcli.pytrees is the one prose field _print_wrap still drops.

wrap_up.report() prints a self/f5 line per tree with its full path. _print_wrap renders checks, notes, strays, events, state, summary, notices, hand-checks and cd — not trees. So ./dev.py wrap never names the two directories ./dev.py wrap --remove will delete; after the fact events names them, before it nothing does, and that is the irreversible verb.

_print_wrap's own docstring is the rule it misses: *"⚠ Every field the prose reports, or none of it"* and *"a key in WRAP_UP_KEYS is a key that gets rendered."* trees into WRAP_UP_KEYS plus two lines in _print_wrap closes this and item 1's trees row together.

3. tools/tests/test_spawn_dev.py:169FakeWorld still swallows unknown kwargs.

nflib.fakes's RUN_KWARGS comment claims the fix is repo-wide: *"Public: every hand-written runner fake in tools/tests/ checks against this one tuple."* Two of the three do — FakeBox (test_nflib_sessions.py:105) gained the call in #787, and ShiftingRunner (test_rebuild_f5.py:2192) inherits it through FakeRunner. FakeWorld is the third, and it drives spawn_dev.py's worktree creation, which is exactly the kind of call site #736 was. One check_kwargs(kwargs, RUN_KWARGS, "nflib.proc.run (faked)") line, or narrow the comment to what is true.

4. Cosmetic / low value, listed so they are not rediscovered:

Dependencies

None.

Comments

No comments.

Add a comment