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
17 changes: 17 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Agents

Pointers for AI coding agents working in this repository.

## Agent skills

### Issue tracker

Issues live in the `Cannon07/code-preview` GitHub repo; skills use the `gh` CLI. See `docs/agents/issue-tracker.md`.

### Triage labels

Default canonical label vocabulary (`needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`). See `docs/agents/triage-labels.md`.

### Domain docs

Single-context layout — `CONTEXT.md` and `docs/adr/` at the repo root. See `docs/agents/domain.md`.
200 changes: 200 additions & 0 deletions CONTEXT.md

Large diffs are not rendered by default.

25 changes: 25 additions & 0 deletions docs/adr/0001-origin-prefixed-status-values.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Bash proposals are indicator-only, with origin-prefixed statuses

Some agents — observed first with GPT-class models in the Codex integration — route file edits through the `Bash` tool instead of structured `Edit`/`Write`/`MultiEdit` calls. The plugin cannot safely render a content preview for arbitrary shell: doing so would require either dry-running the command in a sandbox (expensive, unsafe) or snapshotting and re-inspecting around the agent's execution (intrusive, racy). Two decisions follow from that constraint:

1. **Bash proposals never produce a [preview](../../CONTEXT.md#preview).** They only update [change indicators](../../CONTEXT.md#indicator) in the registry, so the user at least sees in neo-tree that the file was touched. This is the "Tier 1" path in `bin/core-pre-tool.sh`. A future "Tier 2" with real content diffs is named but not committed to.
2. **Bash-origin statuses are prefixed** (`bash_modified`, `bash_created`). The registry is a flat `{path → status}` map shared across all in-flight proposals; the Bash post-tool needs to clear *only its own* markers without clobbering markers from a concurrent Edit/Write/ApplyPatch whose post-tool hasn't fired. The prefix lets it do `clear_by_statuses(["deleted", "bash_modified", "bash_created"])` instead of a blind path-keyed clear.

## Considered Options

For the indicator-only policy:

- **Dry-run the command in a sandbox to compute the proposed diff.** Rejected: arbitrary shell can read network, mutate global state, and consume real time. Not safe to run speculatively, and would block the agent on every Bash proposal.
- **Snapshot before + diff after the agent runs.** Rejected: requires hooking *both* pre and post around the actual write, and the diff would be shown after the agent has already committed — too late to be a preview in any meaningful sense.

For encoding origin in the registry:

- **Single status with a separate origin column** (`{path → {status, origin}}`). Rejected: doubles the shape of every entry to serve one call site.
- **Per-tool sub-registries.** Rejected: complicates the common-case lookup (rendering an indicator) for a problem only the post-tool clear has.

## Consequences

- New origins (e.g. a future LSP-driven write detector) follow the same convention: `<origin>_modified`, `<origin>_created`.
- Renderers must map prefixed statuses to the same indicator as their un-prefixed counterpart, or they'll silently miss change icons. Today this mapping is centralised in `lua/code-preview/neo_tree.lua`.
- The `deleted` status is intentionally un-prefixed: `rm` detection lives in the Bash core path but `*** Delete File:` patch directives produce the same status from a structured-tool path, so origin isn't a clean discriminator there.
- If we ever build Tier 2 (real content diffs for shell writes), Bash proposals would start producing previews and the indicator-only assumption would need revisiting — but the prefix convention would still hold for the period when both tiers coexist.
17 changes: 17 additions & 0 deletions docs/adr/0002-default-force-review-gate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Default to forcing the Claude Code review gate

For the Claude Code integration, the pre-tool hook emits `permissionDecision: "ask"` by default — forcing Claude Code to prompt the user on every edit, even when the user has configured permissive settings (bypass mode, allowlists). Users can opt out with `diff.defer_claude_permissions = true`, which suppresses the JSON output and lets Claude Code's own settings decide.

The reason: the plugin's value proposition is *previewing edits before they land*. If Claude Code is in bypass mode or has the edit allowlisted, the [review gate](../../CONTEXT.md#review-gate) never opens — the agent writes the file immediately and the preview appears alongside an already-committed change, which is worse than no preview at all (the user might assume the diff is pending when it isn't). Defaulting to a forced gate ensures the preview *means something*: there is a real accept/reject moment that lines up with what's on screen.

## Considered Options

- **Default = respect Claude Code's permission settings.** Rejected: users who installed the plugin specifically to get previews would silently lose them whenever Claude Code's settings let an edit through. Surprising and quiet, which is the worst kind of behaviour.
- **No config knob; always force the gate.** Rejected: a user who deliberately wants Claude Code's permission machinery to be authoritative (e.g. relying on a careful allowlist) has no escape hatch. The opt-out exists for that case.
- **Default = force, opt-out via config** *(chosen)*. Prioritises the plugin's core promise; lets advanced users disable when they understand the trade-off.

## Consequences

- A user who installs the plugin while running Claude Code in `--dangerously-skip-permissions` (or equivalent bypass) will start seeing prompts they previously avoided. This is intended: the plugin's preview only works if there's a gate to hold open. README and `:CodePreviewStatus` should make the override discoverable.
- Other agents (OpenCode, Codex, Copilot CLI) are not affected — their gate mechanism is owned by the agent itself, not by output the plugin emits.
- If a new agent is added where the plugin can choose between forcing and deferring (analogous to Claude Code), this ADR sets the precedent: default to forcing.
19 changes: 19 additions & 0 deletions docs/adr/0003-inline-renderer-as-future-default.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# The inline renderer is the strategic direction; side-by-side is legacy

The plugin has two preview [renderers](../../CONTEXT.md#renderer). The inline renderer is where new rendering features land; the side-by-side renderer is retained for users who depend on it but is no longer being invested in. The default [layout](../../CONTEXT.md#layout) (`"tab"`) still uses side-by-side today — flipping the default to `"inline"` is a future migration, not a current one.

The reason: inline already does things side-by-side cannot reasonably do — character-level highlights, `]c`/`[c` navigation inside the diff buffer, a custom statuscolumn showing old|new line numbers — and it occupies a single window, which fits better with the rest of a typical Neovim workflow (file tree, terminal, sidebar). Side-by-side relies on Neovim's built-in `:diffthis`, which is robust but offers no extension surface for plugin-specific UX.

## Considered Options

- **Keep both as first-class peers indefinitely.** Rejected: doubles the surface that any future rendering work has to cover (extmark highlights, statuscolumn integration, keymaps). Two equally-maintained renderers is two backlogs.
- **Delete the side-by-side renderer.** Rejected for now: users have `layout = "tab"` baked into their configs, and `:diffthis` is genuinely battle-tested. A hard removal would force a migration without a clean upgrade signal.
- **Inline is the future, side-by-side is legacy** *(chosen)*. Side-by-side first reaches a *feature floor* with inline — anything inline can do that also makes sense for two side-by-side windows (e.g. character-level highlights) gets back-ported. After that point, no new direction-setting features land in side-by-side; it stays available, stable, and ungrowing.

## Consequences

- New direction-setting work (e.g. word-level highlight tuning, fold support, conflict-marker visualisation) lands in the inline path and is not back-ported to side-by-side. Feature-floor parity (the inline features that translate naturally to a two-window view, e.g. character-level highlights) *does* get back-ported once, then side-by-side is closed for further additions.
- Bug fixes against the side-by-side renderer are still in scope — "legacy" means "no new direction," not "abandoned."
- Items that don't translate to side-by-side at all (the custom statuscolumn with old|new line numbers, the inline-specific `]c`/`[c` keymaps that duplicate `:diffthis`'s built-ins) are *not* part of the feature floor and don't need ports.
- A future ADR (or this one, superseded) will record the default flip. That decision needs at least: the inline renderer reaching feature parity for everything users actually rely on, and a deprecation window communicated in release notes.
- The `lua/code-preview/diff.lua` split (referenced in the user's memory note) should respect this asymmetry: extract the inline renderer cleanly so it can grow, and let the side-by-side path stay as a thin wrapper around `:diffthis`.
18 changes: 18 additions & 0 deletions docs/adr/0004-config-lives-only-in-neovim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Config lives only in the running Neovim; the bash layer carries none of its own

The plugin's config (`M.config` in `lua/code-preview/init.lua`) is the single source of truth. The bash [core handler](../../CONTEXT.md#core-handler) does not read user config files, environment variables, or a cached copy on disk — every config value it needs is fetched at hook time via a [hook context query](../../CONTEXT.md#hook-context-query) RPC into the running Neovim. If Neovim is unreachable, the bash handler degrades to a safe minimum (no logging, no [review gate](../../CONTEXT.md#review-gate), no visibility filtering) and proceeds.

The reason: keeping a second copy of config on the bash side creates two ways to be wrong. The user changes `diff.visible_only` at runtime via `:CodePreviewToggleVisibleOnly`; a file-cached copy would have to be invalidated. The user reloads their `init.lua` and changes `defer_claude_permissions`; the cached file is now stale. RPC-on-demand sidesteps the entire cache-coherency problem at the cost of one round-trip per hook.

## Considered Options

- **Cache config in a JSON file under `stdpath('cache')`.** Rejected: requires invalidation on every config change (runtime toggle, `setup()` re-call, `:source` of init.lua), and the staleness window is silent — the bash side would happily make decisions on yesterday's config.
- **Read config from environment variables exported at `setup()`.** Rejected: env vars only flow forward to processes spawned *after* the export. The bash handler is spawned by the agent, not by Neovim, so it inherits the agent's environment, not Neovim's.
- **Query Neovim at hook time** *(chosen)*. Always reflects current config; trades one RPC round-trip for cache-coherency freedom.

## Consequences

- Per-hook latency includes at least one RPC round-trip (often two: `log.state` early + `hook_context` later). Acceptable today; if the cost ever bites, the two queries can be merged into one.
- The bash handler's behaviour without Neovim is a real fallback path, not a bug — it must remain safe and silent. Tests should exercise the "no nvim" branch.
- After issue #47 phases 3 and 4, the [core handler](../../CONTEXT.md#core-handler) runs as Lua via `nvim --headless -l`. The hook-context-query pattern still applies, but the call becomes an in-process function call rather than an RPC. The principle (config in Neovim, fetched on demand) does not change.
- New config keys that the bash side might need must be added to `hook_context()`'s return shape — not silently exported to the environment or written to a side file.
35 changes: 35 additions & 0 deletions docs/agents/domain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Domain Docs

How the engineering skills should consume this repo's domain documentation when exploring the codebase.

## Before exploring, read these

- **`CONTEXT.md`** at the repo root.
- **`docs/adr/`** — read ADRs that touch the area you're about to work in.

If any of these files don't exist, **proceed silently**. Don't flag their absence; don't suggest creating them upfront. The producer skill (`/grill-with-docs`) creates them lazily when terms or decisions actually get resolved.

## File structure

This is a single-context repo:

```
/
├── CONTEXT.md
├── docs/adr/
│ ├── 0001-some-decision.md
│ └── 0002-another-decision.md
└── lua/
```

## Use the glossary's vocabulary

When your output names a domain concept (in an issue title, a refactor proposal, a hypothesis, a test name), use the term as defined in `CONTEXT.md`. Don't drift to synonyms the glossary explicitly avoids.

If the concept you need isn't in the glossary yet, that's a signal — either you're inventing language the project doesn't use (reconsider) or there's a real gap (note it for `/grill-with-docs`).

## Flag ADR conflicts

If your output contradicts an existing ADR, surface it explicitly rather than silently overriding:

> _Contradicts ADR-0007 (some decision) — but worth reopening because…_
22 changes: 22 additions & 0 deletions docs/agents/issue-tracker.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Issue tracker: GitHub

Issues and PRDs for this repo live as GitHub issues in `Cannon07/code-preview`. Use the `gh` CLI for all operations.

## Conventions

- **Create an issue**: `gh issue create --title "..." --body "..."`. Use a heredoc for multi-line bodies.
- **Read an issue**: `gh issue view <number> --comments`, filtering comments by `jq` and also fetching labels.
- **List issues**: `gh issue list --state open --json number,title,body,labels,comments --jq '[.[] | {number, title, body, labels: [.labels[].name], comments: [.comments[].body]}]'` with appropriate `--label` and `--state` filters.
- **Comment on an issue**: `gh issue comment <number> --body "..."`
- **Apply / remove labels**: `gh issue edit <number> --add-label "..."` / `--remove-label "..."`
- **Close**: `gh issue close <number> --comment "..."`

Infer the repo from `git remote -v` — `gh` does this automatically when run inside a clone.

## When a skill says "publish to the issue tracker"

Create a GitHub issue.

## When a skill says "fetch the relevant ticket"

Run `gh issue view <number> --comments`.
15 changes: 15 additions & 0 deletions docs/agents/triage-labels.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Triage Labels

The skills speak in terms of five canonical triage roles. This file maps those roles to the actual label strings used in this repo's issue tracker.

| Label in mattpocock/skills | Label in our tracker | Meaning |
| -------------------------- | -------------------- | ---------------------------------------- |
| `needs-triage` | `needs-triage` | Maintainer needs to evaluate this issue |
| `needs-info` | `needs-info` | Waiting on reporter for more information |
| `ready-for-agent` | `ready-for-agent` | Fully specified, ready for an AFK agent |
| `ready-for-human` | `ready-for-human` | Requires human implementation |
| `wontfix` | `wontfix` | Will not be actioned |

When a skill mentions a role (e.g. "apply the AFK-ready triage label"), use the corresponding label string from this table.

Edit the right-hand column to match whatever vocabulary you actually use.
Loading