git sync's 'deckle: save before sync' commit shows as an undo of page 'before sync' #270

New issue

Closed

opened 2026-10-13 20:32:14 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-10-13 20:32:14 +02:00

Owner

Found while verifying docs/api.md for #264.

gitrepo sync (internal/gitrepo/sync.go:55) commits pending work with the subject deckle: save before sync. parseSnapshotSubject (internal/server/git.go) reads deckle subjects as <op> [lang] <pages>, so after a sync that committed pending work, GET /api/git/state reports undoOp: "save", undoPage: "before sync", undoPages: ["before sync"] — a page that does not exist.

Expected: that commit is either not undoable as a page step, or its subject names the pages it carries (or is excluded by parseSnapshotSubject), so undoPage never names a non-page.

Found while verifying docs/api.md for #264. `gitrepo` sync (internal/gitrepo/sync.go:55) commits pending work with the subject `deckle: save before sync`. `parseSnapshotSubject` (internal/server/git.go) reads deckle subjects as `<op> [lang] <pages>`, so after a sync that committed pending work, `GET /api/git/state` reports `undoOp: "save"`, `undoPage: "before sync"`, `undoPages: ["before sync"]` — a page that does not exist. Expected: that commit is either not undoable as a page step, or its subject names the pages it carries (or is excluded by `parseSnapshotSubject`), so `undoPage` never names a non-page.

dominik.polakovics added the

needs-triage

bug

labels

2026-10-13 20:32:14 +02:00

dominik.polakovics referenced this issue

2026-10-13 20:43:26 +02:00

docs: complete and verify the split documents — every route, CLI, features, architecture and 22 ADRs #274

dominik.polakovics commented

2026-10-13 22:27:25 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Summary:
The catch-up commit that sync and a branch-mode publish make of uncommitted working-tree changes (subjects deckle: save before sync and deckle: save before publish) is misread by the undo state as a page step on a page called "before sync". It must read as a commit that is not an editor step, so undo on any page stays offered and the Undo tooltip never names a non-page.

Current behavior:
In branch mode, Repo.Sync (internal/gitrepo/sync.go) and Repo.publishBranch (internal/gitrepo/publish.go) commit pending working-tree changes through commitWorktree with the subjects above before doing their work. When that commit ends up as the draft tip, GET /api/git/state reports undoOp: "save", undoPage: "before sync", undoPages: ["before sync"] and undoLang: <default>. The editor's reach check (undoReach in web/src/undoinfo.ts) finds no open page matching that path, so the Undo button is disabled on every page with a tooltip naming the non-page ("Undo: save on before sync, just now").

Reproduced at the parser: parseSnapshotSubject("deckle: save before sync") returns op save, pages ["before sync"], ok true; deckle: save before publish returns op save, pages ["before publish"]. The deckle: save and deckle: wip snapshot defaults are never reached because the server always passes a message; deckle: baseline and deckle: publish parse with no pages and never sit at an undoable tip.

Desired behavior:
For a draft tip whose subject is one of the git layer's catch-up subjects, GET /api/git/state reports undoAvailable: true and undoTime and omits undoOp, undoPage, undoPages and undoLang — exactly as it does today for a commit deckle did not write. POST /api/page/undo on a page that commit changed restores that page from the parent, as it does for any other step; on a page the commit did not touch the restore is a no-op. Every other subject keeps its current reading.

Decisions:

The catch-up commit is "not an editor step", not a new vocabulary op. ADR 0003 says hand edits between snapshots become part of the next one and stay undoable per page; a no-page op would disable undo on every page after a sync. So no client, catalog, de.json or undo_ops.json change.
Undo never reaches past the draft tip. Undo of a catch-up commit reverts what that commit did to the open page; it does not skip to the page step before it (ADR 0011: the draft tip is the one step).
One owner for the rule (CLAUDE.md). The git layer owns the subjects it writes: export them as constants or an exported predicate, as WIPBaselineMessage already is, and make commitWorktree's callers use them. The server's subject parser calls that; the server must not string-match these subjects with its own literal, and the parser must not grow a second copy of the op vocabulary.

Key interfaces:

The subjects passed to commitWorktree in internal/gitrepo (sync.go, publish.go) — become exported constants or an exported predicate on the subject.
parseSnapshotSubject in internal/server/git.go — returns ok false for those subjects, so undoState, undoPageSet and stepTranslationFiles all treat the step as an undescribed commit without further changes.
docs/api.md, GET /api/git/state: the undo* bullet says undoOp is absent "for a commit deckle did not write"; it gains the catch-up commit. POST /api/git/sync already names the subject and may link to that bullet.
Starting hint: the snapshot vocabulary and cutSnapshotOp live in internal/server/snapshot_ops.go; the client's absent-op handling is undoReach / undoStepText in web/src/undoinfo.ts and needs no change.

Acceptance criteria:

parseSnapshotSubject returns ok false for both catch-up subjects and is unchanged for every subject built from snapshotOps (existing parser tests keep passing).
Branch-mode server test: hand-edit a page's file, POST /api/git/sync; GET /api/git/state has undoAvailable: true and undoTime, and none of undoOp, undoPage, undoPages, undoLang.
The same test continues: POST /api/page/undo on that page restores the pre-edit content and answers with the page's state.
The same holds for a path-scoped branch-mode publish that leaves the save before publish commit as the draft tip (see assumptions).
The two subject strings exist once, in internal/gitrepo; internal/server holds no literal of either.
docs/api.md states the exclusion in the GET /api/git/state undo* bullet.

How to verify:

go test ./..., go vet ./...; the new branch-mode test fails before the fix (it sees undoPage: "before sync") and passes after.
cd web && npm test — the client is unchanged; its "unknown" branch already covers absent keys.
The docs drift tests (docs/docs_test.go, the route test in internal/server) pass.

Assumptions to check:

That a save before publish commit can remain the draft tip after a path-scoped branch-mode publish (the draft is rebased onto the new main and collapses only when its tree equals main's). If it always collapses, that criterion reduces to the parser unit test.
That no other gitrepo caller commits with a free-form deckle: subject that could collide with a vocabulary op — grep commitWorktree, SnapshotWIP, CommitDraft, moveDraft (its reflog message is not a commit subject).

Out of scope:

Making the catch-up commit name the pages it carries: the git layer does not know content, and the server would have to compute page sets before every sync.
A new undo op, catalog strings, or undo_ops.json changes.
The deckle: save and deckle: wip snapshot defaults, which the server never sends.
Multi-step undo or undo that reaches behind the draft tip.

> *This was generated by AI during triage.* ## Agent Brief **Summary:** The catch-up commit that sync and a branch-mode publish make of uncommitted working-tree changes (subjects `deckle: save before sync` and `deckle: save before publish`) is misread by the undo state as a page step on a page called "before sync". It must read as a commit that is not an editor step, so undo on any page stays offered and the Undo tooltip never names a non-page. **Current behavior:** In branch mode, `Repo.Sync` (`internal/gitrepo/sync.go`) and `Repo.publishBranch` (`internal/gitrepo/publish.go`) commit pending working-tree changes through `commitWorktree` with the subjects above before doing their work. When that commit ends up as the draft tip, `GET /api/git/state` reports `undoOp: "save"`, `undoPage: "before sync"`, `undoPages: ["before sync"]` and `undoLang: <default>`. The editor's reach check (`undoReach` in `web/src/undoinfo.ts`) finds no open page matching that path, so the Undo button is disabled on every page with a tooltip naming the non-page ("Undo: save on before sync, just now"). Reproduced at the parser: `parseSnapshotSubject("deckle: save before sync")` returns op `save`, pages `["before sync"]`, ok `true`; `deckle: save before publish` returns op `save`, pages `["before publish"]`. The `deckle: save` and `deckle: wip snapshot` defaults are never reached because the server always passes a message; `deckle: baseline` and `deckle: publish` parse with no pages and never sit at an undoable tip. **Desired behavior:** For a draft tip whose subject is one of the git layer's catch-up subjects, `GET /api/git/state` reports `undoAvailable: true` and `undoTime` and omits `undoOp`, `undoPage`, `undoPages` and `undoLang` — exactly as it does today for a commit deckle did not write. `POST /api/page/undo` on a page that commit changed restores that page from the parent, as it does for any other step; on a page the commit did not touch the restore is a no-op. Every other subject keeps its current reading. **Decisions:** - The catch-up commit is "not an editor step", not a new vocabulary op. ADR 0003 says hand edits between snapshots become part of the next one and stay undoable per page; a no-page op would disable undo on every page after a sync. So no client, catalog, `de.json` or `undo_ops.json` change. - Undo never reaches past the draft tip. Undo of a catch-up commit reverts what that commit did to the open page; it does not skip to the page step before it (ADR 0011: the draft tip is the one step). - One owner for the rule (CLAUDE.md). The git layer owns the subjects it writes: export them as constants or an exported predicate, as `WIPBaselineMessage` already is, and make `commitWorktree`'s callers use them. The server's subject parser calls that; the server must not string-match these subjects with its own literal, and the parser must not grow a second copy of the op vocabulary. **Key interfaces:** - The subjects passed to `commitWorktree` in `internal/gitrepo` (`sync.go`, `publish.go`) — become exported constants or an exported predicate on the subject. - `parseSnapshotSubject` in `internal/server/git.go` — returns ok `false` for those subjects, so `undoState`, `undoPageSet` and `stepTranslationFiles` all treat the step as an undescribed commit without further changes. - `docs/api.md`, `GET /api/git/state`: the `undo*` bullet says `undoOp` is absent "for a commit deckle did not write"; it gains the catch-up commit. `POST /api/git/sync` already names the subject and may link to that bullet. - Starting hint: the snapshot vocabulary and `cutSnapshotOp` live in `internal/server/snapshot_ops.go`; the client's absent-op handling is `undoReach` / `undoStepText` in `web/src/undoinfo.ts` and needs no change. **Acceptance criteria:** - [ ] `parseSnapshotSubject` returns ok `false` for both catch-up subjects and is unchanged for every subject built from `snapshotOps` (existing parser tests keep passing). - [ ] Branch-mode server test: hand-edit a page's file, `POST /api/git/sync`; `GET /api/git/state` has `undoAvailable: true` and `undoTime`, and none of `undoOp`, `undoPage`, `undoPages`, `undoLang`. - [ ] The same test continues: `POST /api/page/undo` on that page restores the pre-edit content and answers with the page's state. - [ ] The same holds for a path-scoped branch-mode publish that leaves the `save before publish` commit as the draft tip (see assumptions). - [ ] The two subject strings exist once, in `internal/gitrepo`; `internal/server` holds no literal of either. - [ ] `docs/api.md` states the exclusion in the `GET /api/git/state` `undo*` bullet. **How to verify:** - `go test ./...`, `go vet ./...`; the new branch-mode test fails before the fix (it sees `undoPage: "before sync"`) and passes after. - `cd web && npm test` — the client is unchanged; its "unknown" branch already covers absent keys. - The docs drift tests (`docs/docs_test.go`, the route test in `internal/server`) pass. **Assumptions to check:** - That a `save before publish` commit can remain the draft tip after a path-scoped branch-mode publish (the draft is rebased onto the new main and collapses only when its tree equals main's). If it always collapses, that criterion reduces to the parser unit test. - That no other `gitrepo` caller commits with a free-form `deckle: ` subject that could collide with a vocabulary op — grep `commitWorktree`, `SnapshotWIP`, `CommitDraft`, `moveDraft` (its reflog message is not a commit subject). **Out of scope:** - Making the catch-up commit name the pages it carries: the git layer does not know content, and the server would have to compute page sets before every sync. - A new undo op, catalog strings, or `undo_ops.json` changes. - The `deckle: save` and `deckle: wip snapshot` defaults, which the server never sends. - Multi-step undo or undo that reaches behind the draft tip.

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-10-13 22:27:25 +02:00

dominik.polakovics referenced this issue from a commit

2026-10-13 22:34:48 +02:00

fix(git): the catch-up commit of a sync or publish is not a page step

dominik.polakovics referenced this issue from a pull request that will close it,

2026-10-13 22:34:57 +02:00

fix(git): the catch-up commit of a sync or publish is not a page step #277