feat: attribute the element history to the editor who made each change #296

Merged
dominik.polakovics merged 5 commits from afk/295 into main 2026-10-14 23:48:30 +02:00

Closes #295

What

Each change is now committed in the name of the editor who made it, as a pseudonym the host supplies. A branch-mode publish keeps those commits in main's history. The History sheet shows, for each element, who published and who edited it, and who has unpublished edits.

  • Identity (internal/server/mutation.go requestEditor, the one owner of this rule):
    • harness serve --trust-editor-header (off by default) honours X-Deckle-Editor: Name <email>.
    • A malformed value with the flag on is a 400 request.badEditor.
    • Without the flag, or without the header, the identity is the repository default (gitrepo.DefaultIdentity: config, else deckle <deckle@localhost>).
    • The identity is resolved before the mutation lock, held on the Handler for snapshotWIP and sync, and captured into the publish job.
  • Author and committer (gitrepo):
    • CommitDraft, SnapshotWIP, PublishAll/PublishPaths and Sync take an author Identity.
    • The committer is always the repository default.
    • Policy rebases set only the committer, so replayed commits keep their authors.
  • Publish is a merge (branch mode):
    • The publish commit's parents are the fetched main, then the draft tip.
    • Afterwards the draft fast-forwards to the publish commit when the trees are equal. Otherwise it gets one carry-over commit (deckle: carry over unpublished draft changes).
    • There is no post-publish rebase, and the draft push is a lease push that fast-forwards.
    • A WIP-mode publish stays single-parent.
  • History:
    • %aN everywhere, so mailmap.file resolves display names.
    • HistoryVersion.Edits lists a merge's p1..p2 commits that touched the needle's files. DraftEdits does the same for main..draft.
    • The server decides "did the element change" with the existing locateElement + elemops.DiffElement, now behind one elementChange helper that entries, edits and the pending row all share.
    • Every entry carries authors ([author] for a single-parent commit), and so does pending.
  • Client: a publish row reads "Published by X · Edited by A, B and C" (formatList); the pending row reads "Edited by …". Strings are in en and de.
  • Docs:
    • New ADR 0024, with superseding notes on ADRs 0011 and 0023.
    • docs/architecture.md, docs/api.md (Writes convention, the history authors fields, the error), docs/harness.md (the flag) and docs/host-navigation.md ("The editor's identity": the header, stripping it, the mailmap, the flag).
    • scaffold/site/AGENTS.md gets one line saying the flag is host-only.
    • README unchanged. No skill changes (see the issue's decision).

Decisions made during implementation, beyond the issue

  • Carry-over when main moved under the draft: if a developer pushed to main and no reconcile ran first, the carry-over tree is the policy replay of the draft onto the publish commit, which is what the old rebase produced. Using the old tip's tree would silently revert the developer's change. If the replay fails, it falls back to the old tip's tree, so nothing is lost. Test: TestBranchPublishOntoMovedMainKeepsDeveloperChange.
  • Draft tip already in main: when the draft tip is already contained in main, the publish commit stays single-parent rather than a redundant merge.
  • No undo of a carry-over commit: UndoParent offers no undo when the tip is a carry-over commit. Its parent is the publish commit, so an undo would drop unpublished draft work. This matches "a collapse drops the undo of the last published step".
  • Pending authors go through the mailmap: the request's own name in pending.authors is resolved through the mailmap too (gitrepo.AuthorName, git check-mailmap --stdin), so a user shows the same name for their uncommitted work as for their commits.
  • Malformed header on the history route: on GET /api/element/history a trusted malformed header is refused for every element, not only when a pending row exists. Its uid-repairing GETs are in the mutation set and refuse it too.
  • Same publisher and editor: the client shows the name once when authors is exactly [author], so standalone deckle looks as it does today.

Verification

  • go vet ./..., go test ./... and gofmt are clean.
  • cd web && npm run build && npx tsc --noEmit && npm test: 126 files, 3275 tests passing.
  • New tests:
    • gitrepo: identity parsing and the default identity; author/committer split for saves, snapshots, sync and publish; merge parents, fast-forward and carry-over; reconcile keeps authors; edits and draft edits; mailmap on and off.
    • server, through ServeHTTP: header trusted, untrusted, malformed and absent on saves, publish and sync; per-element authors (two editors on two elements of one page, a third editor touching the file without changing either); newest-first order and de-duplication; pending authors in branch and WIP mode; mailmap.
    • harness: parseServeArgs.
    • Client: vitest for the History sheet.
  • The drift tests (usage ⇔ docs/harness.md, routes ⇔ docs/api.md, docs links/budget, skills, AGENTS.md) are green.
  • Assumptions checked with git 2.47.3:
    • commit-tree with two -p and split author/committer environment variables works.
    • Rebase keeps authors.
    • %aN and check-mailmap honour mailmap.file.
    • p1..p2 excludes shared commits.
    • The first-parent walk is unchanged on merges.
  • Host assumption, unverified here (noted in ADR 0024): deckle-hosted's push path accepts a merge commit on main.

🤖 Generated with Claude Code

Closes #295 ## What Each change is now committed in the name of the editor who made it, as a pseudonym the host supplies. A branch-mode publish keeps those commits in main's history. The History sheet shows, for each element, who published and who edited it, and who has unpublished edits. - **Identity** (`internal/server/mutation.go` `requestEditor`, the one owner of this rule): - `harness serve --trust-editor-header` (off by default) honours `X-Deckle-Editor: Name <email>`. - A malformed value with the flag on is a 400 `request.badEditor`. - Without the flag, or without the header, the identity is the repository default (`gitrepo.DefaultIdentity`: config, else `deckle <deckle@localhost>`). - The identity is resolved before the mutation lock, held on the Handler for snapshotWIP and sync, and captured into the publish job. - **Author and committer** (`gitrepo`): - `CommitDraft`, `SnapshotWIP`, `PublishAll`/`PublishPaths` and `Sync` take an author `Identity`. - The committer is always the repository default. - Policy rebases set only the committer, so replayed commits keep their authors. - **Publish is a merge** (branch mode): - The publish commit's parents are the fetched main, then the draft tip. - Afterwards the draft fast-forwards to the publish commit when the trees are equal. Otherwise it gets one carry-over commit (`deckle: carry over unpublished draft changes`). - There is no post-publish rebase, and the draft push is a lease push that fast-forwards. - A WIP-mode publish stays single-parent. - **History**: - `%aN` everywhere, so `mailmap.file` resolves display names. - `HistoryVersion.Edits` lists a merge's `p1..p2` commits that touched the needle's files. `DraftEdits` does the same for `main..draft`. - The server decides "did the element change" with the existing `locateElement` + `elemops.DiffElement`, now behind one `elementChange` helper that entries, edits and the pending row all share. - Every entry carries `authors` (`[author]` for a single-parent commit), and so does `pending`. - **Client**: a publish row reads "Published by X · Edited by A, B and C" (`formatList`); the pending row reads "Edited by …". Strings are in en and de. - **Docs**: - New ADR 0024, with superseding notes on ADRs 0011 and 0023. - `docs/architecture.md`, `docs/api.md` (Writes convention, the history `authors` fields, the error), `docs/harness.md` (the flag) and `docs/host-navigation.md` ("The editor's identity": the header, stripping it, the mailmap, the flag). - `scaffold/site/AGENTS.md` gets one line saying the flag is host-only. - README unchanged. No skill changes (see the issue's decision). ## Decisions made during implementation, beyond the issue - **Carry-over when main moved under the draft:** if a developer pushed to main and no reconcile ran first, the carry-over tree is the policy replay of the draft onto the publish commit, which is what the old rebase produced. Using the old tip's tree would silently revert the developer's change. If the replay fails, it falls back to the old tip's tree, so nothing is lost. Test: `TestBranchPublishOntoMovedMainKeepsDeveloperChange`. - **Draft tip already in main:** when the draft tip is already contained in main, the publish commit stays single-parent rather than a redundant merge. - **No undo of a carry-over commit:** `UndoParent` offers no undo when the tip is a carry-over commit. Its parent is the publish commit, so an undo would drop unpublished draft work. This matches "a collapse drops the undo of the last published step". - **Pending authors go through the mailmap:** the request's own name in `pending.authors` is resolved through the mailmap too (`gitrepo.AuthorName`, `git check-mailmap --stdin`), so a user shows the same name for their uncommitted work as for their commits. - **Malformed header on the history route:** on `GET /api/element/history` a trusted malformed header is refused for every element, not only when a pending row exists. Its uid-repairing GETs are in the mutation set and refuse it too. - **Same publisher and editor:** the client shows the name once when `authors` is exactly `[author]`, so standalone deckle looks as it does today. ## Verification - `go vet ./...`, `go test ./...` and `gofmt` are clean. - `cd web && npm run build && npx tsc --noEmit && npm test`: 126 files, 3275 tests passing. - New tests: - gitrepo: identity parsing and the default identity; author/committer split for saves, snapshots, sync and publish; merge parents, fast-forward and carry-over; reconcile keeps authors; edits and draft edits; mailmap on and off. - server, through `ServeHTTP`: header trusted, untrusted, malformed and absent on saves, publish and sync; per-element authors (two editors on two elements of one page, a third editor touching the file without changing either); newest-first order and de-duplication; pending authors in branch and WIP mode; mailmap. - harness: `parseServeArgs`. - Client: vitest for the History sheet. - The drift tests (usage ⇔ `docs/harness.md`, routes ⇔ `docs/api.md`, docs links/budget, skills, AGENTS.md) are green. - Assumptions checked with git 2.47.3: - `commit-tree` with two `-p` and split author/committer environment variables works. - Rebase keeps authors. - `%aN` and `check-mailmap` honour `mailmap.file`. - `p1..p2` excludes shared commits. - The first-parent walk is unchanged on merges. - Host assumption, unverified here (noted in ADR 0024): deckle-hosted's push path accepts a merge commit on main. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Commits take an author Identity (committer stays the repository default);
a branch-mode publish is a two-parent merge, after which the draft
fast-forwards or gets one carry-over commit instead of being rebased; the
history reads authors mailmap-aware and lists each merge's second-parent
edits and the unpublished draft edits.

Refs #295

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
harness serve --trust-editor-header makes the server honour the
X-Deckle-Editor header as the author of a request's commits (malformed:
400 request.badEditor); GET /api/element/history serves authors on every
entry and on pending.

Refs #295

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Refs #295

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
docs: ADR 0024 publish is a merge, authorship is pseudonymous
All checks were successful
ci / build-and-test (pull_request) Successful in 3m20s
ci / release (pull_request) Has been skipped
14952e3961
Closes #295

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Author
Owner

[autoland] verdict: pass

Lander verdict: CONCERNS (non-blocking), approved, not merged

Signal relied on: CI aggregate success (ci / build-and-test: npm ci && npm run build && npm test, go vet ./..., go test ./..., go build). That covers every drift test (cmd/harness usage, internal/server routes, docs/docs_test.go, scaffold/skills_test.go, internal/element/agentsdoc_test.go). I also ran two checks CI doesn't: gofmt -l on the changed Go files (clean) and npx tsc --noEmit in web/ (clean).

Conventions: the title is Conventional Commits, Closes #295 is present, and every commit is conventional. The diff matches the issue's scope (identity, merge publish, per-element authors, client, docs and ADR 0024). README and skills are untouched, as the issue decided. The branch is based on the current main tip (b0a7d28), so there is no conflict.

Concern (non-blocking): carryDraftOver silently falls back to the old draft tree

internal/gitrepo/publish.go:419-427 (carryDraftOver): if main moved under the draft and no reconcile has run, and then git merge-base or the policy replay (policyRebase(pub, mb, head)) fails, the error is dropped. keep stays head's tree, and the draft moves to a carry-over commit with that tree on top of the publish commit.

  • Main's change is lost from the draft. The draft is now a descendant of main but lacks the developer's change. A later reconcile sees nothing to replay.
  • The next publish can revert main. A developer change under content/ or assets/media/ that the draft never touched now differs between draft and main. The next PublishAll would publish it back as a revert.
  • Nothing is surfaced. The old code handled a failed post-publish rebase with setUnsynced("published, but draft branch … could not be rebased onto main") and left the draft alone. This path calls no setUnsynced and writes no log.
  • The PR body overstates it. It says the fallback means "nothing is lost". That holds for the draft's changes, not for main's, and the body itself says using the old tip's tree "would silently revert the developer's change".
  • No test covers the fallback. TestBranchPublishOntoMovedMainKeepsDeveloperChange only covers the replay succeeding.

Suggested fix: when the replay fails, keep the old behaviour. Set unsynced with a reason and leave the draft for reconcile, or at least log and set unsynced before falling back. Add a test that forces the replay to fail.

The path is rare: the policy auto-resolves conflicts, so a failure needs a git-level error. That is why this is a concern and not a FAIL. Everything else checked out: author/committer split, merge parents, fast-forward and carry-over, lease push, requestEditor as the single owner, elementChange shared by entries, edits and pending, mailmap via %aN / check-mailmap, client strings in en and de, and docs in their owning documents.

[autoland] verdict: pass ## Lander verdict: CONCERNS (non-blocking), approved, not merged **Signal relied on:** CI aggregate `success` (`ci / build-and-test`: `npm ci && npm run build && npm test`, `go vet ./...`, `go test ./...`, `go build`). That covers every drift test (`cmd/harness` usage, `internal/server` routes, `docs/docs_test.go`, `scaffold/skills_test.go`, `internal/element/agentsdoc_test.go`). I also ran two checks CI doesn't: `gofmt -l` on the changed Go files (clean) and `npx tsc --noEmit` in `web/` (clean). **Conventions:** the title is Conventional Commits, `Closes #295` is present, and every commit is conventional. The diff matches the issue's scope (identity, merge publish, per-element authors, client, docs and ADR 0024). README and skills are untouched, as the issue decided. The branch is based on the current `main` tip (`b0a7d28`), so there is no conflict. ### Concern (non-blocking): `carryDraftOver` silently falls back to the old draft tree `internal/gitrepo/publish.go:419-427` (`carryDraftOver`): if main moved under the draft and no reconcile has run, and then `git merge-base` or the policy replay (`policyRebase(pub, mb, head)`) fails, the error is dropped. `keep` stays `head`'s tree, and the draft moves to a carry-over commit with that tree on top of the publish commit. - **Main's change is lost from the draft.** The draft is now a descendant of main but lacks the developer's change. A later reconcile sees nothing to replay. - **The next publish can revert main.** A developer change under `content/` or `assets/media/` that the draft never touched now differs between draft and main. The next `PublishAll` would publish it back as a revert. - **Nothing is surfaced.** The old code handled a failed post-publish rebase with `setUnsynced("published, but draft branch … could not be rebased onto main")` and left the draft alone. This path calls no `setUnsynced` and writes no log. - **The PR body overstates it.** It says the fallback means "nothing is lost". That holds for the draft's changes, not for main's, and the body itself says using the old tip's tree "would silently revert the developer's change". - **No test covers the fallback.** `TestBranchPublishOntoMovedMainKeepsDeveloperChange` only covers the replay succeeding. Suggested fix: when the replay fails, keep the old behaviour. Set unsynced with a reason and leave the draft for reconcile, or at least log and set unsynced before falling back. Add a test that forces the replay to fail. The path is rare: the policy auto-resolves conflicts, so a failure needs a git-level error. That is why this is a concern and not a FAIL. Everything else checked out: author/committer split, merge parents, fast-forward and carry-over, lease push, `requestEditor` as the single owner, `elementChange` shared by entries, edits and pending, mailmap via `%aN` / `check-mailmap`, client strings in en and de, and docs in their owning documents.
fix(gitrepo): a carry-over replay that fails fails the publish, nothing moves
All checks were successful
ci / build-and-test (pull_request) Successful in 3m14s
ci / release (pull_request) Has been skipped
8839ad40b0
The branch-mode publish settled the draft's carry-over tree after the
push, and when main had moved under the draft and the policy replay
failed it silently kept the old draft tree. That left a draft descending
from main but lacking what main gained: no reconcile would replay it,
and the next publish would revert a developer's content change on main.

Leaving the draft where it was (the pre-merge fallback) is no answer
either: the draft tip is already the merge's second parent, so a later
reconcile collapses it to main's tree and drops the unpublished
remainder.

The carry-over tree is now computed before the push (carryOverTree), so
a replay that cannot complete fails the publish while no ref has moved,
exactly like a rejected push; the same request again retries. ADR 0024
and the architecture narrative say so.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Author
Owner

Addressed the lander's concern in 8839ad4.

Finding confirmed, suggested fix declined. The silent fallback in carryDraftOver was real: a failed replay kept the old draft tree, leaving a draft that descends from main but lacks what main gained, which no reconcile replays and the next publish reverts. But "keep the old behaviour" (leave the draft, set unsynced, let reconcile catch up) is unsafe under the merge publish: the draft tip is already the merge's second parent, so Reconcile finds main..tip empty, collapses the draft to main's tree and drops the unpublished remainder (a first version of the test showed exactly that: reconcile status collapsed).

What the fix does instead. The carry-over tree is settled before the push (carryOverTree). A replay that cannot complete fails the publish while no ref has moved, exactly like a rejected push; the same request again retries. carryDraftOver now only moves the draft onto a tree it was handed.

  • Test TestBranchPublishOntoMovedMainFailsWhenReplayFails forces the failure with a refusing pre-rebase hook and asserts: publish errors with "publish not pushed … could not be replayed", origin main untouched, draft tip untouched, clean checkout, sync status unchanged; then removes the hook and the retry publishes with the developer's change carried into the draft.
  • ADR 0024 and docs/architecture.md state the ordering and why the old-tree fallback is wrong. The PR body's "nothing is lost" claim about the fallback no longer applies: there is no fallback.
  • go vet ./..., go test ./... and gofmt are clean; the API needs no change (a failed publish is the existing publish.failed).
Addressed the lander's concern in 8839ad4. **Finding confirmed, suggested fix declined.** The silent fallback in `carryDraftOver` was real: a failed replay kept the old draft tree, leaving a draft that descends from `main` but lacks what `main` gained, which no reconcile replays and the next publish reverts. But "keep the old behaviour" (leave the draft, set unsynced, let reconcile catch up) is unsafe under the merge publish: the draft tip is already the merge's second parent, so `Reconcile` finds `main..tip` empty, collapses the draft to `main`'s tree and drops the unpublished remainder (a first version of the test showed exactly that: `reconcile status collapsed`). **What the fix does instead.** The carry-over tree is settled *before* the push (`carryOverTree`). A replay that cannot complete fails the publish while no ref has moved, exactly like a rejected push; the same request again retries. `carryDraftOver` now only moves the draft onto a tree it was handed. - Test `TestBranchPublishOntoMovedMainFailsWhenReplayFails` forces the failure with a refusing `pre-rebase` hook and asserts: publish errors with "publish not pushed … could not be replayed", origin `main` untouched, draft tip untouched, clean checkout, sync status unchanged; then removes the hook and the retry publishes with the developer's change carried into the draft. - ADR 0024 and `docs/architecture.md` state the ordering and why the old-tree fallback is wrong. The PR body's "nothing is lost" claim about the fallback no longer applies: there is no fallback. - `go vet ./...`, `go test ./...` and `gofmt` are clean; the API needs no change (a failed publish is the existing `publish.failed`).
Author
Owner

[autoland] verdict: pass

This was generated by AI while landing a PR.

Verdict: PASS.

Signal relied on: CI aggregate success on head 8839ad4 (ci / build-and-test: npm build/test, go vet, go test ./..., go build), including every drift test (usage ⇔ docs/harness.md, routes ⇔ docs/api.md, docs links and budget, skills, AGENTS.md). Not re-run.

Conventions: the title is Conventional Commits and Closes #295 is present. The head is based on the current main tip (b0a7d28), so there is no conflict. The diff matches the issue's scope.

Earlier autoland concern, resolved in 8839ad4: carryOverTree now settles the draft's carry-over tree before the push. A failed replay fails the publish with no ref moved, so the old-tree fallback is gone. TestBranchPublishOntoMovedMainFailsWhenReplayFails covers the failure and the retry. ADR 0024 and docs/architecture.md are updated to match.

Reviewed:

  • requestEditor is the single owner of the editor rule. It runs before the mutation lock and is cleared before the unlock. Publish captures it into the job.
  • Author and committer are split; policy rebases set the committer only.
  • The publish is a merge (base, then head; single-parent when head is already in base), followed by a fast-forward or a carry-over commit and a lease push.
  • elementChange is the one verdict for entries, edits and the pending row.
  • DraftEdits and historyEdits share one implementation.
  • Names are read with %aN and check-mailmap.
  • The client strings are in en and de, and each fact is in its owning document.

Host assumption (ADR 0024): deckle-hosted's only pre-receive hook refuses symlinks at the tip and does not inspect commit shape, so a merge on main passes it.

CONCERNS: none.

[autoland] verdict: pass > *This was generated by AI while landing a PR.* **Verdict: PASS.** **Signal relied on:** CI aggregate `success` on head `8839ad4` (`ci / build-and-test`: npm build/test, `go vet`, `go test ./...`, `go build`), including every drift test (usage ⇔ docs/harness.md, routes ⇔ docs/api.md, docs links and budget, skills, AGENTS.md). Not re-run. **Conventions:** the title is Conventional Commits and `Closes #295` is present. The head is based on the current `main` tip (`b0a7d28`), so there is no conflict. The diff matches the issue's scope. **Earlier autoland concern, resolved in 8839ad4:** `carryOverTree` now settles the draft's carry-over tree before the push. A failed replay fails the publish with no ref moved, so the old-tree fallback is gone. `TestBranchPublishOntoMovedMainFailsWhenReplayFails` covers the failure and the retry. ADR 0024 and `docs/architecture.md` are updated to match. **Reviewed:** - `requestEditor` is the single owner of the editor rule. It runs before the mutation lock and is cleared before the unlock. Publish captures it into the job. - Author and committer are split; policy rebases set the committer only. - The publish is a merge (`base`, then `head`; single-parent when `head` is already in `base`), followed by a fast-forward or a carry-over commit and a lease push. - `elementChange` is the one verdict for entries, edits and the pending row. - `DraftEdits` and `historyEdits` share one implementation. - Names are read with `%aN` and `check-mailmap`. - The client strings are in en and de, and each fact is in its owning document. **Host assumption (ADR 0024):** deckle-hosted's only `pre-receive` hook refuses symlinks at the tip and does not inspect commit shape, so a merge on `main` passes it. **CONCERNS:** none.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Cloonar/deckle!296
No description provided.