feat: CLASSIFICATION function-header field + shadow-classification lint #163

2026-09-22T01:27:36Z

rootiest commented

2026-09-22 01:27:36 +00:00

Summary

Started from a Ctrl+Alt+U keybind fix and grew into a systematic audit of
every function's interaction with the C1 command-shadow system (ls→eza,
cat→bat, rm→trash, cd→zoxide, etc.), plus a new lint that keeps it
that way going forward.

`pretty-history` rename

conf.d/tricks.fish shadowed history itself to add timestamps, which
broke _replace_command_token.fish (Ctrl+Alt+U on an empty prompt dumped
the entire history with timestamps instead of recalling one command).
Renamed the shadow to pretty-history so history is never shadowed —
every function that reads history now gets real builtin semantics, no
builtin history defensive-coding required.

`CLASSIFICATION` function-header field

New optional header label (schema: docs/function-classification-schema.md,
introduced to contributors in CONTRIBUTING.md) with a closed tag set:

uses-shadow(name) — calls a shadow bare, wants the override.
bypasses-shadow(name) — calls command/builtin explicitly, needs
stock behavior.
self-limiting(name) — calls a shadow bare, but the shadow's own logic
(a flag check, tty/pipe auto-detection) already neutralizes the override,
so no tag would technically be required for safety, but it's
documented anyway: if that internal logic ever changes, every site is
one grep away instead of silently wrong.
destructive, network, blocking-prompt — general hazard flags.

Tagged across all 65 functions/conf.d entries that interact with a
shadow, using an agy-delegated first pass (fully reviewed and corrected by
hand — several judgment errors caught: false blocking-prompt from piped
read, a missed --yes escape hatch, destructive on a function's own
temp-file cleanup where the schema explicitly excludes that).

Real bugs found and fixed along the way

rm: three functions (fc.fish, dng2avif.fish,
_scrollback_prune_junk.fish) sent their own scratch/junk files to the
user's trash instead of wiping them.
cd: mkcd.fish and mkrep.fish (9 sites, including 8
rollback-to-original-directory calls on error paths) routed through
zoxide's shadow — a failed mkrep run's error-path cleanup could
silently land somewhere zoxide guessed instead of the directory it was
trying to return to.
less: config-help.fish's --man path explicitly checks
type -q less for real-less-only flag syntax (-R +N), then called
it bare, routing through our $PAGER → ov → less → more → cat
fallback chain instead.
cp/bash: _fish_deps_install.fish/_fish_deps_update.fish's
binary-upgrade path cps over an already-installed binary with no
existence guard — our cp shadow forces -i unconditionally (a plain
alias, not flag-aware like rm's), so this could hang waiting on a
confirmation prompt with nothing there to answer it. Same two files
piped curl into bare bash, invoking our shell-switch wrapper.
mv: agents-init.fish's AGENTS.md/CLAUDE.md relocation (4 sites)
called mv bare; each was already guarded by test -f on the
destination, so lower severity, but fixed to command mv anyway rather
than relying on that guard being the only thing standing between it and
an unattended hang.

New: shadow-classification lint (`tests/run-tests.fish` Phase 1b)

Runtime auto-unwrapping isn't viable in fish (no hook finer than shadowing
itself), so this is a static check: flags a bare shadow-command call in
functions/*.fish with no matching uses-shadow/self-limiting in that
function's own header. CI-blocking. Running the first draft of this
surfaced the less/cp/bash/mv bugs above before they shipped.

Testing

fish tests/run-tests.fish: 759/759 assertions, including the new
196/196 shadow-classification check.
python3 docs/verify-manual.py: 83/84 — the one failure is the expected
drift in the CI-generated docs/fish-config.md concat, regenerated and
auto-committed by CI on merge (same as every prior docs-touching PR).
Every CLASSIFICATION tag mechanically cross-checked against the actual
code it describes.

## Summary Started from a `Ctrl+Alt+U` keybind fix and grew into a systematic audit of every function's interaction with the C1 command-shadow system (`ls`→eza, `cat`→bat, `rm`→trash, `cd`→zoxide, etc.), plus a new lint that keeps it that way going forward. ## `pretty-history` rename `conf.d/tricks.fish` shadowed `history` itself to add timestamps, which broke `_replace_command_token.fish` (`Ctrl+Alt+U` on an empty prompt dumped the entire history with timestamps instead of recalling one command). Renamed the shadow to `pretty-history` so `history` is never shadowed — every function that reads history now gets real builtin semantics, no `builtin history` defensive-coding required. ## `CLASSIFICATION` function-header field New optional header label (schema: `docs/function-classification-schema.md`, introduced to contributors in `CONTRIBUTING.md`) with a closed tag set: - `uses-shadow(name)` — calls a shadow bare, wants the override. - `bypasses-shadow(name)` — calls `command`/`builtin` explicitly, needs stock behavior. - `self-limiting(name)` — calls a shadow bare, but the shadow's own logic (a flag check, tty/pipe auto-detection) already neutralizes the override, so no tag would technically be *required* for safety, but it's documented anyway: if that internal logic ever changes, every site is one grep away instead of silently wrong. - `destructive`, `network`, `blocking-prompt` — general hazard flags. Tagged across all 65 functions/`conf.d` entries that interact with a shadow, using an agy-delegated first pass (fully reviewed and corrected by hand — several judgment errors caught: false `blocking-prompt` from piped `read`, a missed `--yes` escape hatch, `destructive` on a function's own temp-file cleanup where the schema explicitly excludes that). ## Real bugs found and fixed along the way - **`rm`**: three functions (`fc.fish`, `dng2avif.fish`, `_scrollback_prune_junk.fish`) sent their own scratch/junk files to the user's trash instead of wiping them. - **`cd`**: `mkcd.fish` and `mkrep.fish` (9 sites, including 8 rollback-to-original-directory calls on error paths) routed through zoxide's shadow — a failed `mkrep` run's error-path cleanup could silently land somewhere zoxide guessed instead of the directory it was trying to return to. - **`less`**: `config-help.fish`'s `--man` path explicitly checks `type -q less` for real-`less`-only flag syntax (`-R +N`), then called it bare, routing through our `$PAGER → ov → less → more → cat` fallback chain instead. - **`cp`/`bash`**: `_fish_deps_install.fish`/`_fish_deps_update.fish`'s binary-upgrade path `cp`s over an already-installed binary with no existence guard — our `cp` shadow forces `-i` unconditionally (a plain alias, not flag-aware like `rm`'s), so this could hang waiting on a confirmation prompt with nothing there to answer it. Same two files piped `curl` into bare `bash`, invoking our shell-switch wrapper. - **`mv`**: `agents-init.fish`'s AGENTS.md/CLAUDE.md relocation (4 sites) called `mv` bare; each was already guarded by `test -f` on the destination, so lower severity, but fixed to `command mv` anyway rather than relying on that guard being the only thing standing between it and an unattended hang. ## New: shadow-classification lint (`tests/run-tests.fish` Phase 1b) Runtime auto-unwrapping isn't viable in fish (no hook finer than shadowing itself), so this is a static check: flags a bare shadow-command call in `functions/*.fish` with no matching `uses-shadow`/`self-limiting` in that function's own header. CI-blocking. Running the first draft of this surfaced the `less`/`cp`/`bash`/`mv` bugs above before they shipped. ## Testing - `fish tests/run-tests.fish`: 759/759 assertions, including the new 196/196 shadow-classification check. - `python3 docs/verify-manual.py`: 83/84 — the one failure is the expected drift in the CI-generated `docs/fish-config.md` concat, regenerated and auto-committed by CI on merge (same as every prior docs-touching PR). - Every `CLASSIFICATION` tag mechanically cross-checked against the actual code it describes.

rootiest added the Kind/Bug Kind/Feature Kind/Testing Area/Functions Area/Docs Area/Tests labels 2026-09-22 01:27:36 +00:00

rootiest added 6 commits 2026-09-22 01:27:38 +00:00

feat(docs): add CLASSIFICATION function-header field, rename history shadow aef686af86

Rename the C1 history() shadow to pretty-history so it never collides
with the fish builtin -- every function expecting stock history
semantics (search, --max, merge, ...) would otherwise silently break.
hist.fish, which relied on the shadow's timestamp formatting, now
requests it explicitly via builtin history --show-time.

Add a CLASSIFICATION doc-header label so a function can declare its
interaction with C1-shadowed commands (uses-shadow/bypasses-shadow)
and general hazards (destructive, network, blocking-prompt) for
anyone deciding to disable an opinionated category or call the
function from automation. Wired into the manual/site build pipeline
(manualtools.py, build-manual.py) and the C1 shadow doc gets a new
"For function authors" bypass-mechanism reference table
(command/builtin/__original_help, and which shadows have no real
bypass target at all).

feat(functions): tag CLASSIFICATION across functions/ and conf.d/ 859f14a6e9

Audits every function's interaction with the C1-shadowed commands
(uses-shadow/bypasses-shadow) and general hazards (destructive,
network, blocking-prompt) per the CLASSIFICATION schema.

Delegated the initial mechanical sweep to agy, then reviewed every
file by hand: fixed a systemic double-blank-comment-line formatting
bug from the delegate pass, and corrected several judgment errors
found on review -- three false blocking-prompt tags where a fish
'read' was consuming piped input rather than waiting on a terminal
(open-url.fish, sbver.fish, play-media.fish, now untagged entirely),
a blocking-prompt tag on mkrep.fish despite its documented --yes
escape hatch, an untagged read in jobrunner.fish's own baseless
blocking-prompt claim (removed, along with a destructive tag on
cleanup of its own mktemp output -- the schema explicitly excludes
that), the same own-output-cleanup false positive on
_zellij_dump_log.fish's destructive tag, an interactive fzf-gated
confirmation on logs.fish and replay.fish's piped read misread the
same way as the first three, and a uses-shadow(mkdir) on mkcd.fish
that actually belongs to the _fish_mkdir_p helper it delegates to,
not to mkcd itself.

docs(classification): move schema out of gitignored AGENTS/, into docs/ 069a1f7743

AGENTS/functions/CLAUDE.md is git-ignored local agent state, not part
of the repo -- a comment/commit referencing it as the schema's home
points contributors at a file they can't see. The canonical CLASSIFICATION
schema now lives at docs/function-classification-schema.md (tracked),
with CONTRIBUTING.md's existing function-header-conventions section
extended to introduce it, and the C1 shadow doc's pointer updated to
match. AGENTS/functions/CLAUDE.md keeps only a one-line pointer to the
tracked file instead of duplicating the definitions.

fix(functions): stop leaking scratch files to trash via bare rm 100cb478bc

Verified an agy audit of every bare rm call (the trash-routing C1
shadow) by hand rather than trusting its report. Confirmed correct:
scrub.fish's custom_rm strategy and logs.fish's Ctrl-D delete both
deliberately want trash for a real, user-facing deletion.

Confirmed and fixed three cases where a function's own throwaway
scratch file was going to the user's trash instead of being wiped:
fc.fish's edited-command tmpfile, dng2avif.fish's intermediate PNM
(inconsistent with its own failure-path cleanup two lines up, which
already used -f), and _scrollback_prune_junk.fish's junk log files
(its sibling _prune_terminal_logs.fish already documents this exact
pitfall in its header).

Also went further than the report and classified every bypasses-shadow(rm)
caller found by grep that had never been audited at all:
config-settings.fish and edit.fish (own scratch cleanup, no destructive
data at stake) and key-crypt.fish (--remove deletes the user's real
input file after encryption, genuinely destructive, already documented
in its own header as 'not a secure wipe'). Corrected scrub.fish's tag,
which was missing uses-shadow(rm) for its deliberate trash-routing
branch alongside the bypass branch it already had tagged.

Added a note to the schema doc: rm's flag-based fallback lives inside
the shadow itself, so a bare rm -f/rm -rf call is not the caller
bypassing anything -- only an explicit command rm/builtin rm earns
the tag. This is why dng2avif.fish's fix needed no CLASSIFICATION
change: it already used rm -f, which was never actually the bug --
the missing -f on line 122 was.

fix(functions): stop routing deterministic cd through zoxide's shadow 373917d002

Verified an agy audit of every bare cd call by hand. conf.d/zoxide.fish
gates alias cd=z behind status is-interactive plus the C1 toggle, and
_zoxide_hook fires on --on-variable PWD, so it tracks a directory
change no matter how PWD got there -- switching to builtin cd loses
zoxide's frecency tracking nothing.

mkcd.fish's single cd and mkrep.fish's 9 (entering the new repo, plus
8 rollback-to-original-directory sites on error paths and --no-cd)
were both intended as exact, deterministic path navigation, never a
zoxide query. The real risk was mkrep's rollback path: if $orig_pwd
ever failed cd's own -d check for any reason, z's fallback branch
queries zoxide for a *guessed* frecent directory instead -- landing a
failed run's cleanup in a directory the caller never asked for, not
the one it was trying to return to. All 9 sites now use builtin cd.

Corrected both functions' CLASSIFICATION from uses-shadow(cd) to
bypasses-shadow(cd) -- neither wanted zoxide's query, they were tagged
that way only because the header audit recorded what the code was
doing at the time, not what it needed.

integrations/fzf.fish's fzf-alt-c-widget also calls bare cd, but it's
vendored upstream code (PatrickF1/fzf.fish) and is itself an
interactive directory-jump binding, not a script/automation caller --
left alone, same as fisher.fish's rm calls.

feat(tests): add shadow-classification lint; fix real cp/mv/less bugs 3414f81cb6

New Phase 1b in tests/run-tests.fish: catches a bare C1-shadowed-command
call in a functions/*.fish body with no matching uses-shadow(name) or
self-limiting(name) in that function's own CLASSIFICATION header. This
is exactly the check discussed after the rm and cd audits -- runtime
auto-unwrapping isn't viable in fish (there's no hook finer than
shadowing itself, and rewriting behavior invisibly at runtime is its
own footgun); a static lint using the CLASSIFICATION tag as the
declared-intentional marker is. Scoped to functions/*.fish only: the
one-function-per-file convention there makes body extraction exact
with no block-depth parser needed.

Added a new self-limiting(name) tag to the schema for the case a bare
call is safe not because the caller did anything, but because the
shadow's own logic already neutralizes the override: rm's and mkdir's
flag checks (verified precisely -- rm falls back to command rm for any
flag except a bare -r/-R/--recursive alone, which still routes to
trash; mkdir falls back to command mkdir -p for any flag, no
exception), and grep/fgrep/egrep/dir/vdir/cat's own tty auto-detection
(--color=auto, and bat's default color behavior -- verified
byte-identical to stock cat when piped, since bat also auto-disables
highlighting on a non-terminal). Explicit and durable rather than a
silent lint exemption: if a shadow's bypass condition is ever
weakened, every self-limiting site is one grep away instead of
silently wrong.

Running the first draft of the lint surfaced three more real bugs,
none previously audited:

- config-help.fish's --man pager path checks `type -q less` (proving
  it wants the real less binary specifically, for less-only -R/+N
  flag syntax) then called it bare, routing through our own
  $PAGER -> ov -> less -> more -> cat fallback chain instead -- which
  could hand those less-specific flags to a completely different
  program. Now command less.
- _fish_deps_install.fish and _fish_deps_update.fish's binary-upgrade
  paths cp a freshly downloaded binary over an already-installed one
  with no existence guard -- the update flow's target is guaranteed to
  already exist. Our cp shadow forces -i unconditionally (a plain
  alias, not flag-aware like rm's), so this would hang waiting on a
  confirmation prompt in any non-interactive run. Now command cp.
  Same two files' lazydocker install path piped curl output into bare
  bash, invoking our shell-switch wrapper instead of a plain
  subshell. Now command bash.
- agents-init.fish's AGENTS.md/CLAUDE.md relocation calls mv bare in
  four places; each is already guarded by a preceding test -f check on
  the destination, so the -i alias was unlikely to ever fire in
  practice, but explicit command mv removes the reliance on that guard
  entirely rather than leaving it as the only thing standing between a
  file move and an unattended hang.

The remaining ~65 flagged call sites across ~24 files were reviewed
individually and tagged self-limiting(rm)/self-limiting(mkdir)
(verified flagged with -f/-rf or -p) and self-limiting(grep)/
self-limiting(cat) (verified piped, captured, or -q/-c; none display
color to a human), plus uses-shadow(ls) for two existence-check-only
calls (cffetch.fish, ffetch.fish) whose output is redirected to
/dev/null.

rootiest added 1 commit 2026-09-22 01:36:58 +00:00

docs(index): add config-help keyword aliases for CLASSIFICATION 2367f5749d

The CLASSIFICATION schema (docs/function-classification-schema.md) had
no path into config-help's lookup: no fish-config.index keyword, and
the only in-pipeline section (the C1 doc's "For function authors")
doesn't contain the word classification itself, so even the
normalized-heading-scan fallback missed it on that term. Two aliases
added, pointing at the existing section -- no heading renamed, matching
the index file's own stated purpose.

rootiest merged commit abdb9cfacf into main

2026-09-22 02:12:58 +00:00

rootiest deleted branch feat/classification-header-field

2026-09-22 02:12:58 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: rootiest/fish-config#163