Skip to content

fix(protocol): discard also unstages the discarded blocks - #689

Merged
tlgimenes merged 1 commit into
mainfrom
fix/discard-unstages
Oct 14, 2026
Merged

tlgimenes merged 1 commit into
mainfrom
fix/discard-unstages

Conversation

@tlgimenes

@tlgimenes tlgimenes commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

changes.discard on the filesystem/git storage (deco serve) restored the working tree to HEAD but left the git index untouched. A block staged with git add (or a git mv rename, a git rm deletion, a staged new file) stayed staged after the discard, so a later git commit would record the change that was just discarded.

PO: "deco serve should unstage git added blocks on discard".

How

Inside the existing single commit (same lock, after the expectedBase/version checks), a reason: "discard" commit of the fs storage now brings the index entries of exactly the files it restores or deletes back to HEAD, plus both sides of any staged rename that involves one of them (git diff-index --cached -M -z --name-status HEAD):

  • paths HEAD has: git reset -q HEAD -- <paths>
  • paths HEAD doesn't have (staged new files; everything on an unborn HEAD): git rm --cached -f --quiet --ignore-unmatch -- <paths> (-f because a staged new file that was edited again matches neither HEAD nor the disk)

It always uses execFile with argv arrays (no shell), literal pathspecs, chunked argv, and runs from the work tree's top level with repo-relative paths. The unstage runs before the file write. The previous entries (git ls-files -s -z) are kept, and if the write then fails they're put back (update-index --force-remove + update-index -z --index-info), so the commit stays all or nothing. No other path's staging is touched. After a discard, git status --porcelain -- .deco/blocks/<name>.json is empty for each discarded name.

Docs updated: storage/fs/git.ts and storage/fs/index.ts headers, CommitAttempt.reason in storage.ts, and the changes.discard notes in server/methods/changes.ts.

Known limit (unchanged): changes.list never consults the index. A change that is only staged, with a working file equal to HEAD, isn't listed, so nothing offers to discard it.

Tests

New describe("a discard unstages what it restores") in storage/fs/git.test.ts: staged modification, partially staged (staged + further edit), staged new file, staged new file edited again, staged deletion (git rm), git mv rename discarded by its new side / old side / both at once (both sides unstaged), unrelated staged files stay staged, a repo with no commits yet, and the index put back when the file write fails. The monorepo-folder test now stages its changes too.

Gates: bun run typecheck, biome check, knip, and vitest run packages/blocks/src/protocol (18 files, 557 passed / 85 skipped) all pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WNwbSEePYNcY5YCgqZURig


Summary by cubic

Fixes changes.discard so discarding a block also unstages it from the git index, not just restoring the working tree to HEAD.

  • A block staged with git add, git mv, git rm, or as a new file stayed staged after a discard and a later git commit would record the discarded change.
  • The unstage happens inside the same commit and covers both sides of any staged rename; the previous index entries are restored if the file write fails.
  • No other path's staging is touched, and the behavior works in repositories with no commits yet.
  • Known limit: changes.list doesn't consult the index, so a solely staged change isn't listed and nothing offers to discard it.

Written for commit 392e3e6. Summary will update on new commits.

View guided diff Turn on auto-fix

changes.discard restored the working tree to HEAD but left the git index
alone, so a block staged with `git add` (or `git mv`, `git rm`, a staged new
file) stayed staged after a discard and a later commit recorded it.

Inside the same commit (lock, expectedBase and version checks first), a
`reason: "discard"` commit of the fs storage now brings the index entries of
exactly the files it restores or deletes, plus both sides of any staged
rename involving one of them, back to HEAD: `git reset -q HEAD --` for paths
HEAD has, `git rm --cached -f --quiet --ignore-unmatch --` for the rest (all
of them on an unborn HEAD). The previous entries are kept and put back if the
file write then fails. No other path's staging is touched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WNwbSEePYNcY5YCgqZURig
@tlgimenes
tlgimenes requested a review from a team October 14, 2026 03:46
@tlgimenes
tlgimenes merged commit c90fbe9 into main Oct 14, 2026
2 checks passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 8.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

tlgimenes added a commit that referenced this pull request Oct 14, 2026
…he saved content (#690)

Vite sites no longer need template code to show a block saved with `deco
serve`. Today every v8 Vite site carries two pieces: an
`import.meta.hot.accept("../.deco/blocks.gen", ...)` block in
`src/cms.ts` that calls `createCMS` again and sends
`site:content-updated`, and a `site-content-reload` plugin in
`vite.config.ts` that turns that event into `full-reload`. This PR ships
that as one opt-in Vite plugin:

```ts
// vite.config.ts
import { decoContent } from "@decocms/blocks/vite";

export default defineConfig({ plugins: [/* cloudflare(), tanstackStart(), react(), */ decoContent()] });
```

Only Vite sites add it. Next.js sites never import it, so nothing
changes for them. The generated `.deco/blocks.gen.ts` on disk is
unchanged (`deco content` is untouched).

**Supersedes #676.** That PR fixed only the timing, in the docs (reload
after `createCMS` takes the new module, not on the file change). This PR
makes the plugin do that and rewrites the same "Editing locally"
section, so #676 can be closed.

## How it works

- **`@decocms/blocks/vite`, `decoContent()`**: `apply: "serve"`. In a
server environment only (`this.environment.config.consumer ===
"server"`, or `ssr` before the Environment API), its `transform` hook
finds each module that statically imports the site's
`.deco/blocks.gen.ts`. It reads the module's static imports and
re-exports and resolves each one with `this.resolve`, so any specifier
form counts: `"../.deco/blocks.gen"`, an alias like
`"~/.deco/blocks.gen"`, with or without `.ts`. Modules under
`node_modules`, non-JS files and the content module itself are skipped.
To each importer (usually `src/cms.ts`) it appends the block sites used
to write by hand:
  ```js
import { adoptContent as __decoAdoptContent } from
"@decocms/blocks/vite/hot";
  if (import.meta.hot) {
    import.meta.hot.accept("../.deco/blocks.gen", (next) => {
      if (next) __decoAdoptContent(next.default);
      import.meta.hot?.send("decocms:content-updated");
    });
  }
  ```
The block is appended, so no line moves and the source map holds (`map:
null`). The importer is the update's HMR boundary, so neither it nor
anything above it (the router entry, `routeTree.gen`) is invalidated or
runs again. JSON block edits bubble up through `blocks.gen.ts` to the
same boundary.
- **Already accepted by hand:** if the importer already has an
`….accept(<specifier>)` (string or array) that resolves to the content
module, the plugin adds nothing to it.
- **Pre-bundled hot module:** `configEnvironment` adds
`@decocms/blocks/vite/hot` to `optimizeDeps.include` in the server
environments. The appended import appears only after the transform, so
the dependency scan can't see it. Discovered on the first request
instead, the worker re-optimizes its dependencies and reloads the whole
program once.
- **One reload per save:** `configureServer` listens for the event on
every server environment (the `@cloudflare/vite-plugin` worker whatever
it's named, `rsc`, plain `ssr`) and sends `{ type: "full-reload" }` to
the client environment. It waits 100 ms because a save swaps the module
twice (the block's JSON, then `blocks.gen.ts`), so open pages reload
once.
- **`adoptContent(content)`** (`src/v8/cms.ts`, exposed only on
`@decocms/blocks/vite/hot`): hands the module to every CMS instance
keyed for that content's `.deco` root (`module:<root>`, with or without
`|site:…`), the same adopt path a second `createCMS` call takes. It
doesn't need the site's config: each instance re-adopts with its own
stored config, so a hosted CMS keeps its `site`/`dev`. It returns how
many instances took it. With none (no CMS yet, or a module without
`root`) it does nothing.
- `vite` is an optional peer (`>=6.0.0`, for the Environment API) and a
devDependency. The plugin uses `import type` only, so nothing is
imported from vite at runtime. New `exports` entries are `./vite` and
`./vite/hot` (types/source/default, like the others), and `bun run
build` emits `dist/v8/vite/*`.

**Sites that still have the old code:** they keep working.
- Without the plugin, nothing changes.
- With the plugin and the old `cms.ts` accept block both in place, the
plugin leaves that module alone. The old callback re-creates the CMS and
the old `site-content-reload` plugin reloads the page, as before. If you
delete only the old Vite plugin, pages don't reload, so the migration
reference says to delete both together.
- The older docs recipe's reload plugin is a `hotUpdate` hook on the
file change. That hook fires early, so the reference says to delete it.

## Review fixes

- **`optimizeDeps` (041a9b4):** the server environments pre-bundle
`@decocms/blocks/vite/hot`. Without that, the first request found the
new import and re-optimized the worker, which reloaded the whole program
once.
- **Importer boundary instead of a self-accepting content module
(ea8618c):** the first version made `blocks.gen.ts` accept its own
updates. Vite still invalidates every importer of an updated module
unless the importer accepts it. TanStack Start imports its server entry
again on each request, so with the cloudflare plugin `src/cms.ts` and
the route tree re-ran on every save. On the demo store, bursts of saves
gave 500s (`Cannot read properties of undefined (reading 'method')` in
`handleServerAction`, `Route` undefined in `routeTree.gen`), and the
client then stopped reloading. The plugin now writes the accept that
worked before (the hand-written `cms.ts` block had 14/14 clean bursts),
on the importer.

## Tests

- `src/v8/vite/vite.test.ts` (unit tests):
- `adoptContent`: the same instance, another `.deco` root left alone, a
hosted instance keeps its options, nothing before a CMS exists or for a
root-less module.
- The import reader: static imports and re-exports, not dynamic
`import()` or `import.meta`. It also finds accepted dependencies in
string and array forms.
- The transform: relative, alias, `.ts` and bare-alias specifiers
resolving to the content module get the block with their own specifier.
Nothing is added in the client, to the content module itself, under
`node_modules`, to non-JS files, to modules that don't import it, or to
a module that accepts it by hand. An unrelated accept doesn't count. The
`ssr` flag works without the Environment API.
- `optimizeDeps.include` only in server environments; `configureServer`
listens on every server environment, and two events make one
`full-reload`.
- `src/v8/vite/devServer.test.ts` (integration): a real Vite dev server
(`createServer`, SSR `RunnableDevEnvironment`). Like TanStack Start,
every "request" imports the site entry again (`entry.ts → cms.ts`, which
imports `~/.deco/blocks.gen` through an alias).
- Three saves: each gives a `full-reload`, and a request at that moment
already resolves the new content.
- **Regression:** the test counts module runs. `cms.ts` and the entry
each ran once, and the transformed `cms.ts` has exactly one accept while
`blocks.gen.ts` has none. With the previous self-accepting design the
same test fails with `{ cms: 4, entry: 4 }`.
- A `cms.ts` with its own accept block keeps exactly one accept, its
callback runs, and the plugin sends no reload.
- End to end with `@cloudflare/vite-plugin` (Node `vite dev`; 10 bursts
of 3 saves 120 ms apart, each save writing the block JSON and then
`blocks.gen.ts`; two request loops running throughout; a `vite-hmr`
WebSocket counting reloads; `cms.ts` logging each run):
- **Demo store** (deco-sites/demo-storefront#133 with this build, Vite
7, TanStack Start):
- This PR: 52/52 responses 200, `cms.ts` ran once in all, `hot updated:
/.deco/blocks.gen.ts via /src/cms.ts`, and the page had each burst's
last save 10/10.
- Previous self-accepting design: 12 of 54 responses were 500, `cms.ts`
ran 16 times, and 1 burst's page lacked the last save.
- **`examples/tanstack-smoke`:** 276/276 responses 200, `cms.ts` ran
once, and 10/10 bursts showed the last save. Saves more than 100 ms
apart each get their own reload, so a burst gave 1 to 3 reloads, never
more than its saves.
- `bun run check` (typecheck, biome, knip), `bun run build` and
`skills:check` pass. In `bun run test`, 1926 tests pass. The failures
are not from this PR:
- `delivery.cli.test.ts` DS-2: it greps `protocol/storage/fs/git.ts` for
`"reset"`, which #689 added. It fails on the base too.
- `schema.test.ts` (`is deterministic`, and one more): they hit the 5 s
timeout under full-suite load and pass when run alone (58/58).

## Also in this PR

- `examples/tanstack-smoke` and `examples/tanstack-rsc-smoke` add
`decoContent()`. The `ts-09` guide conformance test now expects that one
`@decocms/blocks/vite` import.
- `.agents/skills/deco-v7-to-v8-migration/reference/content.md`,
"Editing locally": add the plugin; drop the old snippet and plugin
together when you do; and why: the "client is not a function" and
one-edit-behind history from #676, and why the boundary is the importer.
- `README.md` mentions the plugin and the `/vite` subpath.

Follow-ups opened alongside:
- The TanStack guide snippet becomes the plugin line:
deco-sites/docs-tanstack#25. Merge it after the release.
- The demo store drops the template code: deco-sites/demo-storefront#133
(draft; it needs the release containing this fix).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01WNwbSEePYNcY5YCgqZURig

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
tlgimenes added a commit that referenced this pull request Oct 14, 2026
…he release) (#692)

## What

`cms.draftPointer(request)` now returns `null` for a `?__draft=` value
or `__deco_draft` cookie that `parseDraftPointer` rejects, the same as
when neither is present. The documented wiring (`pointer ?
cms.forDraft(pointer) : cms.forRelease()`) then serves the release for a
stray `?__draft=junk` link instead of answering 500.

- A malformed `?__draft=` is ignored the way an empty one is: a valid
draft cookie still applies.
- A malformed cookie is `null`. Before this change only `?__draft=off`
was filtered out.
- `cms.forDraft("garbage")` is unchanged and still returns
`LOADER_FAILED`, as the API reference and the conformance suite
document. Only the request-reading path changes.
- The migration skill's Next.js note now describes the new behavior and
keeps the site-side guard only for releases without this fix.

Sites that carry their own `parseDraftPointer` guard can drop it once
they're on a release with this fix. That's a follow-up, and no site is
touched here.

## Rendered output

A request with a malformed draft value used to get an error. It now
renders the release. A valid pointer renders exactly as before.

## Tests

- `packages/blocks/src/v8/draft.test.ts`: malformed params and cookies
give `null`, a malformed param falls back to a valid cookie, and the
end-to-end wiring serves the release revision.
- `bun run test` (packages/blocks): all draft, settings, cms and
sdk-conformance tests pass. The only failures are unrelated to this
change: `delivery.cli.test.ts` DS-2 also fails on main after #689
(`git.ts` now runs `update-index`), and `schema.test.ts` hit 5s timeouts
on a loaded machine. Different files time out on different runs.
- `bun run typecheck` (packages/blocks), `bun run lint`, `bun run
skills:check`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01WNwbSEePYNcY5YCgqZURig

<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
`cms.draftPointer` now treats a `?__draft=` value or `__deco_draft`
cookie that doesn't parse as absent, so a malformed pointer renders the
release instead of a `LOADER_FAILED` error. Previously only
`?__draft=off` was filtered out.

- A malformed parameter falls back to a valid cookie, like an empty one;
a malformed cookie returns `null`.
- `cms.forDraft("garbage")` is unchanged and still fails; only the
request-reading path changes.
- The migration skill's Next.js note now describes the new behavior.

<sup>Written for commit 6b78b1e.
Summary will update on new commits.</sup>

<a href="https://cubic.dev/pr/decocms/blocks/pull/692?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="View guided diff"
src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a>
<a
href="https://www.cubic.dev/action/auto-fix/pr/decocms/blocks/692?returnTo=https%3A%2F%2Fgithub.com%2Fdecocms%2Fblocks%2Fpull%2F692&source=description"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img
alt="Turn on auto-fix"
src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a>

<!-- End of auto-generated description by cubic. -->

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant