Skip to content

Record a launcher the hook can keep across upgrades - #694

Closed
MongLong0214 wants to merge 1 commit into
mainfrom
fix-693-hook-wrapper
Closed

Record a launcher the hook can keep across upgrades#694
MongLong0214 wants to merge 1 commit into
mainfrom
fix-693-hook-wrapper

Conversation

@MongLong0214

Copy link
Copy Markdown
Owner

Closes #693.

$ git config --local --get commitlore.bin
/Users/isaac/.local/share/commitlore/v0.8.2/dist/commitlore.mjs

After upgrading to 1.0.1 the repository was still validating commits with the
0.8.2 build while the CLI reported 1.0.1. doctor noticed and said so; nothing
fixed it, because install.sh cannot know which repositories have hooks.

What changed

The wrapper is recorded instead — when it demonstrably launches this build.
It is an absolute path too, so it keeps the property the versioned path was
chosen for (independence from PATH and from any node_modules/.bin/commitlore
above the repository) and simply does not name a release.

A wrapper belonging to a different installation is worse than a versioned path
— it would send the hook to code this install never verified — so an unreadable
or unrelated one falls back rather than guessing. Both directions are pinned.

Fourth instance of one pattern today

.mcp.json             "commitlore"                  the wrapper → followed every upgrade
Hermes external_dirs  .../v0.8.2/hermes/skills      versioned   → broke   (#686)
plugin cache          a copy of the code            a copy      → stalled (#660)
hook commitlore.bin   .../v0.8.2/dist/commitlore.mjs versioned  → this

The configuration that pinned nothing is the one that survived.

Scope

This changes what new installs record. Existing hooks still need
commitlore hooks install re-run, which doctor already prints as its fix line.

Verified by restoring the old resolution: the wrapper case fails. 72 tests
passing across hooks and the new file.

Canonical artifact 036397f2.

commitlore.bin held <data-root>/v0.8.2/dist/commitlore.mjs, so after upgrading
to 1.0.1 the repository was still validating commits with the 0.8.2 build while
the CLI reported 1.0.1. doctor noticed and said so; nothing fixed it, because
install.sh cannot know which repositories have hooks.

The wrapper is recorded instead when it demonstrably launches this build. It is
an absolute path too, so it keeps the property the versioned path was chosen for
-- independence from PATH and from any node_modules/.bin/commitlore above the
repository -- and simply does not name a release.

Only when it names this bundle. A wrapper belonging to a different installation
is worse than a versioned path: it would send the hook to code this install
never verified. Unreadable or unrelated falls back rather than guessing.

Fourth instance of one pattern today. .mcp.json recorded the bare wrapper and
followed every upgrade untouched; Hermes external_dirs, the plugin cache, and
this each held a version or a copy, and each needed a fix. The configuration
that pinned nothing is the one that survived.

Existing hooks still need `commitlore hooks install` re-run -- doctor already
prints exactly that as its fix. This only changes what new installs record.

Verified by restoring the old resolution: the wrapper case fails.

Limit: a hook records a launcher, never a release
Blast: module
Undo: easy
Certainty: firm
Provenance: authored
Record-Id: r-693hook
@github-actions

Copy link
Copy Markdown

CommitLore — record lint

Trailers: clean — 1 commit in origin/main..5098f406e6af37e190398a0607450d0194559405
Active constraints: not read — commitlore: git log --follow accepts exactly one pathspec, so renames are not followed for 6 paths; query one path at a time to follow its rename chain (6 changed paths)

Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR.

@MongLong0214

Copy link
Copy Markdown
Owner Author

Closing: the approach is wrong, and CI caught why.

commitlore.bin: /Users/runner/work/_temp/home/.local/bin/commitlore
outcome: the hook exited 1 under the restricted PATH —
         commitlore: cannot find the CLI this hook was installed with.

Recording the wrapper worked. Executing it did not. The wrapper is a shell script that finds node on PATH, and the hook runs in an environment that deliberately has no useful PATH — which is the entire reason commitlore.node is recorded beside commitlore.bin. A .mjs bundle can be launched with a recorded interpreter; a shell script cannot.

My PR body claimed the wrapper 'keeps the property the versioned path was chosen for'. That was wrong. Being an absolute path and being launchable by a recorded interpreter are different properties, and I asserted the second from the first.

Three local reproductions passed — no wrapper, foreign wrapper, and the unit suite — because my PATH had node in it. The restricted-PATH hook environment exists precisely to make that assumption visible, and it did.

What #693 needs instead

Not this. Either the stub grows a branch that executes a shell-script entry point directly (no recorded interpreter involved), or the launcher recorded is a version-free path to a .mjs that the wrapper also targets — a stable symlink under the data root rather than the bin wrapper.

The second is closer to what the other three instances of this pattern did, and it does not require the stub to learn a new execution mode. Reopening #693 with that.

Nothing about the defect changed: commitlore.bin still pins a version and an upgrade still leaves the hook behind.

@MongLong0214
MongLong0214 deleted the fix-693-hook-wrapper branch August 15, 2026 14:42
MongLong0214 added a commit that referenced this pull request Aug 16, 2026
…696)

* Give the hook a path that outlives the release it was installed from

commitlore.bin recorded <data-root>/v<version>/dist/commitlore.mjs, so an
upgrade left every repository validating commits with the build it was installed
from. Measured on the first machine to upgrade: three repositories pinned to
0.8.2 and 0.8.0 while the CLI was 1.0.1 -- this repository among them, through
two releases. doctor said so on every run and stopped nothing.

install.sh now maintains <data-root>/current beside the versioned checkouts, and
hooks record that. It stays an absolute path to a .mjs, so the recorded
interpreter launches it and the hook remains independent of PATH -- the two
properties the versioned path was chosen for.

The bin wrapper cannot serve here and that is not a guess: #694 recorded it and
hooks failed under the restricted PATH a hook actually runs in, because a shell
script cannot be launched with a recorded interpreter.

Two things deliberately narrow. The link is only recorded when it resolves to
the running bundle -- a current belonging to another install would send the hook
to code this one never verified, which is worse than a version pin. And a host
where the symlink cannot be made still installs: hooks there keep recording the
versioned path, and `commitlore hooks install` after an upgrade remains the
repair doctor already names.

The path is derived from the layout rather than from the directory being called
commitlore, after the first attempt matched on that name and failed anywhere the
data root is named otherwise.

Existing pins still need `commitlore hooks install` re-run. This changes what is
recorded from now on.

Limit: a hook records a path that does not name a release
Blast: system
Undo: easy
Certainty: firm
Provenance: authored
Record-Id: r-693curr

* Make the rejection case prove the rejection

The test asserting a foreign current is refused passed for the wrong reason: the
fixture pointed current at a path with no bundle, so realpathSync threw and the
fallback happened because the link was broken. The identity comparison never
ran, and removing it left the test green.

The other installation is now a working one. Only the comparison can reject it.

Two mutations, two different failures:
  removing the derivation      -> the version-free case fails
  removing the identity check  -> the foreign-current case fails

Before this, one mutation covered both and the second layer was unpinned. Caught
in review by asking which mutation each assertion answers -- a question the
passing suite could not answer for itself.

Limit: a rejection test names what does the rejecting
Blast: local
Undo: easy
Certainty: firm
Provenance: authored
Record-Id: r-693mut2
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.

The hook records a version-pinned path, so an upgrade leaves it behind

1 participant