fix: accept 'pkg.mod:name' colon refs; warn on local-path leaks in golden recordings - #44
Merged
Merged
Conversation
…a golden records a home path Two independent fixes, both additive. resolve_to_function rejected the colon reference (#40, part 2) -------------------------------------------------------------- `'pkg.mod:name'` is the house spelling -- `commands_from`, `import_object`, `mk_parser`'s `obj:` doc and `python -m cw` all document it and say the colon is required -- but `cw.resolve_to_function`, the exported and most generically named resolver, raised `ValueError` on it. Two public "turn a string into a function" entry points accepted different grammars, and the stricter one was the one people reach for first. - `parse_spec_with_dot_path` now validates against `_DOT_OR_COLON_REF` (`^[\w.]+(?::[\w.]+)?$`), built from `commands.REF_SEPARATOR` so there is one spelling of the grammar. Two colons are still refused, and the error names both accepted forms while keeping the phrase existing tests match on. - `resolve_func_from_dot_path` delegates a colon reference to `commands.import_object` -- one implementation of the import -- and keeps the existing `callable()` check and the `(ImportError, AttributeError) -> ValueError` wrapping, so the failure shape is unchanged. Pure widening: every string accepted before resolves to the identical object, and the only delta is that strings which used to raise now succeed. The one observable shift is with a Mapping `get_func`, where a colon spec moves from `ValueError` to the `TypeError` a Mapping already raises for *any* unknown key -- so a colon reference stops being singled out by the parser and behaves like every other key. The single fleet dependent is green. characterize() recorded home directory paths silently (#38, fix 3 only) ----------------------------------------------------------------------- `characterize`'s own docstring says to commit the golden, and argparse renders parameter defaults into `--help`, so a body routinely froze an absolute path under the recording user's home into a public repo. Nothing caught it: a `--help` body is tier 3, so it is a snapshot and never asserted, and `replay` reports `identical` on every machine. Four repos in one migration wave had to drop the case from the corpus after noticing by hand. - `local_path_hits(text, *, home='~')` reports which markers a body carries (the running user's home, then the generic `HOME_ROOTS`). It reports rather than scrubs, because only the caller knows what belongs in the path's place. - `characterize(..., warn_on_local_paths=True)` warns at record time, naming the offending argv. Recorded content is byte-identical either way, and nothing near `_record`, the golden schema or the compare path is touched. Deliberately excluded: the `redact=` half of the original proposal, which has to be persisted in the golden and reapplied by `replay` to be correct, and `hyphenate_groups` from #40 part 1, which renames a live subcommand. `warnings` is imported inside the one function that needs it, so testing.py's D4 standalone contract (its module-scope imports are an asserted set) holds. The existing corpus produces zero hits, so the default is silent today. Tests: 952 passed, 2 skipped -> 973 passed, 2 skipped. Parity gate still "8 shapes / 137 cases: identical". Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
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
1 participant
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
resolve_to_function/resolve_func_from_dot_path/parse_spec_with_dot_pathnow also accept the'pkg.mod:name'colon form thatcommands_fromandmk_parser'sobj:already document elsewhere in cw, delegating tocw.commands.import_object(the one existing implementation of that grammar) when a colon is present. The plain dot-path grammar is unchanged and still works exactly as before — this only widens what's accepted; no previously-valid input becomes invalid. Closes part 2 of Two resolution/naming inconsistencies: hyphenate_groups ignored by add_commands(group_name=), and resolve_to_function rejects the documented 'pkg.mod:name' form #40.cw.testing: newlocal_path_hits()helper plus acharacterize(..., warn_on_local_paths=True)opt-out flag that warns (UserWarning, doesn't rewrite anything) when a recorded golden's--helpbody embeds an absolute path under the recording machine's home directory — argparse routinely renders$HOME-derived defaults into--help, and a golden is meant to be committed. Closes cw.testing: recorded --help bodies can embed the recording user's home directory, silently #38.characterizeparameter defaults toTrue(a new warning surfaces only when a golden genuinely does carry a local path — nothing about non-offending callers changes), and the accepted-input grammar only grows.Does not touch
#40's first item (hyphenate_groupsignored byadd_commands(group_name=...)) — that's a separate, larger behaviour-change question (verbatim vs. hyphenated) left for its own PR. Referencing, not closing, #40.Dependents check (cw has ~40 fleet dependents per
fleet_dependents.json)Grepped the whole fleet for callers of the touched surface (
resolve_to_function,parse_spec_with_dot_path,characterize,local_path_hits) and ran each hit's relevant test file against this branch installed editable (not PyPI cw) in an isolated venv:pip, fixed by installing it — nothing to do with this change).the local .pth ecosystem is not importable) — pre-existing environment limitation, unaffected by this branch.Test plan
wads ci-local(private-Actions-blocked account, priv#139): ruff format + lint, pytest on py3.10/py3.12, build — all green (680 passed, 10 skipped, 1 warning).🤖 Generated with Claude Code