Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/vcs-extension-ownership.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": minor
---

Decouple bundled VCS providers from core, add provider-neutral repository bootstrapping, and let extension source readers report files that exceed their safe read limit.
3 changes: 3 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ CLI input

- CLI entrypoints: `diff`, `show`, `stash show`, `patch`, `pager`, `difftool`.
- All input sources normalize into one internal changeset model.
- Bundled VCS implementations live under `src/extensions/default/vcs/<provider>/` and consume the
public extension contract; `src/app` composes their registrations into the provider-neutral
core VCS catalog. Do not add provider commands, spawning, or source readers under `src/core`.
- Pager mode has two paths: full diff UI for patch-like stdin, plain-text fallback for non-diff pager content.
- View defaults are layered through built-ins, user config, repo `.hunk/config.toml`, command sections, pager sections, and CLI flags.
- `hunk daemon serve` runs one loopback daemon that brokers agent commands to many live Hunk sessions. Normal Hunk sessions should auto-start and register with that daemon when session brokering is enabled. Keep it local-only and session-brokered rather than opening per-TUI ports.
Expand Down
48 changes: 29 additions & 19 deletions docs/extension-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ object and registry collection (`src/extensions/runExtension.ts`):
`src/extensions/trust.ts`.
- **Bundled extensions** live in `src/extensions/default/` and are compiled
into the binary. `default/vcs/{git,jujutsu,sapling}` is statically imported
and loaded synchronously _from VCS adapter resolution_
(`default/vcs/index.ts`), so backends exist during config resolution — that
load path must stay renderer-free. `default/ui/index.ts` is deliberately not
part of that list: it synchronously loads the bundled files pane through
`runExtensionFactory` only where the app resolves UI panes.
by the app composition root (`app/vcsCatalog.ts`) and loaded synchronously
before config resolution, so backends exist without making core import the
extension host. `default/ui/index.ts` is deliberately not part of that list:
it synchronously loads the bundled files pane through `runExtensionFactory`
only where the app resolves UI panes.

Git and the built-in file navigation use the public `registerVcsAdapter` and
`registerPane` paths. The current-line lens remains an installable example.
Expand All @@ -34,7 +34,7 @@ owns for commands (`<extensionId>.<commandId>`), panes
(`<extensionId>:<viewId>`), and config (`[extension.<id>]`). `host.ts` is the
one place those ids are vetted — discovery stays a pure filesystem walk, and
every way an id can be derived arrives there as `candidate.id`. It refuses
reserved ids (`hunk`, plus the bundled backends via `isVcsId`), ids outside
reserved ids (`hunk`, plus the base catalog's bundled backend ids), ids outside
`/^[A-Za-z0-9][A-Za-z0-9_-]*$/` (a dot or colon would make the composed ids
unsplittable), and the later of two sources claiming one id; each refusal is a
load issue and costs only that extension. The rules themselves are stated in
Expand All @@ -45,9 +45,13 @@ load issue and costs only that extension. The rules themselves are stated in
Registrations (themes, file languages, VCS adapters, changeset transforms,
panes, commands, lifecycle/UI events, and bus listeners) collect into one
`ExtensionRegistry` (`src/extensions/types.ts`) and are resolved/applied
through `src/extensions/apply.ts` on both startup and reload. A factory that
throws is rolled back to its pre-run registration counts
(`runExtension.ts`); failures cost a warning, not the session.
through `src/extensions/apply.ts` on both startup and reload. Staged external-VCS
bootstrap retains the provisional candidate/config snapshot: a final pass that
only appends repo candidates extends the same registry, while a changed prefix
receives bounded `shutdown` before being rebuilt. Live registry replacement uses
the same shutdown/startup lifecycle. A factory that throws is rolled back to its
pre-run registration counts (`runExtension.ts`); failures cost a warning, not the
session.

## Host-served runtime modules

Expand Down Expand Up @@ -200,16 +204,22 @@ none — which is why the visible menu list is derived from the menus record

## VCS adapters

`src/core/vcs/index.ts` is the single assembly point ordering bundled + user
adapters by `detectionPriority` (Git is the baseline at 0; jj 200 / sl 100
sit above it for colocated checkouts — the constants in
`src/extension-api/types.ts` document the reasoning). Detection is uniform
across tiers: nearest checkout wins, priority breaks equal-distance ties, an
explicit `vcs` id a loaded backend owns beats detection
(`src/extensions/apply.ts`). `src/extensions/vcsPatchResult.ts` is the one
conversion boundary where a published `ExtensionVcsPatchResult` becomes
Hunk's internal diff model — anything a backend needs that cannot be
expressed publicly is a real gap in the contract.
`src/core/vcs/index.ts` owns provider-neutral catalog ordering, lookup,
detection, and operation dispatch. `src/app/vcsCatalog.ts` composes bundled
registrations, while `src/app/sessionBootstrap.ts` extends that catalog with
accepted user adapters and threads the same value through loading, reload, and
watch. Detection is uniform across tiers: nearest checkout wins, priority breaks
equal-distance ties, and an explicit `vcs` id owned by the catalog wins.

Provider implementations — command construction, spawning, error translation,
and exact-source reading — live entirely under
`src/extensions/default/vcs/<provider>/`. `src/extensions/vcsPatchResult.ts` is
the one conversion boundary where a published `ExtensionVcsPatchResult`
becomes Hunk's internal diff model, including structural `too-large` source
results. `src/core/projectRoot.ts` treats `.hunk` as a provider-independent
bootstrap marker and also consults the available catalog; startup performs a
second root/config pass when a global, config-path, or CLI adapter recognizes a
repository unavailable to the bundled catalog.

## Public contract rules

Expand Down
57 changes: 35 additions & 22 deletions docs/extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,10 @@ writes to the extension and asks for consent.

The factory receives one object. Registration calls are only valid while the
factory is running; Hunk seals the object afterwards so a deferred callback
cannot mutate the registry mid-session.
cannot mutate the registry mid-session. Keep the factory registration-only:
start watchers, processes, connections, and other long-lived resources from
`startup`, and release them from `shutdown`. Extension-registry reloads create
new instances and run that shutdown/startup pair around the replacement.

### `hunk.apiVersion`

Expand Down Expand Up @@ -290,7 +293,7 @@ carries two sets of markers.
| ------------------------ | -------------------------------------------- |
| bundled `jj` | 200 |
| bundled `sl` | 100 |
| bundled `git` | 0 (`HUNK_CORE_VCS_DETECTION_PRIORITY`) |
| bundled `git` | 0 (`HUNK_VCS_DETECTION_BASELINE_PRIORITY`) |
| your adapter, by default | -100 (`HUNK_DEFAULT_VCS_DETECTION_PRIORITY`) |

Higher is consulted first; equal priorities fall back to registration order.
Expand All @@ -303,12 +306,12 @@ silently changes how an existing repository is reviewed. Set
`detectionPriority` explicitly to outrank a shipped backend; it is your machine.

```ts
import { HUNK_CORE_VCS_DETECTION_PRIORITY } from "hunkdiff/extension";
import { HUNK_VCS_DETECTION_BASELINE_PRIORITY } from "hunkdiff/extension";

hunk.registerVcsAdapter({
id: "hg",
name: "Mercurial",
detectionPriority: HUNK_CORE_VCS_DETECTION_PRIORITY + 10,
detectionPriority: HUNK_VCS_DETECTION_BASELINE_PRIORITY + 10,
detect,
});
```
Expand All @@ -322,7 +325,13 @@ adapter list — and that second answer is the one the session uses.

What detection never overrides is an explicit choice: a `vcs = "<id>"` in Hunk
config naming a backend this session loaded is honored as-is, however near a
checkout some other adapter finds.
checkout some other adapter finds. A repository-local adapter can bootstrap a
provider Hunk has never seen because `.hunk` itself establishes the project root;
global, config-path, and `--extension` adapters also participate in a staged
root/config pass before the review loads. When the final root only adds repo
candidates, Hunk extends the provisional registry instead of executing its
already loaded factories again. If repo config changes an existing extension's
factory config, Hunk sends that provisional instance `shutdown` before rebuilding it.

#### Watch support

Expand Down Expand Up @@ -381,7 +390,10 @@ async load(input, ctx) {
```

Return `null` for a side that has no content — the old side of an added file, a
path the revision never contained — rather than throwing. Hunk calls the reader
path the revision never contained — rather than throwing. Return
`{ kind: "too-large", maxBytes }` when fetching the source would exceed your
resource limit; Hunk shows expansion as unavailable without treating the result
as an extension failure. Hunk calls the reader
**at most once per file and side** and caches what it resolves, so you do not
need your own cache, and it never calls it for a file the diff reports as
binary. When equivalent reloads close over the same source base, return the same
Expand Down Expand Up @@ -1345,20 +1357,20 @@ the UI waiting for one. Alongside `cwd` and `notify`, every handler receives
That means a `changeset_loaded` handler can reveal its extension's pane when
it finds something worth showing — no keypress required.

| Event | Payload | When |
| ---------------------- | ----------------------- | -------------------------------------------------------- |
| `startup` | `{ cwd }` | once, after the app mounts with its first changeset |
| `changeset_loaded` | `{ changeset }` | first load and every reload |
| `selection_changed` | `{ fileId, hunkIndex }` | when the review selection settles (debounced ~150ms) |
| `file_viewed` | `{ file, hunkIndex }` | when selection settles on a file or a reload replaces it |
| `filter_changed` | `{ filter }` | whenever the file-filter query changes |
| `theme_changed` | `{ themeId }` | when the user commits a new theme |
| `layout_changed` | `{ mode, layout }` | mode or responsive split/stack layout changes |
| `watch_reload_pending` | `{}` | watcher observed a change before its reload check |
| `note_created` | `{ note }` | a user saves an inline review note |
| `note_edited` | `{ note }` | an in-progress inline note's body changes |
| `session_reload` | `{ changeset, reason }` | on every session reload |
| `shutdown` | `{}` | on exit, best-effort within a short timeout |
| Event | Payload | When |
| ---------------------- | ----------------------- | --------------------------------------------------------- |
| `startup` | `{ cwd }` | once per loaded instance, after its review UI mounts |
| `changeset_loaded` | `{ changeset }` | first load and every reload |
| `selection_changed` | `{ fileId, hunkIndex }` | when the review selection settles (debounced ~150ms) |
| `file_viewed` | `{ file, hunkIndex }` | when selection settles on a file or a reload replaces it |
| `filter_changed` | `{ filter }` | whenever the file-filter query changes |
| `theme_changed` | `{ themeId }` | when the user commits a new theme |
| `layout_changed` | `{ mode, layout }` | mode or responsive split/stack layout changes |
| `watch_reload_pending` | `{}` | watcher observed a change before its reload check |
| `note_created` | `{ note }` | a user saves an inline review note |
| `note_edited` | `{ note }` | an in-progress inline note's body changes |
| `session_reload` | `{ changeset, reason }` | on every session reload |
| `shutdown` | `{}` | before instance replacement or exit, with a short timeout |

`selection_changed` is trailing-debounced on purpose: holding `[`/`]` retargets
the selection many times a second, and handlers only care where the user landed.
Expand All @@ -1375,8 +1387,9 @@ these events, and a `session_reload` may remap or drop notes without one
either. A list accumulated from these events is therefore "notes the user saved
here this session", not a complete review record; present it as such.

`shutdown` handlers get a short window (250ms) to finish before Hunk exits
anyway, so treat it as best-effort flushing rather than guaranteed cleanup.
`shutdown` handlers get a short window (250ms) to finish before Hunk replaces
the extension registry or exits anyway, so make cleanup prompt and idempotent.
The replacement instance receives `startup` after its review is mounted.

### `hunk.events`

Expand Down
20 changes: 11 additions & 9 deletions docs/source-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Use it when adding a new module or deciding where an existing responsibility bel
src/app/ executable composition: startup plans and shared session bootstrap
src/core/ normalized review model, loading, patch handling, VCS contracts,
configuration, and runtime primitives
src/core/vcs/ VCS-specific helpers and adapter-facing support code
src/core/vcs/ provider-neutral VCS catalog, contracts, operation dispatch, and host support
src/extensions/ extension host, registry, trust, lifecycle, and bundled extensions
src/session/ shared session protocol, schemas, types, agent surface, app bridge, and broker transport
src/session/client/ shared session-daemon HTTP and compatibility client support
Expand All @@ -32,16 +32,17 @@ one owns its behaviour.

- `app` may compose `core`, `extensions`, `session`, and `ui`.
- `ui` may consume core models and the extension/session contracts; it owns terminal rendering.
- `extensions` may consume core model and VCS contracts, but must stay renderer-free except for
`extensions/default/ui/`, which is the explicit bundled-sidebar boundary.
- `core` must not import `ui`. Shared data needed by both belongs in `core`, not `ui/lib`.
- `extensions` may consume provider-neutral core models and contracts, but bundled VCS provider
implementations must depend only on `hunkdiff/extension`, local modules, and `src/lib` utilities.
Renderer access remains limited to `extensions/default/ui/`, the bundled-sidebar boundary.
- `core` must not import `ui` or `extensions`. Shared data needed by both belongs in core-owned
structural contracts or `src/lib`, never in a reverse dependency.
- `extension-api/types.ts` stays import-free. It is a published declaration boundary, enforced by
the package checks.
- `opentui` and `extension-api` are public entrypoint directories, not general internal buckets.

The boundary test intentionally enforces the currently mechanical rule (`core` cannot import
`ui`). Broader direction is reviewed at feature boundaries until the session consolidation is
complete.
`scripts/source-boundaries.test.ts` mechanically enforces `core -> ui`, `core -> extensions`,
and bundled-provider -> core boundaries, including the public extension-barrel requirement.

## Bootstrap invariant

Expand All @@ -63,5 +64,6 @@ This is an incremental migration, not a bulk rename:
4. Prefer a named ownership boundary over a generic `lib` folder.
5. Update this map and feature architecture docs when a boundary changes.

Current first cuts place startup composition in `app/`, the shared Shiki theme catalog in
`core/themeCatalog.ts`, and bundled VCS implementation helpers in `core/vcs/`.
Current composition lives in `app/`: `app/vcsCatalog.ts` assembles bundled registrations into a
provider-neutral catalog, and `app/sessionBootstrap.ts` extends that catalog with user adapters.
Provider commands, source readers, and tests live under `extensions/default/vcs/<provider>/`.
48 changes: 48 additions & 0 deletions packages/session-broker-core/src/brokerState.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,54 @@ describe("session broker state", () => {
);
});

test("resolves repo subdirectories to the nearest eligible registered root", () => {
const outer = createListedSession({ sessionId: "outer", repoRoot: "/repo", cwd: "/repo" });
const inner = createListedSession({
sessionId: "inner",
repoRoot: "/repo/packages/app",
cwd: "/repo/packages/app",
});

expect(
resolveSessionTarget([outer, inner], {
repoRoot: "/repo/packages/app/src",
repoBoundary: "/repo/packages/app",
}).sessionId,
).toBe("inner");
expect(
resolveSessionTarget([outer], {
repoRoot: "/repo/other",
repoBoundary: "/repo",
}).sessionId,
).toBe("outer");
expect(() =>
resolveSessionTarget([outer], {
repoRoot: "/repo/packages/app/src",
repoBoundary: "/repo/packages/app",
}),
).toThrow("No active session matches repoRoot");

// An external adapter may own a nested root inside the nearest bundled
// boundary. Its active session remains eligible and wins by distance.
const custom = createListedSession({
sessionId: "custom",
repoRoot: "/repo/custom",
cwd: "/repo/custom",
});
expect(
resolveSessionTarget([outer, custom], {
repoRoot: "/repo/custom/src",
repoBoundary: "/repo",
}).sessionId,
).toBe("custom");

// Older clients omit the boundary; containment fallback remains compatible.
expect(resolveSessionTarget([outer], { repoRoot: "/repo/packages/app/src" }).sessionId).toBe(
"outer",
);
expect(resolveSessionTarget([outer], { repoRoot: "/repo/..cache" }).sessionId).toBe("outer");
});

test("keeps session-path matching tied to the live session cwd", () => {
const sessions = [
createListedSession({
Expand Down
23 changes: 15 additions & 8 deletions packages/session-broker-core/src/brokerState.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { randomUUID } from "node:crypto";
import { matchesSessionSelector, type SelectableSession } from "./selectors";
import { matchesSessionSelector, repoSelectorDistance, type SelectableSession } from "./selectors";
import type {
SessionRegistration,
SessionServerMessage,
Expand Down Expand Up @@ -60,11 +60,7 @@ export interface SessionBrokerViewAdapter<

export type UpdateSnapshotResult = "updated" | "invalid" | "not-found";

export interface SessionTargetSelector {
sessionId?: string;
sessionPath?: string;
repoRoot?: string;
}
export type SessionTargetSelector = SessionTargetInput;

function describeSessionChoices<ListedSession extends SessionBrokerListedSession>(
sessions: ListedSession[],
Expand Down Expand Up @@ -104,11 +100,22 @@ export function resolveSessionTarget<ListedSession extends SessionBrokerListedSe
}

if (selector.repoRoot) {
const matches = sessions.filter((session) => matchesSessionSelector(session, selector));
if (matches.length === 0) {
const candidates = sessions
.map((session) => ({
session,
distance: repoSelectorDistance(session, selector.repoRoot!, selector.repoBoundary),
}))
.filter(
(entry): entry is { session: ListedSession; distance: number } => entry.distance !== null,
);
if (candidates.length === 0) {
throw new Error(`No active session matches repoRoot ${selector.repoRoot}.`);
}

const nearestDistance = Math.min(...candidates.map((entry) => entry.distance));
const matches = candidates
.filter((entry) => entry.distance === nearestDistance)
.map((entry) => entry.session);
if (matches.length > 1) {
throw new Error(
`Multiple active sessions match repoRoot ${selector.repoRoot}; specify sessionId instead. ` +
Expand Down
Loading
Loading