Add managed extension installation with hunk extension commands - #712
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryThis PR adds managed Git-based extension installation, persistent ownership records, extension discovery integration, and manifest API-version gating.
Confidence Score: 2/5The PR should not merge until local tilde sources, failure-safe promotion, and concurrent record updates are corrected. Documented local installs fail for tilde paths, a failed staged rename can delete the working extension, and overlapping commands can lose ownership records or interfere with shared staging directories. Files Needing Attention: src/extensions/manage/source.ts, src/extensions/manage/install.ts, src/extensions/manage/cli.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
CLI[hunk extension command] --> Parse[Parse source and optional ref]
Parse --> Stage[Clone into .staging-name]
Stage --> Validate[Validate extension layout]
Validate --> Dependencies[Install production dependencies]
Dependencies --> Promote[Promote into installed/name]
Promote --> Records[Write records.json]
Records --> Discovery[Global extension discovery]
Discovery --> Gate[API version and ID gate]
Gate --> Load[Load extension]
Prompt To Fix All With AI### Issue 1
src/extensions/manage/source.ts:101-102
**Tilde paths remain unexpanded**
When a user runs the documented `hunk extension install ~/dev/hunk-word-diff` form, the parser preserves `~` in `cloneUrl` and passes it directly to Git without shell expansion, causing Git to report that the local repository does not exist.
### Issue 2
src/extensions/manage/install.ts:220-221
**Promotion deletes the active install**
If `renameSync` fails during an update, `promoteStagedClone` has already recursively deleted the active installation, leaving the staged clone orphaned and the unchanged record pointing to a missing directory.
### Issue 3
src/extensions/manage/install.ts:225-232
**Concurrent commands lose install records**
If two extension-management processes overlap, each can rewrite `records.json` from a stale full-map snapshot, causing one successful installation to lose its ownership record and disappear from `list`, `update`, and `remove`; same-name operations also share and delete the same deterministic staging directory.
### Issue 4
src/extensions/manage/cli.ts:62
**Direct global environment access**
The production command path falls back directly to `process.env`, bypassing the repository's required validated environment configuration boundary and adding untyped global state that callers and tests must account for.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(extensions): require deliberate exte..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (hasExplicitTransport(location) || isLocalPath(location)) { | ||
| cloneUrl = location; |
There was a problem hiding this comment.
When a user runs the documented hunk extension install ~/dev/hunk-word-diff form, the parser preserves ~ in cloneUrl and passes it directly to Git without shell expansion, causing Git to report that the local repository does not exist.
Knowledge Base Used: Extension System: API, Loading, and Host Integration
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/extensions/manage/source.ts
Line: 101-102
Comment:
**Tilde paths remain unexpanded**
When a user runs the documented `hunk extension install ~/dev/hunk-word-diff` form, the parser preserves `~` in `cloneUrl` and passes it directly to Git without shell expansion, causing Git to report that the local repository does not exist.
**Knowledge Base Used:** [Extension System: API, Loading, and Host Integration](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/extension-system.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in a8e4d19. Local path sources now go through the same expandHomePath discovery uses for config paths, and are additionally resolved to absolute before being recorded — so the documented ~/dev/... form clones correctly and a later hunk extension update works from any working directory. Covered by a new test asserting both the tilde expansion and the absolute-path recording.
Generated by Claude Code
| rmSync(directory, { recursive: true, force: true }); | ||
| renameSync(stagingDir, directory); |
There was a problem hiding this comment.
If renameSync fails during an update, promoteStagedClone has already recursively deleted the active installation, leaving the staged clone orphaned and the unchanged record pointing to a missing directory.
Context Used: guidelines.mdc Cursor rule (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/extensions/manage/install.ts
Line: 220-221
Comment:
**Promotion deletes the active install**
If `renameSync` fails during an update, `promoteStagedClone` has already recursively deleted the active installation, leaving the staged clone orphaned and the unchanged record pointing to a missing directory.
**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in a8e4d19. promoteStagedClone now moves the existing install aside to a dot-prefixed backup directory, renames the staged clone into place, and only then deletes the backup; if the swap rename fails, the previous install is restored and the staging dir cleaned up, so an update either lands whole or leaves the prior install untouched. Discovery also explicitly skips the installer's dot-prefixed workspace directories so a backup or staging dir can never load as an extension.
Generated by Claude Code
| function saveRecord( | ||
| context: ExtensionManageContext, | ||
| records: ExtensionInstallRecordMap, | ||
| name: string, | ||
| record: ExtensionInstallRecord, | ||
| ) { | ||
| writeInstallRecords(context.installedRoot, { ...records, [name]: record }); | ||
| } |
There was a problem hiding this comment.
If two extension-management processes overlap, each can rewrite records.json from a stale full-map snapshot, causing one successful installation to lose its ownership record and disappear from list, update, and remove; same-name operations also share and delete the same deterministic staging directory.
Context Used: guidelines.mdc Cursor rule (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/extensions/manage/install.ts
Line: 225-232
Comment:
**Concurrent commands lose install records**
If two extension-management processes overlap, each can rewrite `records.json` from a stale full-map snapshot, causing one successful installation to lose its ownership record and disappear from `list`, `update`, and `remove`; same-name operations also share and delete the same deterministic staging directory.
**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Addressed in a8e4d19. Staging directories are now pid-suffixed, so overlapping commands for the same name can't clone into or delete each other's workspace, and saveRecord merges over a fresh read of records.json at write time instead of the snapshot taken before the clone — the long clone-in-the-middle window is gone. The remaining read-to-write gap matches the documented, accepted posture of Hunk's shared state file (updateHunkStateRecord), which deliberately trades a tiny unlocked window over a lock file that needs crash reaping.
Generated by Claude Code
| input: ExtensionManageCommandInput, | ||
| io: ExtensionManageIo, | ||
| ): Promise<number> { | ||
| const env = io.env ?? process.env; |
There was a problem hiding this comment.
The production command path falls back directly to process.env, bypassing the repository's required validated environment configuration boundary and adding untyped global state that callers and tests must account for.
Context Used: guidelines.mdc Cursor rule (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/extensions/manage/cli.ts
Line: 62
Comment:
**Direct global environment access**
The production command path falls back directly to `process.env`, bypassing the repository's required validated environment configuration boundary and adding untyped global state that callers and tests must account for.
**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Leaving this one as is: the cited guideline is from a different repository's rules, and hunk's convention is the opposite — env parameters defaulting to process.env are the established pattern across this codebase (resolveGlobalExtensionsDir, resolveRepoTrust, discoverExtensions, loadStartupExtensions all do exactly this). The io.env seam exists so tests inject an isolated environment, which they do.
Generated by Claude Code
There was a problem hiding this comment.
Fair point — if env defaulting to process.env is the consistent pattern throughout this codebase, then flagging it here is noise. The io.env seam already gives tests the isolation they need. I'll drop the concern.
Shall I remember this as a rule: "In this codebase, env parameters defaulting to process.env are the accepted pattern — do not flag them as a violation"?
a8e4d19 to
0a18632
Compare
Shared extensions need a way to state the minimum extension API they were built against, so an older Hunk refuses them with one actionable startup notice instead of failing somewhere inside the factory. The manifest's hunk field now accepts apiVersion alongside extensions, discovery carries it on the candidate, and the host gates it with the id checks before anything is imported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
Sharing an extension previously meant telling users to clone and cp by hand. hunk extension install clones a git source (owner/repo shorthand, git:host/path, full URLs, or a local path, each with an optional @ref) into a managed directory under the global extensions dir, validates it actually contains an extension, installs its npm dependencies when it declares any, and records the source and resolved commit so list, update, and remove can operate on exactly what Hunk installed. Installs are confirmed interactively (or with --yes) because extensions run with full user permissions, and discovery loads the managed directory through the existing global origin so no new trust surface is introduced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
Sharing an extension now has a documented path end to end: the repo guide, website guide, README, and hunk-extensions skill all describe the folder-extension repository layout, manifest metadata, hunk extension install sources, and the hunk-extension GitHub topic that serves as the zero-infrastructure community listing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
An update stages a fresh clone and compares its commit before swapping, but dependencies were installed during staging — before the comparison — so a pinned or unchanged install paid a full bun install on every update run. Stage now clones and validates only, and dependencies install just before a staged clone is promoted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
Install validation accepted any repository with a bare src/index.ts one level down — the shape of nearly every JavaScript project, including pi extensions whose manifests use a pi field instead of hunk — and such installs could only fail later at load time. The installer now requires a root hunk manifest, a root index entry, top-level entry files, or a subfolder with its own hunk manifest before recording a clone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
Package-manager muscle memory expects a short spelling, and the daemon command already set the precedent with its mcp alias. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
Address review findings: local install sources now expand ~ and are recorded absolute so a later update works from any directory; promotion moves the previous install aside and restores it if the swap fails instead of deleting it first; records merge over a fresh read at write time and staging directories are pid-suffixed, so overlapping commands cannot drop each other's records or share a workspace; discovery skips the installer's dot-prefixed workspace directories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
Repository-name derivation split only on / and :, so a Windows local source like C:\dev\hunk-ext produced a backslash-riddled name that failed id validation and broke every install on Windows. Name derivation and ref splitting now treat both separators, Windows-style relative prefixes count as local paths, and the local-path test builds its fixture with join so CI exercises real separators on every platform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb
6d3e285 to
8f8375a
Compare
2 participants
Implement a complete extension management system that allows users to install, list, update, and remove shared extensions from git repositories.
Summary
This change adds four new
hunk extensionsubcommands that enable users to discover and manage extensions shared as plain git repositories, without requiring a centralized registry. Extensions install into a managed directory (~/.config/hunk/extensions/installed/), with their sources and commits recorded for reproducible updates.Key Changes
Extension installation CLI (
src/extensions/manage/install.ts): Core operations for cloning extension repositories, validating they contain extensions, installing npm dependencies, and managing install records. Includes staging/promotion pattern to ensure failed installs leave the filesystem untouched.Install source parsing (
src/extensions/manage/source.ts): Flexible source spec parser supporting GitHub shorthand (owner/repo), git-prefixed hosts (git:host/path), explicit git URLs, and local paths, all with optional@refpinning for branches/tags/commits.Install records (
src/extensions/manage/records.ts): Persistent JSON records tracking each managed install's source, clone URL, pinned ref, current commit, and timestamps. Records are the source of truth for which directories Hunk owns.CLI integration (
src/extensions/manage/cli.ts): User-facing command runner with interactive confirmation for installs (required for security since extensions execute with full user permissions), plus output formatting for list/update/remove operations.Manifest API versioning: Extensions can declare a minimum extension API version via
"hunk": { "apiVersion": N }in theirpackage.json. Hunk refuses to load extensions requiring a newer API, with a clear startup notice instead of runtime failures.Discovery integration (
src/extensions/discovery.ts): Updated to attachrequiresApiVersionfrom manifests to candidates, and gated loading insrc/extensions/host.tsto reject candidates whose API requirements exceed the currentHUNK_EXTENSION_API_VERSION.Tests: Comprehensive test coverage for install/update/remove operations, source parsing, manifest API versioning, and CLI parsing.
Documentation: Updated extension authoring guide and CLI reference with install command syntax, examples, and guidance on declaring API version requirements.
Implementation Details
.staging-<name>directory; only successful clones are promoted to their final location, ensuring atomicity.bun install --productionwhen declared; missingbundegrades to a warning rather than failing the install.--branchclones are attempted first for efficiency; on failure (bare commits, shallow-fetch refusal), falls back to full clone + checkout.--yesflag; confirmation is skipped in non-TTY environments when--yesis provided.https://claude.ai/code/session_01KLy4tadVCxapwRdfT9C7nb