Skip to content

refactor(core): remove the duplicate helper and correct drifted headers - #501

Merged
ss-o merged 1 commit into
nextfrom
bug-449
Sep 3, 2026
Merged

refactor(core): remove the duplicate helper and correct drifted headers#501
ss-o merged 1 commit into
nextfrom
bug-449

Conversation

@ss-o

@ss-o ss-o commented Sep 3, 2026

Copy link
Copy Markdown
Member

Wave 1 item 2 from the preserved architecture audit, plus the structural tests that stop both defects recurring.

The duplicate

.zi-at-eval was defined in both lib/zsh/autoload.zsh:303 and lib/zsh/install.zsh:2047. I diffed them: same behaviour, different wording. So nothing was broken.

What makes it worth fixing is that both libraries are sourced conditionally and in different orders depending on the command:

zi.zsh:1383  (( ${+functions[.zi-setup-plugin-dir]} )) || source .../install.zsh
zi.zsh:1384  (( ${+functions[.zi-confirm]} ))          || source .../autoload.zsh
zi.zsh:1393  (( ${+functions[.zi-setup-plugin-dir]} )) || source .../install.zsh   # install only

So which definition wins varies by code path. Two copies that must stay in lockstep, selected non-deterministically, become a heisenbug the first time they drift.

Removing the autoload.zsh copy is safe: the only call sites are install.zsh:2410 and install.zsh:2430, inside the file that defines it; it is absent from contracts/public-contract-v1.json; and it has no consumer anywhere in the organization.

The drifted headers

lib/zsh/autoload.zsh   header '.zi-update-in-parallel'  precedes  '.zi-update-all-parallel'
zi.zsh                 header '+zi-parse-opts'          precedes  '.zi-parse-opts'

The second is not in the issue. A systematic sweep of every # FUNCTION: header against the definition that follows it turned it up, which is why the test does that sweep rather than checking one known case.

Why a structural test

Neither defect is reachable behaviourally. Duplicate definitions that agree behave correctly right up until they drift, and a wrong header misleads only the next reader. tests/source-hygiene.zsh asserts both directly over zi.zsh and lib/zsh/*.zsh, currently 175 functions.

Each defect was reintroduced separately and the test observed failing on it:

functions defined more than once:
  .zi-at-eval: lib/zsh/autoload.zsh:304 and lib/zsh/install.zsh:2047
not ok - a function is defined in more than one shipped source        exit=1

FUNCTION headers naming a different function:
  lib/zsh/autoload.zsh:1912 header '.zi-update-in-parallel' precedes '.zi-update-all-parallel'
not ok - a FUNCTION header does not match the function it precedes    exit=1

Registered in zsh-n.yml, both path filters and a job.

Two issue items not addressed, deliberately

Item 2 is not reproducible. The header above @zsh-plugin-run-on-unload at zi.zsh:1309 names that function correctly. Either it was fixed after the audit or the audit was slightly off. Nothing to change, and the new test would catch it if it regressed.

Item 4 is out of scope. The .zi-load-ices snippet-directory TODO is untouched; the issue itself says it needs an ID-collision test before it can be classified as a defect. That deserves its own investigation rather than being bundled here.

Full suite 20/20, zsh -n clean on zi.zsh and both libraries.

Closes #449

.zi-at-eval was defined in both lib/zsh/autoload.zsh and lib/zsh/install.zsh.
The two agreed on behaviour and differed in wording, so nothing was broken, but
both libraries are sourced conditionally and in different orders depending on
the command, so which definition won varied by code path. That is a heisenbug
waiting for the first divergence.

The only call sites are install.zsh:2410 and install.zsh:2430, inside the file
that defines it. It is absent from contracts/public-contract-v1.json and has no
consumer anywhere in the organization, so removing the autoload.zsh copy leaves
one deterministic definition next to its callers.

Two `# FUNCTION:' headers named a different function than they preceded:

  lib/zsh/autoload.zsh  .zi-update-in-parallel  -> .zi-update-all-parallel
  zi.zsh                +zi-parse-opts          -> .zi-parse-opts

The second was not in the issue; a systematic sweep of every header against the
following definition found it.

Neither defect can be caught behaviourally. Duplicate definitions that agree
behave correctly until they drift, and a wrong header misleads only the next
reader. tests/source-hygiene.zsh asserts both structurally over zi.zsh and
lib/zsh/*.zsh, currently 175 functions, and was observed failing with each
defect reintroduced separately.

Issue item 2 is not reproducible: the header above @zsh-plugin-run-on-unload
names that function correctly. Item 4, the .zi-load-ices snippet-directory
TODO, is untouched; the issue itself says it needs an ID-collision test before
it can be classified as a defect.

Closes #449
@ss-o
ss-o requested a review from a team as a code owner September 3, 2026 01:47
@ss-o
ss-o merged commit 3a0d632 into next Sep 3, 2026
81 checks passed
@ss-o
ss-o deleted the bug-449 branch September 3, 2026 01:52
@github-actions github-actions Bot mentioned this pull request Sep 3, 2026
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