mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 06:19:49 +00:00
fix(collab): a schema-version rebuild sets unflushed edits aside instead of deleting them (BUG-3244 A) (#1598)
* wip(collab): the schema-version rebuild sets unflushed edits aside and the item stays marked (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * wip(server): set-aside rows refuse every content write and restore unless overwrite_pending_edits, which discards and reports them (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * wip: set-aside read/discard endpoints and pad item set-aside; census and migrated-table entries (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * wip(store): set-aside edits travel in the workspace bundle; the migrate gate asks the op-log only (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * wip: set-aside advice branches at every door that names a remedy; mark-only doors share one helper (BUG-3244 A, ruling 1) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * test(cli): set-aside notices name the right remedy (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * docs: the rebuild sets unflushed edits aside; freeze reason, API docs and op-log prose follow (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * test(web): identity-gate rows re-reviewed for the set-aside reason threading (BUG-3244 A) The edits in each covered unit are a pure pendingEditsReason(e) argument or string branch, or a reason passed to the dialog; no await, write or fence changed. Three anonymous-unit matchers follow the new call text, and the keepalive failure row lists the pure classifier in may. Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * test(server): a restore override counts the set-aside rows it discarded (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * fix(store): the rebuild sets aside the rows its DELETE returned, so an append between choosing and clearing is not lost (BUG-3244 A, codex r1) On Postgres each statement takes its own snapshot and the setup lock does not stop a connected peer appending, so a row committed between the old INSERT ... SELECT copy and the DELETE was deleted without being set aside. The kept rows are now chosen from DELETE ... RETURNING. A test hook in the gap drives a Postgres leg; a mutant that chooses before the gap loses the row. Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * test(web): playbook save row re-reviewed after the BUG-3230 U3 rebase (BUG-3244 A) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * fix(store): the set-aside move's first statement is the DELETE, so SQLite never upgrades a stale read snapshot (BUG-3244 A, codex r2) Reading the watermark first took a read snapshot on SQLite; a concurrent append would then make the DELETE fail with SQLITE_BUSY_SNAPSHOT instead of waiting. The watermark is now read after the DELETE, where a later value is only more accurate. Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * feat(mcp): ToolSurfaceVersion 0.56, content_state gains superseded_set_aside (BUG-3244 A) The value reaches pad_item get / full list, pad_playbook run and the item resource's line above the body, and its remedy differs from applied_pending_flush (opening the item will not restore the edits). An additive bump with a version.go entry; README, CLAUDE.md and the MCP instructions follow. Read/discard of set-aside rows stays off the catalog. Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * test(server): after a bumped rebuild the next open is granted the seed once and the item stays superseded (BUG-3244 A x BUG-3240) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9
This commit is contained in:
@@ -0,0 +1,52 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3244 ruling 1 at the CLI: every stale-body notice that names a remedy
|
||||
// branches on the value. For set-aside edits it names `pad item set-aside`
|
||||
// and never says a tab will catch the body up. Each case renders BOTH values.
|
||||
|
||||
func setAsideItem(state string) *models.Item {
|
||||
return &models.Item{Slug: "fix-oauth", Ref: "TASK-5", ContentState: state}
|
||||
}
|
||||
|
||||
func TestSetAsideStaleNoticesNameTheRightRemedy(t *testing.T) {
|
||||
type door struct {
|
||||
name string
|
||||
run func(*models.Item) string
|
||||
}
|
||||
doors := []door{
|
||||
{"item show table line", func(it *models.Item) string { return captureStdout(t, func() { printStaleBodyLine(it) }) }},
|
||||
{"item show markdown warning", func(it *models.Item) string { return captureStderr(t, func() { warnContentStale(it) }) }},
|
||||
{"item edit refusal", func(it *models.Item) string { return staleEditRefusal(it).Error() }},
|
||||
{"item edit --force warning", func(it *models.Item) string { return captureStderr(t, func() { warnStaleEditSeed(it) }) }},
|
||||
{"playbook body warning", func(it *models.Item) string {
|
||||
return captureStderr(t, func() { warnPlaybookBodyStale(it.ContentState) })
|
||||
}},
|
||||
}
|
||||
for _, d := range doors {
|
||||
t.Run(d.name, func(t *testing.T) {
|
||||
setAside := d.run(setAsideItem(models.ContentStateSetAside))
|
||||
pending := d.run(setAsideItem(models.ContentStatePendingFlush))
|
||||
if setAside == "" {
|
||||
t.Fatal("set-aside item printed nothing")
|
||||
}
|
||||
if !strings.Contains(setAside, "earlier editor version") && !strings.Contains(setAside, "set aside") {
|
||||
t.Errorf("set-aside notice does not name the upgrade: %q", setAside)
|
||||
}
|
||||
for _, promise := range []string{"flushes", "catches up", "browser tab so", "Open the item in a browser"} {
|
||||
if strings.Contains(setAside, promise) {
|
||||
t.Errorf("set-aside notice promises a tab remedy (%q): %q", promise, setAside)
|
||||
}
|
||||
}
|
||||
if pending == "" || pending == setAside {
|
||||
t.Errorf("pending notice missing or identical to the set-aside one: %q", pending)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -162,7 +162,7 @@ func printBootstrapMarkdown(raw []byte) error {
|
||||
fmt.Printf("## Playbooks (%d)\n", len(b.Playbooks))
|
||||
var stalePlaybookSummaries []string
|
||||
for _, p := range b.Playbooks {
|
||||
if p.ContentState == models.ContentOutcomeAppliedPendingFlush {
|
||||
if models.IsContentStateStale(p.ContentState) {
|
||||
stalePlaybookSummaries = append(stalePlaybookSummaries, p.Ref)
|
||||
}
|
||||
invocation := "—"
|
||||
|
||||
+7
-1
@@ -869,6 +869,12 @@ func gateLateStale(wsSlug string, stale []store.PendingFlushItem, migratedSoFar
|
||||
// the item, on the same reasoning as ListItemsPendingContentFlush: a refusal
|
||||
// exists to tell an operator what to open, and a fabricated "PREFIX-0" sends
|
||||
// them looking for something that does not exist.
|
||||
//
|
||||
// Only applied_pending_flush counts. superseded_set_aside (BUG-3244) marks
|
||||
// edits the bundle CARRIES (ItemExport.CollabSetAside), so migrating them loses
|
||||
// nothing and opening a tab would not recover them anyway. An item holding
|
||||
// both reads superseded_set_aside here, which is why the pre-export gate asks
|
||||
// the op-log on its own and runs first.
|
||||
func staleBundleItems(data *models.WorkspaceExport) []store.PendingFlushItem {
|
||||
if data == nil {
|
||||
return nil
|
||||
@@ -879,7 +885,7 @@ func staleBundleItems(data *models.WorkspaceExport) []store.PendingFlushItem {
|
||||
}
|
||||
var out []store.PendingFlushItem
|
||||
for _, it := range data.Items {
|
||||
if it.ContentState == "" {
|
||||
if it.ContentState != models.ContentStatePendingFlush {
|
||||
continue
|
||||
}
|
||||
ref := it.Slug
|
||||
|
||||
+57
-12
@@ -1444,6 +1444,9 @@ Examples:
|
||||
// reading this as a server_error.
|
||||
if apiErr.AsContentPendingFlush() {
|
||||
cli.WriteContentPendingFlushError(os.Stderr, apiErr)
|
||||
if apiErr.ContentSetAsideRows() > 0 {
|
||||
return fmt.Errorf("update rejected: the item holds edits an editor upgrade set aside")
|
||||
}
|
||||
return fmt.Errorf("update rejected: the item has unflushed collaborative edits")
|
||||
}
|
||||
}
|
||||
@@ -3289,7 +3292,7 @@ Examples:
|
||||
// BUG-3033 — the marker describes the SOURCE item of each
|
||||
// link, so the refs named are source refs, not the target
|
||||
// the caller asked about.
|
||||
if bl.ContentState == models.ContentOutcomeAppliedPendingFlush {
|
||||
if models.IsContentStateStale(bl.ContentState) {
|
||||
staleBacklinks = append(staleBacklinks, bl.SourceRef)
|
||||
}
|
||||
}
|
||||
@@ -3604,7 +3607,7 @@ func searchCmd() *cobra.Command {
|
||||
// BUG-3033. The JSON form carries the marker on the embedded
|
||||
// item; this renderer decodes that item and prints only the
|
||||
// snippet, so without this the signal stops here.
|
||||
if r.Item.ContentState == models.ContentOutcomeAppliedPendingFlush {
|
||||
if models.IsContentStateStale(r.Item.ContentState) {
|
||||
// Ref first, ItemRef as the fallback: the server computes
|
||||
// `ref` on the item it returns, but this renderer must name
|
||||
// something actionable even from a payload that omits it,
|
||||
@@ -3692,7 +3695,9 @@ Refuses to open when the stored body is behind the item's live
|
||||
collaborative document (an editor in a browser tab holds edits not yet
|
||||
written back), because saving would replace those edits. Open the item
|
||||
in a browser tab so it flushes, then retry — or pass --force to edit the
|
||||
stored body anyway.`,
|
||||
stored body anyway. It refuses the same way when the item holds edits an
|
||||
editor upgrade set aside (see pad item set-aside); opening the item does not
|
||||
restore those, and --force discards them.`,
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
client, cfg := getClient()
|
||||
@@ -3708,7 +3713,7 @@ stored body anyway.`,
|
||||
// predates edits which exist, and the save replaces the WHOLE body.
|
||||
// The concurrency token below cannot catch this: those edits live in
|
||||
// the op-log, not the row, so the row's seq never moved for them.
|
||||
if item.ContentState == models.ContentOutcomeAppliedPendingFlush && !force {
|
||||
if models.IsContentStateStale(item.ContentState) && !force {
|
||||
return staleEditRefusal(item)
|
||||
}
|
||||
|
||||
@@ -3742,7 +3747,7 @@ stored body anyway.`,
|
||||
if rerr != nil {
|
||||
return saveEditRecovery(item, edited, rerr)
|
||||
}
|
||||
if latest.ContentState == models.ContentOutcomeAppliedPendingFlush {
|
||||
if models.IsContentStateStale(latest.ContentState) {
|
||||
return saveEditRecovery(item, edited, staleEditRefusal(latest))
|
||||
}
|
||||
}
|
||||
@@ -3799,6 +3804,12 @@ func staleEditRefusal(item *models.Item) error {
|
||||
if name == "" {
|
||||
name = item.Slug
|
||||
}
|
||||
if item.ContentState == models.ContentStateSetAside {
|
||||
return fmt.Errorf("refusing to edit %s: it holds edits from an earlier editor version that are not "+
|
||||
"in its stored content, and opening the item will not restore them. See them with "+
|
||||
"`pad item set-aside %s`; pass --force to edit the stored content anyway, which discards them",
|
||||
name, name)
|
||||
}
|
||||
return fmt.Errorf("refusing to edit %s: its stored content is behind its live collaborative "+
|
||||
"document — an editor holds edits that have not been written back yet, and saving would "+
|
||||
"replace them with a version derived from a state before them. Open the item in a browser "+
|
||||
@@ -4202,13 +4213,33 @@ func warnUndeclaredFields(item *models.Item) {
|
||||
// matters more on this path than on the write path, because `pad item show
|
||||
// --format markdown` exists precisely to be redirected into a file.
|
||||
func warnContentStale(item *models.Item) {
|
||||
if item == nil || item.ContentState != models.ContentOutcomeAppliedPendingFlush {
|
||||
if item == nil {
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(os.Stderr, "warning: this item's stored content is behind its live collaborative "+
|
||||
"document — an editor holds edits that have not been written back yet, so what follows is "+
|
||||
"the previous content. It catches up when a tab next flushes the item, and nothing on the "+
|
||||
"server forces that to happen.")
|
||||
switch item.ContentState {
|
||||
case models.ContentStatePendingFlush:
|
||||
fmt.Fprintln(os.Stderr, "warning: this item's stored content is behind its live collaborative "+
|
||||
"document — an editor holds edits that have not been written back yet, so what follows is "+
|
||||
"the previous content. It catches up when a tab next flushes the item, and nothing on the "+
|
||||
"server forces that to happen.")
|
||||
case models.ContentStateSetAside:
|
||||
fmt.Fprintln(os.Stderr, setAsideNotice(item))
|
||||
}
|
||||
}
|
||||
|
||||
// setAsideNotice is the one wording of the BUG-3244 state for the CLI's
|
||||
// stale-body notices. It must not say a tab will catch the body up: these
|
||||
// edits were written under an earlier editor schema and opening the item does
|
||||
// not replay them. It names the command that reads or discards them.
|
||||
func setAsideNotice(item *models.Item) string {
|
||||
name := cli.ItemRef(*item)
|
||||
if name == "" {
|
||||
name = item.Slug
|
||||
}
|
||||
return fmt.Sprintf("warning: %s has edits from an earlier editor version that are not in its stored "+
|
||||
"content, and opening the item will NOT restore them; what follows is the content without them. "+
|
||||
"See them with `pad item set-aside %s`, or discard them with `pad item set-aside %s --discard`.",
|
||||
name, name, name)
|
||||
}
|
||||
|
||||
// warnStaleDerivedText prints ONE line to STDERR naming the items whose
|
||||
@@ -4258,7 +4289,13 @@ func warnStaleDerivedText(what string, refs []string, howToSee string) {
|
||||
// It is printed BEFORE the editor is launched. Printed afterwards it would be
|
||||
// read, at best, next to a "Updated TASK-5" line — after the damage.
|
||||
func warnStaleEditSeed(item *models.Item) {
|
||||
if item == nil || item.ContentState != models.ContentOutcomeAppliedPendingFlush {
|
||||
if item == nil || !models.IsContentStateStale(item.ContentState) {
|
||||
return
|
||||
}
|
||||
if item.ContentState == models.ContentStateSetAside {
|
||||
fmt.Fprintln(os.Stderr, "warning: this item holds edits from an earlier editor version that are "+
|
||||
"not in its stored content. You are editing the content without them, and --force will "+
|
||||
"DISCARD them when you save.")
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(os.Stderr, "warning: this item's stored content is behind its live collaborative "+
|
||||
@@ -4368,7 +4405,15 @@ func thousands(n int) string {
|
||||
// printStaleBodyLine is the TABLE-format stale-body notice: STDOUT, first line
|
||||
// (TASK-3132). The markdown and JSON formats do not use it — see showCmd's help.
|
||||
func printStaleBodyLine(item *models.Item) {
|
||||
if item == nil || item.ContentState != models.ContentOutcomeAppliedPendingFlush {
|
||||
if item == nil || !models.IsContentStateStale(item.ContentState) {
|
||||
return
|
||||
}
|
||||
if item.ContentState == models.ContentStateSetAside {
|
||||
name := cli.ItemRef(*item)
|
||||
if name == "" {
|
||||
name = item.Slug
|
||||
}
|
||||
fmt.Printf("⚠ stale body: edits from an earlier editor version are set aside and are NOT in the content below; opening the item will not restore them (pad item set-aside %s).\n", name)
|
||||
return
|
||||
}
|
||||
fmt.Println(`⚠ stale body: an editor holds edits not yet written back, so the content below is the PREVIOUS body; it catches up if and when a tab next flushes the item, which nothing guarantees.`)
|
||||
|
||||
@@ -70,7 +70,7 @@ func playbookListCmd() *cobra.Command {
|
||||
}
|
||||
var staleSummaries []string
|
||||
for _, p := range list {
|
||||
if p.ContentState == models.ContentOutcomeAppliedPendingFlush {
|
||||
if models.IsContentStateStale(p.ContentState) {
|
||||
staleSummaries = append(staleSummaries, p.Ref)
|
||||
}
|
||||
slug := "—"
|
||||
@@ -347,7 +347,14 @@ Example:
|
||||
// is piped into scripts, and the markdown form of both commands exists to be
|
||||
// redirected into a file. This one is never in that stream.
|
||||
func warnPlaybookBodyStale(contentState string) {
|
||||
if contentState != models.ContentOutcomeAppliedPendingFlush {
|
||||
if contentState == models.ContentStateSetAside {
|
||||
// BUG-3244: no tab will catch this body up, so the advice differs.
|
||||
fmt.Fprintln(os.Stderr, "warning: this playbook has edits from an earlier editor version that "+
|
||||
"are not in its stored body, and opening it will not restore them, so the steps below may "+
|
||||
"be superseded. See them with `pad item set-aside <ref>`.")
|
||||
return
|
||||
}
|
||||
if contentState != models.ContentStatePendingFlush {
|
||||
return
|
||||
}
|
||||
fmt.Fprintln(os.Stderr, "warning: this playbook's stored body is behind its live collaborative "+
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"text/tabwriter"
|
||||
"time"
|
||||
|
||||
"github.com/fatih/color"
|
||||
"github.com/spf13/cobra"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/cli"
|
||||
)
|
||||
|
||||
// setAsideCmd reads or discards an item's set-aside edits (BUG-3244).
|
||||
func setAsideCmd() *cobra.Command {
|
||||
var discard bool
|
||||
cmd := &cobra.Command{
|
||||
Use: "set-aside <ref>",
|
||||
Short: "Show or discard edits an editor upgrade set aside",
|
||||
Long: `Show the edits an editor upgrade set aside on an item, or discard them.
|
||||
|
||||
When the collaborative editor's schema changes, edits that were typed under the
|
||||
old editor and never saved to the item's body can no longer be replayed. They
|
||||
are set aside instead of deleted, and the item reads
|
||||
content_state=superseded_set_aside. Opening the item does NOT restore them.
|
||||
|
||||
With no flag this prints them as raw Yjs updates (base64 in --format json), so
|
||||
they can be kept or recovered with a matching editor. --discard deletes them and
|
||||
clears the state. A content write or version restore sent with
|
||||
--overwrite-pending-edits discards them too.`,
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
client, _ := getClient()
|
||||
ws := getWorkspace()
|
||||
|
||||
item, err := client.GetItem(ws, args[0])
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if discard {
|
||||
n, err := client.DiscardCollabSetAside(ws, item.Slug)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if formatFlag == "json" {
|
||||
return cli.PrintJSON(map[string]any{"ref": item.Ref, "discarded": n})
|
||||
}
|
||||
if n == 0 {
|
||||
fmt.Printf("%s has no set-aside edits.\n", item.Ref)
|
||||
return nil
|
||||
}
|
||||
color.New(color.Faint).Printf("Discarded %d set-aside edit row(s) on %s\n", n, item.Ref)
|
||||
return nil
|
||||
}
|
||||
|
||||
res, err := client.ListCollabSetAside(ws, item.Slug)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if formatFlag == "json" {
|
||||
return cli.PrintJSON(res)
|
||||
}
|
||||
if len(res.SetAside) == 0 {
|
||||
fmt.Printf("%s has no set-aside edits.\n", item.Ref)
|
||||
return nil
|
||||
}
|
||||
fmt.Printf("%s: %d edit row(s) set aside by an editor upgrade. Opening the item will not restore them.\n\n",
|
||||
item.Ref, len(res.SetAside))
|
||||
w := tabwriter.NewWriter(cmd.OutOrStdout(), 0, 4, 2, ' ', 0)
|
||||
fmt.Fprintf(w, "OP-LOG ID\tEDITOR SCHEMA\tWRITTEN\tSET ASIDE\tBYTES\tUPDATE (BASE64)\n")
|
||||
for _, r := range res.SetAside {
|
||||
fmt.Fprintf(w, "%d\t%s\t%s\t%s\t%d\t%s\n", r.OpLogID, r.SchemaVersion,
|
||||
r.CreatedAt.UTC().Format(time.RFC3339), r.SetAsideAt.UTC().Format(time.RFC3339),
|
||||
len(r.UpdateData), base64.StdEncoding.EncodeToString(r.UpdateData))
|
||||
}
|
||||
if err := w.Flush(); err != nil {
|
||||
return err
|
||||
}
|
||||
fmt.Printf("\nDiscard them with: pad item set-aside %s --discard\n", item.Ref)
|
||||
return nil
|
||||
},
|
||||
}
|
||||
cmd.Flags().BoolVar(&discard, "discard", false, "delete the set-aside edits and clear the state")
|
||||
return cmd
|
||||
}
|
||||
@@ -126,6 +126,7 @@ func itemCmd() *cobra.Command {
|
||||
commentsCmd(),
|
||||
remindCmd(),
|
||||
remindersCmd(),
|
||||
setAsideCmd(),
|
||||
ackCmd(),
|
||||
unremindCmd(),
|
||||
noteCmd(),
|
||||
|
||||
+72
-2
@@ -395,6 +395,34 @@ func (c *Client) UpdateItem(wsSlug, itemSlug string, input models.ItemUpdate) (*
|
||||
return &result, nil
|
||||
}
|
||||
|
||||
// CollabSetAside is the item's set-aside edits (BUG-3244): op-log rows a collab
|
||||
// schema-version rebuild moved out because they can no longer replay.
|
||||
type CollabSetAside struct {
|
||||
Ref string `json:"ref"`
|
||||
SetAside []models.YjsSetAside `json:"set_aside"`
|
||||
}
|
||||
|
||||
// ListCollabSetAside reads the item's set-aside edits as raw updates.
|
||||
func (c *Client) ListCollabSetAside(wsSlug, itemSlug string) (*CollabSetAside, error) {
|
||||
var result CollabSetAside
|
||||
if err := c.get("/workspaces/"+wsSlug+"/items/"+itemSlug+"/collab-set-aside", &result); err != nil {
|
||||
return nil, wrapItemNotFound(err, itemSlug, wsSlug)
|
||||
}
|
||||
return &result, nil
|
||||
}
|
||||
|
||||
// DiscardCollabSetAside deletes the item's set-aside edits and returns how many
|
||||
// rows it removed.
|
||||
func (c *Client) DiscardCollabSetAside(wsSlug, itemSlug string) (int64, error) {
|
||||
var result struct {
|
||||
Discarded int64 `json:"discarded"`
|
||||
}
|
||||
if err := c.deleteWithResult("/workspaces/"+wsSlug+"/items/"+itemSlug+"/collab-set-aside", &result); err != nil {
|
||||
return 0, wrapItemNotFound(err, itemSlug, wsSlug)
|
||||
}
|
||||
return result.Discarded, nil
|
||||
}
|
||||
|
||||
func (c *Client) DeleteItem(wsSlug, itemSlug string) error {
|
||||
return wrapItemNotFound(c.delete("/workspaces/"+wsSlug+"/items/"+itemSlug), itemSlug, wsSlug)
|
||||
}
|
||||
@@ -1648,6 +1676,30 @@ const ContentPendingFlushCode = "content_pending_flush"
|
||||
const ContentPendingFlushHint = "Re-reading will not clear this: the stored item is unchanged until the open editor saves its edits. " +
|
||||
"Wait for that and re-read, or resend with overwrite_pending_edits=true to replace them."
|
||||
|
||||
// ContentSetAsideHint replaces ContentPendingFlushHint when the refusal is
|
||||
// about edits a collab schema-version rebuild set aside (BUG-3244,
|
||||
// details.set_aside_rows > 0). Waiting and re-reading cannot clear those: no
|
||||
// editor will write them back. Duplicated in internal/mcp; a test
|
||||
// asserts the two match.
|
||||
const ContentSetAsideHint = "Re-reading will not clear this, and neither will waiting or opening the item: these edits were set aside by an editor upgrade and no editor will write them back. " +
|
||||
"Read them with pad item set-aside <ref>, then resend with overwrite_pending_edits=true to discard them."
|
||||
|
||||
// ContentSetAsideRows reads details.set_aside_rows from a content_pending_flush
|
||||
// refusal: how many set-aside edit rows caused it. Zero when the refusal was
|
||||
// about unflushed edits only, or came from a server that predates BUG-3244.
|
||||
func (e *APIError) ContentSetAsideRows() int {
|
||||
if e == nil || len(e.Details) == 0 {
|
||||
return 0
|
||||
}
|
||||
var d struct {
|
||||
SetAsideRows int `json:"set_aside_rows"`
|
||||
}
|
||||
if json.Unmarshal(e.Details, &d) != nil {
|
||||
return 0
|
||||
}
|
||||
return d.SetAsideRows
|
||||
}
|
||||
|
||||
// AsContentPendingFlush reports whether the server refused the write with
|
||||
// content_pending_flush.
|
||||
func (e *APIError) AsContentPendingFlush() bool {
|
||||
@@ -1660,11 +1712,15 @@ func (e *APIError) AsContentPendingFlush() bool {
|
||||
// out. It deliberately does NOT say "re-read and retry" — re-reading returns
|
||||
// the same row and seq, so that loop never ends.
|
||||
func WriteContentPendingFlushError(w io.Writer, apiErr *APIError) {
|
||||
hint, remedy := ContentPendingFlushHint, "Pass --overwrite-pending-edits to replace those edits."
|
||||
if apiErr.ContentSetAsideRows() > 0 {
|
||||
hint, remedy = ContentSetAsideHint, "Pass --overwrite-pending-edits to discard those edits."
|
||||
}
|
||||
envelope := map[string]any{
|
||||
"error": map[string]any{
|
||||
"code": apiErr.Code,
|
||||
"message": apiErr.Message,
|
||||
"hint": ContentPendingFlushHint,
|
||||
"hint": hint,
|
||||
"details": apiErr.Details,
|
||||
},
|
||||
}
|
||||
@@ -1672,7 +1728,7 @@ func WriteContentPendingFlushError(w io.Writer, apiErr *APIError) {
|
||||
fmt.Fprintln(w, StructuredErrorMarker+string(data))
|
||||
}
|
||||
fmt.Fprintln(w, apiErr.Message)
|
||||
fmt.Fprintln(w, "Pass --overwrite-pending-edits to replace those edits.")
|
||||
fmt.Fprintln(w, remedy)
|
||||
}
|
||||
|
||||
// StoredStateUnreadableCode is the structured error code for "the item's
|
||||
@@ -2124,6 +2180,20 @@ func (c *Client) delete(path string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// deleteWithResult is delete for an endpoint that answers with a body.
|
||||
func (c *Client) deleteWithResult(path string, result interface{}) error {
|
||||
req, err := c.newRequest("DELETE", path, nil)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
resp, err := c.httpClient.Do(req)
|
||||
if err != nil {
|
||||
return fmt.Errorf("request failed: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
return c.handleResponse(resp, result)
|
||||
}
|
||||
|
||||
func (c *Client) handleResponse(resp *http.Response, result interface{}) error {
|
||||
if resp.StatusCode >= 400 {
|
||||
return c.parseError(resp)
|
||||
|
||||
+38
-57
@@ -18,9 +18,9 @@ import (
|
||||
// `web/src/lib/collab/schemaVersion.ts` SCHEMA_VERSION on any
|
||||
// breaking change to the Tiptap extension set or Y.Doc shape.
|
||||
//
|
||||
// Frozen until BUG-3244 is closed: that prune deletes unflushed edits
|
||||
// (see maybeRebuildOnSchemaMismatch), and schema_version_guard_test.go
|
||||
// refuses a bump.
|
||||
// Frozen (BUG-3244): the rebuild sets unflushed edits aside, but nothing
|
||||
// recovers them until TASK-3246, and schema_version_guard_test.go refuses a
|
||||
// bump until that lands and the freeze is ruled lifted.
|
||||
const DefaultSchemaVersion = "1"
|
||||
|
||||
// SchemaVersion exposes the version this manager stamps on persisted
|
||||
@@ -495,35 +495,36 @@ func (m *RoomManager) Join(itemID string, conn *websocket.Conn, since int64, con
|
||||
return errTooManyJoinRetries
|
||||
}
|
||||
|
||||
// distantFuture is the prune-everything cutoff we hand to
|
||||
// PruneYjsUpdatesBefore. The store's prune is a strict-less-than on
|
||||
// created_at; any row written with a sane RFC3339 timestamp will
|
||||
// satisfy `created_at < 9999-01-01`.
|
||||
// distantFuture is a prune-everything cutoff for PruneYjsUpdatesBefore. The
|
||||
// store's prune is a strict-less-than on created_at; any row written with a
|
||||
// sane RFC3339 timestamp satisfies `created_at < 9999-01-01`. The rebuild no
|
||||
// longer uses it (it sets rows aside instead); tests that wipe an op-log do.
|
||||
var distantFuture = time.Date(9999, 1, 1, 0, 0, 0, 0, time.UTC)
|
||||
|
||||
// maybeRebuildOnSchemaMismatch implements the TASK-1268 rebuild flow.
|
||||
//
|
||||
// Reads the latest persisted op-log row's schema_version for itemID.
|
||||
// If a row exists AND its version differs from the manager's current
|
||||
// `schemaVersion`, the entire op-log for the item is pruned. Caller
|
||||
// MUST hold the per-item setup lock so a concurrent peer's replayTo
|
||||
// can't load the soon-to-be-pruned rows.
|
||||
// Reads the latest persisted op-log row's schema_version for itemID. If a row
|
||||
// exists AND its version differs from the manager's current `schemaVersion`,
|
||||
// the item's op-log is emptied: rows written under another schema cannot
|
||||
// replay into a document built with this one. Caller MUST hold the per-item
|
||||
// setup lock so a concurrent peer's replayTo can't load the rows being moved.
|
||||
//
|
||||
// Returns nil on the no-rows path and on the matched-version path —
|
||||
// both are "nothing to do". A real DB error from either step short-
|
||||
// circuits Join with the same error so the WS upgrade fails loudly.
|
||||
// Returns nil on the no-rows path and on the matched-version path — both are
|
||||
// "nothing to do". A real DB error short-circuits Join with the same error so
|
||||
// the WS upgrade fails loudly, and leaves the op-log untouched.
|
||||
//
|
||||
// **Data loss disclosure.** When the latest op-log row's id exceeds
|
||||
// `items.content_flushed_op_log_id` for the item (i.e. unflushed
|
||||
// edits exist), the prune is unrecoverable: those ops are stamped
|
||||
// with the OLD schema and can't be replayed against the new schema
|
||||
// regardless of where they're stored. Lazy-seed (TASK-1261) will
|
||||
// repopulate the Y.Doc from items.content, which is stale relative
|
||||
// to the unflushed ops. We log a warn so operators see when a
|
||||
// schema bump is dropping unsaved client edits. Per Codex review of
|
||||
// TASK-1309 round 4 [P2].
|
||||
// **Unflushed edits are set aside, not deleted (BUG-3244).** Content-bearing
|
||||
// rows above items.content_flushed_op_log_id are edits items.content never
|
||||
// received. They used to be pruned with the rest, and content_state then read
|
||||
// clean, so the stale body looked current and the only trace was a WARN.
|
||||
// SetAsideAndClearOpLog now moves them to the set-aside table in the same
|
||||
// transaction that empties the op-log. They still cannot replay here, and
|
||||
// opening the item does not recover them: they hold the item's content_state
|
||||
// at superseded_set_aside until they are recovered (TASK-3246) or explicitly
|
||||
// discarded. Lazy-seed (TASK-1261) then rebuilds the document from
|
||||
// items.content, as before.
|
||||
func (m *RoomManager) maybeRebuildOnSchemaMismatch(itemID string) error {
|
||||
latest, latestID, ok, err := m.store.LatestYjsUpdateSchemaVersion(itemID)
|
||||
latest, _, ok, err := m.store.LatestYjsUpdateSchemaVersion(itemID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -531,44 +532,24 @@ func (m *RoomManager) maybeRebuildOnSchemaMismatch(itemID string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Pre-prune watermark check. Unflushed ops would be lost; we
|
||||
// can't avoid the loss (old-schema ops can't migrate forward),
|
||||
// but we surface it.
|
||||
flushedID, flushedOK, err := m.store.GetItemContentFlushedOpLogID(itemID)
|
||||
if err != nil {
|
||||
// Watermark read failed — proceed with the prune (we still
|
||||
// have to: the schema-mismatch case is non-negotiable) but
|
||||
// log the failure separately.
|
||||
slog.Warn("collab: schema-mismatch rebuild: watermark read failed",
|
||||
"item_id", itemID,
|
||||
"error", err,
|
||||
)
|
||||
} else if !flushedOK || latestID > flushedID {
|
||||
// flushedOK==false → never flushed, every op is unflushed.
|
||||
// latestID > flushedID → some ops past the watermark.
|
||||
// We can't avoid the prune here (old-schema ops can't replay
|
||||
// in the new schema regardless of where they're stored), but
|
||||
// the WARN tells operators a schema bump dropped some
|
||||
// unsaved client edits — they may want to investigate which
|
||||
// items were affected and contact the affected users.
|
||||
// Per Codex review of TASK-1309 round 4 [P2].
|
||||
slog.Warn("collab: schema-mismatch rebuild will drop unflushed ops",
|
||||
"item_id", itemID,
|
||||
"latest_op_log_id", latestID,
|
||||
"content_flushed_op_log_id", flushedID,
|
||||
"watermark_set", flushedOK,
|
||||
)
|
||||
}
|
||||
|
||||
pruned, err := m.store.PruneYjsUpdatesBefore(itemID, distantFuture)
|
||||
setAside, cleared, err := m.store.SetAsideAndClearOpLog(itemID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
slog.Info("collab: schema-version mismatch; pruned op-log",
|
||||
if setAside > 0 {
|
||||
slog.Warn("collab: schema-version mismatch; unflushed edits set aside",
|
||||
"item_id", itemID,
|
||||
"server_version", m.schemaVersion,
|
||||
"persisted_version", latest,
|
||||
"rows_set_aside", setAside,
|
||||
)
|
||||
}
|
||||
slog.Info("collab: schema-version mismatch; cleared op-log",
|
||||
"item_id", itemID,
|
||||
"server_version", m.schemaVersion,
|
||||
"persisted_version", latest,
|
||||
"rows_pruned", pruned,
|
||||
"rows_cleared", cleared,
|
||||
"rows_set_aside", setAside,
|
||||
)
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -35,6 +35,9 @@ type fakeOpLog struct {
|
||||
// it. Missing key = NULL watermark = "never flushed".
|
||||
contentFlushedIDs map[string]int64
|
||||
|
||||
// setAside receives the rows SetAsideAndClearOpLog moves (BUG-3244).
|
||||
setAside []models.YjsUpdate
|
||||
|
||||
// lastRestoreSeqs simulates the DURABLE items.last_restore_seq column
|
||||
// (BUG-2264). Tests populate it to exercise the Join stale-seed fence's
|
||||
// durable read (e.g. a fresh RoomManager on the same store = a "restart"
|
||||
@@ -155,8 +158,8 @@ func (f *fakeOpLog) GetItemContentFlushedOpLogID(itemID string) (int64, bool, er
|
||||
}
|
||||
|
||||
// PruneYjsUpdatesBefore deletes every row for itemID whose CreatedAt
|
||||
// is strictly less than the cutoff. The schema-mismatch rebuild path
|
||||
// passes a far-future cutoff so this becomes "delete every row".
|
||||
// is strictly less than the cutoff. Tests pass a far-future cutoff to
|
||||
// wipe an op-log; the rebuild itself uses SetAsideAndClearOpLog.
|
||||
func (f *fakeOpLog) PruneYjsUpdatesBefore(itemID string, before time.Time) (int64, error) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
@@ -173,6 +176,31 @@ func (f *fakeOpLog) PruneYjsUpdatesBefore(itemID string, before time.Time) (int6
|
||||
return pruned, nil
|
||||
}
|
||||
|
||||
// SetAsideAndClearOpLog mirrors the store's rebuild wipe (BUG-3244): rows
|
||||
// above the item's watermark (every row when it has none) move to setAside,
|
||||
// then every row of the item is deleted. The fake does not model
|
||||
// content_bearing; the real classifier is exercised by the server test.
|
||||
func (f *fakeOpLog) SetAsideAndClearOpLog(itemID string) (int64, int64, error) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
wm, hasWM := f.contentFlushedIDs[itemID]
|
||||
kept := f.rows[:0]
|
||||
var moved, cleared int64
|
||||
for _, r := range f.rows {
|
||||
if r.ItemID != itemID {
|
||||
kept = append(kept, r)
|
||||
continue
|
||||
}
|
||||
cleared++
|
||||
if !hasWM || r.ID > wm {
|
||||
f.setAside = append(f.setAside, r)
|
||||
moved++
|
||||
}
|
||||
}
|
||||
f.rows = kept
|
||||
return moved, cleared, nil
|
||||
}
|
||||
|
||||
// ListDormantOpLogItemsBefore returns item_ids whose entire op-log
|
||||
// is older than the cutoff. Mirrors the production store's
|
||||
// `GROUP BY item_id HAVING MAX(created_at) < ?`. The fake doesn't
|
||||
|
||||
+8
-12
@@ -250,19 +250,15 @@ type opLogStore interface {
|
||||
// returned ID is what the frame is acknowledged with.
|
||||
AppendSyncFrame(itemID string, data []byte, schemaVersion string) (store.SyncFrameAppend, error)
|
||||
LoadYjsUpdatesSince(itemID string, sinceID int64) ([]models.YjsUpdate, error)
|
||||
// LatestYjsUpdateSchemaVersion + PruneYjsUpdatesBefore power the
|
||||
// schema-mismatch rebuild flow (TASK-1268). The room manager
|
||||
// checks the most recent op-log row's stamp on Join and prunes
|
||||
// the entire item's op-log when the persisted version no longer
|
||||
// matches the server's current SCHEMA_VERSION. The latest row's
|
||||
// id is returned alongside the version so the rebuild can detect
|
||||
// when it's about to drop unflushed edits and log a warning.
|
||||
// LatestYjsUpdateSchemaVersion + SetAsideAndClearOpLog power the
|
||||
// schema-mismatch rebuild (TASK-1268). The room manager checks the most
|
||||
// recent op-log row's stamp on Join and empties the item's op-log when
|
||||
// the persisted version no longer matches the server's SCHEMA_VERSION.
|
||||
// SetAsideAndClearOpLog moves the unflushed content-bearing rows to the
|
||||
// set-aside table in the same transaction (BUG-3244), so edits the row
|
||||
// never received are kept, and marked, rather than deleted.
|
||||
LatestYjsUpdateSchemaVersion(itemID string) (string, int64, bool, error)
|
||||
PruneYjsUpdatesBefore(itemID string, before time.Time) (int64, error)
|
||||
// GetItemContentFlushedOpLogID returns the per-item flush
|
||||
// watermark (TASK-1309). Returns (0, false) for items with NULL
|
||||
// watermark or no row.
|
||||
GetItemContentFlushedOpLogID(itemID string) (int64, bool, error)
|
||||
SetAsideAndClearOpLog(itemID string) (setAside, cleared int64, err error)
|
||||
// ListDormantOpLogItemsBefore + PruneItemOpLogIfDormantBefore
|
||||
// power the periodic prune sweep (TASK-1309). Selects items
|
||||
// whose ENTIRE op-log is older than cutoff AND whose
|
||||
|
||||
@@ -7,27 +7,26 @@ import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
// frozenSchemaVersion is the schema version BUG-3244 freezes. A mismatched
|
||||
// join prunes the item's whole op-log (maybeRebuildOnSchemaMismatch), and
|
||||
// every content-bearing row above the flush watermark goes with it: the
|
||||
// first join after a bump deletes that item's unflushed edits and clears
|
||||
// its content_state, so the stale body then reads as current. Until the
|
||||
// rebuild preserves those edits, a bump is a data-loss release. Moving this
|
||||
// value is the act the guard exists to make visible; do it only in the change
|
||||
// that closes BUG-3244.
|
||||
// frozenSchemaVersion is the schema version BUG-3244 froze. A mismatched join
|
||||
// empties the item's op-log (maybeRebuildOnSchemaMismatch). Since BUG-3244 the
|
||||
// unflushed content-bearing rows are SET ASIDE rather than deleted and the item
|
||||
// stays marked superseded_set_aside, so a bump no longer loses them silently.
|
||||
// But nothing yet turns set-aside rows back into text: until TASK-3246 ships a
|
||||
// decoder for the outgoing era, every item with unflushed edits at the moment
|
||||
// of a bump is left with edits a user can read only as raw updates or discard.
|
||||
// Moving this value is the act the guard exists to make visible, and it needs
|
||||
// a ruling on the BUG-3244 / TASK-3246 trail first.
|
||||
const frozenSchemaVersion = "1"
|
||||
|
||||
func TestSchemaVersionFrozenUntilRebuildPreservesUnflushedEdits(t *testing.T) {
|
||||
func TestSchemaVersionFrozenUntilSetAsideEditsAreRecoverable(t *testing.T) {
|
||||
if DefaultSchemaVersion != frozenSchemaVersion {
|
||||
t.Fatalf("DefaultSchemaVersion is %q, frozen at %q: a schema bump prunes every "+
|
||||
"item's unflushed collaborative edits on its next open and then reports the "+
|
||||
"stale body as current (BUG-3244). Before a bump, maybeRebuildOnSchemaMismatch "+
|
||||
"must set the content-bearing rows above the flush watermark aside instead of "+
|
||||
"deleting them, and keep the item marked stale until they are recovered or "+
|
||||
"discarded; closing BUG-3244 without that change does not make a bump safe. "+
|
||||
"A bump that removes or renames a node or mark also needs TASK-3246 (a decoder "+
|
||||
"that recovers the outgoing era's edits). Update frozenSchemaVersion only in "+
|
||||
"that change.",
|
||||
t.Fatalf("DefaultSchemaVersion is %q, frozen at %q. A schema bump sets every "+
|
||||
"item's unflushed collaborative edits aside on its next open (BUG-3244): they "+
|
||||
"are kept and the item reads superseded_set_aside, but no editor can restore "+
|
||||
"them. Before a bump, TASK-3246 must recover the outgoing era's set-aside edits "+
|
||||
"(a decoder for the removed or renamed nodes and marks, or a measurement showing "+
|
||||
"that the new schema decodes the old era as it is), and the lead must rule the "+
|
||||
"freeze lifted on that trail. Update frozenSchemaVersion only in that change.",
|
||||
DefaultSchemaVersion, frozenSchemaVersion)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
package mcp
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/mark3labs/mcp-go/mcp"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/cli"
|
||||
)
|
||||
|
||||
// BUG-3244 ruling 1 at the MCP doors: an agent told about set-aside edits must
|
||||
// not be told that waiting, re-reading or opening the item stores them. Each
|
||||
// test pairs the set-aside leg with the pending leg it must differ from.
|
||||
|
||||
func TestSetAsideHintMatchesAcrossPackages(t *testing.T) {
|
||||
if cli.ContentSetAsideHint != ContentSetAsideHint {
|
||||
t.Errorf("hints differ:\n cli %q\n mcp %q", cli.ContentSetAsideHint, ContentSetAsideHint)
|
||||
}
|
||||
if ContentSetAsideHint == ContentPendingFlushHint {
|
||||
t.Fatal("the set-aside hint must differ from the pending one")
|
||||
}
|
||||
}
|
||||
|
||||
// Stdio: the CLI's marker line carries the hint the refusal's details select,
|
||||
// and the classifier lifts it as-is.
|
||||
func TestSetAsideHintSelectedOnStdio(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
details string
|
||||
want string
|
||||
}{
|
||||
{"set aside", `{"ref":"TASK-2","pending_rows":0,"set_aside_rows":1}`, ContentSetAsideHint},
|
||||
{"pending only", `{"ref":"TASK-2","pending_rows":3}`, ContentPendingFlushHint},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
apiErr := &cli.APIError{Code: cli.ContentPendingFlushCode, Message: "refused", Details: json.RawMessage(tc.details)}
|
||||
var stderr bytes.Buffer
|
||||
cli.WriteContentPendingFlushError(&stderr, apiErr)
|
||||
res := extractStructuredCLIError(stderr.String())
|
||||
if res == nil {
|
||||
t.Fatal("stdio classifier did not recognise the marker line")
|
||||
}
|
||||
env, ok := res.StructuredContent.(ErrorEnvelope)
|
||||
if !ok || env.Error.Hint != tc.want {
|
||||
t.Fatalf("hint = %q, want %q", env.Error.Hint, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Remote: the same selection from the server's response body.
|
||||
func TestSetAsideHintSelectedOnHTTP(t *testing.T) {
|
||||
if got := setAsideRowsIn(json.RawMessage(`{"set_aside_rows":2}`)); got != 2 {
|
||||
t.Fatalf("setAsideRowsIn = %d, want 2", got)
|
||||
}
|
||||
for _, raw := range []string{`{"pending_rows":3}`, ``, `not json`} {
|
||||
if got := setAsideRowsIn(json.RawMessage(raw)); got != 0 {
|
||||
t.Fatalf("setAsideRowsIn(%q) = %d, want 0", raw, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadItemSetAsideMarkerOffersNoTabRemedy(t *testing.T) {
|
||||
read := func(t *testing.T, state string) string {
|
||||
t.Helper()
|
||||
body := `{"ref":"TASK-5","title":"Fix OAuth","fields":"{}","content":"Plan.","content_state":"` + state + `"}`
|
||||
r := &resources{fetcher: &fakeFetcher{stdout: body}}
|
||||
req := mcp.ReadResourceRequest{}
|
||||
req.Params.URI = "pad://workspace/docapp/items/TASK-5"
|
||||
contents, err := r.readItem(context.Background(), req)
|
||||
if err != nil {
|
||||
t.Fatalf("readItem: %v", err)
|
||||
}
|
||||
return contents[0].(mcp.TextResourceContents).Text
|
||||
}
|
||||
setAside := read(t, "superseded_set_aside")
|
||||
if !strings.Contains(setAside, "Stale body") || !strings.Contains(setAside, "superseded_set_aside") {
|
||||
t.Fatalf("set-aside item carries no marker:\n%s", setAside)
|
||||
}
|
||||
if strings.Contains(setAside, "next flushes") || strings.Contains(setAside, "catches up") {
|
||||
t.Fatalf("set-aside marker promises a tab will catch the body up:\n%s", setAside)
|
||||
}
|
||||
if pending := read(t, "applied_pending_flush"); !strings.Contains(pending, "next flushes") {
|
||||
t.Fatalf("pending marker lost its wording:\n%s", pending)
|
||||
}
|
||||
}
|
||||
@@ -967,6 +967,9 @@ func classifyHTTPStatusKind(
|
||||
// the row is unchanged, so the re-read returns the same seq
|
||||
// and the retry is refused again (BUG-3133).
|
||||
hint = ContentPendingFlushHint
|
||||
if setAsideRowsIn(upstream.Details) > 0 {
|
||||
hint = ContentSetAsideHint
|
||||
}
|
||||
}
|
||||
if upstream.Code == string(ErrStoredStateUnreadable) {
|
||||
// Same reason: the stored value stays undecodable, so every
|
||||
@@ -1308,6 +1311,28 @@ func permissionHintFor(bodyMsg, route string) string {
|
||||
const ContentPendingFlushHint = "Re-reading will not clear this: the stored item is unchanged until the open editor saves its edits. " +
|
||||
"Wait for that and re-read, or resend with overwrite_pending_edits=true to replace them."
|
||||
|
||||
// ContentSetAsideHint replaces ContentPendingFlushHint when the refusal is
|
||||
// about edits a collab schema-version rebuild set aside (BUG-3244,
|
||||
// details.set_aside_rows > 0). Waiting and re-reading cannot clear those: no
|
||||
// editor will write them back. Duplicated in internal/cli; a test
|
||||
// asserts the two match.
|
||||
const ContentSetAsideHint = "Re-reading will not clear this, and neither will waiting or opening the item: these edits were set aside by an editor upgrade and no editor will write them back. " +
|
||||
"Read them with pad item set-aside <ref>, then resend with overwrite_pending_edits=true to discard them."
|
||||
|
||||
// setAsideRowsIn reads set_aside_rows from a refusal's details (BUG-3244).
|
||||
func setAsideRowsIn(details json.RawMessage) int {
|
||||
if len(details) == 0 {
|
||||
return 0
|
||||
}
|
||||
var d struct {
|
||||
SetAsideRows int `json:"set_aside_rows"`
|
||||
}
|
||||
if json.Unmarshal(details, &d) != nil {
|
||||
return 0
|
||||
}
|
||||
return d.SetAsideRows
|
||||
}
|
||||
|
||||
// conflictHintFor generates the actionable hint for ErrConflict.
|
||||
func conflictHintFor(bodyMsg, route string) string {
|
||||
parts := []string{"Re-read the current item state and retry the update."}
|
||||
|
||||
@@ -6,7 +6,7 @@ Pad is a project tracker for developers and AI agents — issues (TASK, BUG), pl
|
||||
|
||||
If the user is asking general code questions with no project-management thread, you don't need this server.
|
||||
|
||||
## Tool surface (v0.55)
|
||||
## Tool surface (v0.56)
|
||||
|
||||
Ten resource × action tools, plus `pad_set_workspace` (which takes a `workspace` slug only — no action enum). Eleven tools total.
|
||||
|
||||
|
||||
@@ -414,12 +414,21 @@ func formatItemAsMarkdown(jsonBlob string) (string, error) {
|
||||
// reading top-down meets it in the sentence before the text it is
|
||||
// about. The literal state token is included so the line is greppable
|
||||
// by the same name the JSON field uses, not only readable as prose.
|
||||
if state, _ := item["content_state"].(string); state == models.ContentOutcomeAppliedPendingFlush {
|
||||
switch state, _ := item["content_state"].(string); state {
|
||||
case models.ContentStatePendingFlush:
|
||||
fmt.Fprintf(&b, "> **Stale body** (`content_state: %s`) — this item's stored content is "+
|
||||
"behind its live collaborative document. An editor holds edits that have not been "+
|
||||
"written back, so the body below is the previous content. It catches up when a tab "+
|
||||
"next flushes the item, and nothing on the server forces that to happen.\n\n",
|
||||
models.ContentOutcomeAppliedPendingFlush)
|
||||
models.ContentStatePendingFlush)
|
||||
case models.ContentStateSetAside:
|
||||
// BUG-3244: the remedy above is false for this state, so it is not
|
||||
// offered. No MCP action reads set-aside rows; the CLI and API do.
|
||||
fmt.Fprintf(&b, "> **Stale body** (`content_state: %s`) — this item has edits from an "+
|
||||
"earlier editor version that are not in its stored content, and opening the item will "+
|
||||
"not restore them. The body below is the content without them. They can be read or "+
|
||||
"discarded with `pad item set-aside`, or through the item's collab-set-aside API.\n\n",
|
||||
models.ContentStateSetAside)
|
||||
}
|
||||
|
||||
// Body. Pad items often have rich markdown here already; pass
|
||||
|
||||
+25
-1
@@ -1017,6 +1017,30 @@ const CmdhelpVersion = "0.1"
|
||||
// browser tab and BUG-3000 carries the open half — so no surface
|
||||
// here states a duration.
|
||||
//
|
||||
// 0.56 — BUG-3244. ADDITIVE bump: `content_state` gains a second value,
|
||||
// `superseded_set_aside`, which no write produces. It reaches this
|
||||
// surface everywhere the POST-0.33 entry below says `content_state`
|
||||
// does: `pad_item` get, `pad_item` list with full=true, `pad_playbook`
|
||||
// run, and the line above the body in the
|
||||
// `pad://workspace/{ws}/items/{ref}` resource. It means an editor
|
||||
// schema-version rebuild SET ASIDE edits that items.content never
|
||||
// received: they are kept, but no tab can restore them, so the
|
||||
// `applied_pending_flush` advice (open the item, re-read later) is
|
||||
// wrong for it and the resource line says so. It wins when both would
|
||||
// apply. A consumer that treats any `content_state` as "the body is
|
||||
// stale" stays correct; one that branches on the word must add a case.
|
||||
// The same value makes every content write refuse, token or not, with
|
||||
// the existing `content_pending_flush` code plus an additive
|
||||
// `details.set_aside_rows`; the structured hint branches on it (it
|
||||
// names `pad item set-aside` and `overwrite_pending_edits`, never
|
||||
// opening the item). `overwrite_pending_edits` still lifts it,
|
||||
// discarding the rows, and they are counted in
|
||||
// `warnings.pruned_pending_edits`. Reading or discarding the rows
|
||||
// alone (GET/DELETE collab-set-aside, `pad item set-aside`) is NOT on
|
||||
// this catalog. No name, enum or param shape changed; the bump is
|
||||
// owed because an agent that learned the one-word vocabulary now
|
||||
// meets a word it does not know, whose remedy differs.
|
||||
//
|
||||
// 0.55 — BUG-2659. BEHAVIOR bump, on the v0.36 / v0.40 grounds: the same
|
||||
// `pad_item.action=search` call can now answer from a DIFFERENT
|
||||
// collection. Its `collection` param reaches /search exactly as sent,
|
||||
@@ -1635,7 +1659,7 @@ const CmdhelpVersion = "0.1"
|
||||
// this surface can receive it; the entry exists so a future action does
|
||||
// not collapse it to permission_denied. When an action that can reach
|
||||
// it is added, that addition is the contract change and owns the bump.
|
||||
const ToolSurfaceVersion = "0.55"
|
||||
const ToolSurfaceVersion = "0.56"
|
||||
|
||||
// MetaVersionURI is the canonical URI of the queryable version document.
|
||||
// Lives outside the pad://workspace/{ws}/... namespace because it's a
|
||||
|
||||
+29
-12
@@ -161,18 +161,35 @@ type ItemExport struct {
|
||||
// `pad db migrate-to-pg` — export piped into import across two databases,
|
||||
// after which the source is abandoned — an unflushed body becomes the only
|
||||
// copy. The marker is what lets that door refuse; it is not a repair.
|
||||
ContentState string `json:"content_state,omitempty"`
|
||||
Fields string `json:"fields"`
|
||||
Tags string `json:"tags"`
|
||||
Pinned bool `json:"pinned"`
|
||||
SortOrder int `json:"sort_order"`
|
||||
ParentID string `json:"parent_id,omitempty"`
|
||||
CreatedBy string `json:"created_by"`
|
||||
LastModifiedBy string `json:"last_modified_by"`
|
||||
Source string `json:"source"`
|
||||
ItemNumber int `json:"item_number"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
UpdatedAt string `json:"updated_at"`
|
||||
ContentState string `json:"content_state,omitempty"`
|
||||
// CollabSetAside carries the edits a collab schema-version rebuild set
|
||||
// aside on this item (BUG-3244), which no body holds. Unlike the op-log,
|
||||
// which a bundle never carries, these travel: they can only be recovered
|
||||
// by a decoder, not by a tab, so a copy of the rows is the whole of what
|
||||
// can be kept. Additive and omitempty, on the same no-version-bump grounds
|
||||
// as ContentState.
|
||||
CollabSetAside []YjsSetAsideExport `json:"collab_set_aside,omitempty"`
|
||||
Fields string `json:"fields"`
|
||||
Tags string `json:"tags"`
|
||||
Pinned bool `json:"pinned"`
|
||||
SortOrder int `json:"sort_order"`
|
||||
ParentID string `json:"parent_id,omitempty"`
|
||||
CreatedBy string `json:"created_by"`
|
||||
LastModifiedBy string `json:"last_modified_by"`
|
||||
Source string `json:"source"`
|
||||
ItemNumber int `json:"item_number"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
UpdatedAt string `json:"updated_at"`
|
||||
}
|
||||
|
||||
// YjsSetAsideExport is one set-aside edit row in a bundle (BUG-3244). The item
|
||||
// it belongs to is the enclosing ItemExport; update_data is base64 in JSON.
|
||||
type YjsSetAsideExport struct {
|
||||
OpLogID int64 `json:"op_log_id"`
|
||||
SchemaVersion string `json:"schema_version"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
SetAsideAt string `json:"set_aside_at"`
|
||||
UpdateData []byte `json:"update_data"`
|
||||
}
|
||||
|
||||
// CommentExport holds a comment's data for export.
|
||||
|
||||
+27
-2
@@ -257,6 +257,28 @@ func (r *RelationTargetSet) UnmarshalJSON(b []byte) error {
|
||||
// never matching (BUG-2995).
|
||||
const ContentOutcomeAppliedPendingFlush = "applied_pending_flush"
|
||||
|
||||
// ContentStatePendingFlush is Item.ContentState's value when the op-log holds
|
||||
// unflushed content-bearing rows: the live document is ahead of the row, and a
|
||||
// tab opening the item can catch the row up. The same string as the write
|
||||
// outcome, for the reason given on ContentOutcomeAppliedPendingFlush.
|
||||
const ContentStatePendingFlush = ContentOutcomeAppliedPendingFlush
|
||||
|
||||
// ContentStateSetAside is Item.ContentState's value when edits the row never
|
||||
// received were SET ASIDE by a collab schema-version rebuild (BUG-3244): they
|
||||
// were written under an older editor schema, so they can no longer replay into
|
||||
// the live document and opening the item does NOT recover them. They are kept,
|
||||
// readable as raw updates, until someone recovers or discards them. Advice that
|
||||
// names a remedy must branch on this value; a door that only marks the body
|
||||
// stale should ask IsContentStateStale instead.
|
||||
const ContentStateSetAside = "superseded_set_aside"
|
||||
|
||||
// IsContentStateStale reports whether an Item.ContentState value says the
|
||||
// stored body is behind edits that exist elsewhere, for any reason. It is the
|
||||
// one helper for doors that mark a body stale without saying what to do.
|
||||
func IsContentStateStale(state string) bool {
|
||||
return state == ContentStatePendingFlush || state == ContentStateSetAside
|
||||
}
|
||||
|
||||
// IsReservedItemField reports whether key is system-written metadata rather than
|
||||
// a user-facing schema field. Callers that filter, migrate, or render an item's
|
||||
// fields map should consult this rather than enumerating the constants.
|
||||
@@ -320,10 +342,13 @@ type Item struct {
|
||||
// whenever the row is current, which is the overwhelmingly common case, so a
|
||||
// consumer that does not know this field sees byte-identical responses.
|
||||
//
|
||||
// It takes the same values as ItemWriteWarnings.ContentOutcome, deliberately:
|
||||
// a caller should have ONE story about where content is, whether it learned it
|
||||
// Its pending value is ItemWriteWarnings.ContentOutcome's, deliberately: a
|
||||
// caller should have ONE story about where content is, whether it learned it
|
||||
// from a write response or from a read. The name differs because a write
|
||||
// warning describes what a REQUEST did, while this describes what the ROW is.
|
||||
// It has a second value no write produces, superseded_set_aside (BUG-3244,
|
||||
// ContentStateSetAside): edits a schema rebuild set aside, which opening the
|
||||
// item does not restore. It wins when both apply; see IsContentStateStale.
|
||||
//
|
||||
// The predicate is "the op-log holds a CONTENT-BEARING row above
|
||||
// items.content_flushed_op_log_id", i.e. the document is ahead of the row.
|
||||
|
||||
@@ -19,3 +19,17 @@ type YjsUpdate struct {
|
||||
SchemaVersion string `json:"schema_version"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
}
|
||||
|
||||
// YjsSetAside is an op-log row a collab schema-version rebuild moved out of
|
||||
// the op-log because it can no longer replay (BUG-3244). OpLogID is its id in
|
||||
// item_yjs_updates before the move; UpdateData is the frame, byte for byte,
|
||||
// and SchemaVersion the editor era it was written under.
|
||||
type YjsSetAside struct {
|
||||
ID int64 `json:"id"`
|
||||
ItemID string `json:"item_id"`
|
||||
OpLogID int64 `json:"op_log_id"`
|
||||
UpdateData []byte `json:"update_data"`
|
||||
SchemaVersion string `json:"schema_version"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
SetAsideAt time.Time `json:"set_aside_at"`
|
||||
}
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/websocket"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/collab"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3244 x BUG-3240: after a bumped rebuild sets the unflushed rows aside,
|
||||
// the op-log is empty, so the room's elector grants the next writable open
|
||||
// `seed: true` and that tab seeds its empty doc from items.content, the body
|
||||
// WITHOUT the set-aside edits. That is intended (the editor has to show the
|
||||
// stored body; the set-aside rows are kept for TASK-3246), but the seed must
|
||||
// neither clear the set-aside rows nor the state that names them, and a second
|
||||
// open must be neither granted nor rebuilt again.
|
||||
func TestBUG3244RebuildThenSeedGrantKeepsTheItemSuperseded(t *testing.T) {
|
||||
f := newBUG3244Fixture(t)
|
||||
f.append(t, bug3244Frame, "1")
|
||||
|
||||
rm := collab.NewRoomManagerWithConfig(f.srv.store, f.bus, collab.RoomManagerConfig{SchemaVersion: "2"})
|
||||
t.Cleanup(rm.Close)
|
||||
f.srv.SetCollabRoomManager(rm)
|
||||
ts := httptest.NewServer(f.srv)
|
||||
t.Cleanup(ts.Close)
|
||||
u, _ := url.Parse(ts.URL)
|
||||
|
||||
dial := func() (*websocket.Conn, collab.ControlMessage) {
|
||||
t.Helper()
|
||||
c, resp, err := (&websocket.Dialer{HandshakeTimeout: 3 * time.Second}).Dial(
|
||||
"ws://"+u.Host+"/api/v1/collab/"+f.itemID+"?schema_version=2", nil)
|
||||
if err != nil {
|
||||
st := ""
|
||||
if resp != nil {
|
||||
st = resp.Status
|
||||
}
|
||||
t.Fatalf("dial: %v %s", err, st)
|
||||
}
|
||||
_ = c.SetReadDeadline(time.Now().Add(3 * time.Second))
|
||||
for {
|
||||
mt, data, err := c.ReadMessage()
|
||||
if err != nil {
|
||||
t.Fatalf("no op_log_cursor frame after join: %v", err)
|
||||
}
|
||||
if mt != websocket.TextMessage {
|
||||
continue
|
||||
}
|
||||
var msg collab.ControlMessage
|
||||
if err := json.Unmarshal(data, &msg); err != nil {
|
||||
t.Fatalf("unmarshal control frame: %v", err)
|
||||
}
|
||||
if msg.Type == collab.ControlMessageOpLogCursor {
|
||||
_ = c.SetReadDeadline(time.Time{})
|
||||
return c, msg
|
||||
}
|
||||
}
|
||||
}
|
||||
setAside := func() int {
|
||||
t.Helper()
|
||||
rows, err := f.srv.store.ListYjsSetAside(f.itemID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListYjsSetAside: %v", err)
|
||||
}
|
||||
return len(rows)
|
||||
}
|
||||
|
||||
// The first open at the bumped version rebuilds, and is the seeder.
|
||||
a, cursor := dial()
|
||||
defer a.Close()
|
||||
if !cursor.Seed {
|
||||
t.Fatal("the first open after the rebuild was not granted the seed")
|
||||
}
|
||||
if n := setAside(); n != 1 {
|
||||
t.Fatalf("set-aside rows after the rebuild = %d, want 1", n)
|
||||
}
|
||||
if got := f.contentState(t); got != models.ContentStateSetAside {
|
||||
t.Fatalf("content_state after the rebuild = %q, want %q", got, models.ContentStateSetAside)
|
||||
}
|
||||
|
||||
// It seeds: its first update is a content-bearing frame at the new version.
|
||||
seed := []byte{0x00, 0x02, 0x03, 0x07, 0x08, 0x09}
|
||||
if err := a.WriteMessage(websocket.BinaryMessage, seed); err != nil {
|
||||
t.Fatalf("seed write: %v", err)
|
||||
}
|
||||
deadline := time.Now().Add(3 * time.Second)
|
||||
for f.opLogRows(t) != 1 {
|
||||
if time.Now().After(deadline) {
|
||||
t.Fatalf("the seed was not persisted: op-log rows = %d", f.opLogRows(t))
|
||||
}
|
||||
time.Sleep(5 * time.Millisecond)
|
||||
}
|
||||
|
||||
// A second open while the seeder is live: not granted, and no second
|
||||
// rebuild (the op-log now holds a current-version row).
|
||||
b, cursor := dial()
|
||||
defer b.Close()
|
||||
if cursor.Seed {
|
||||
t.Fatal("a second open was granted the seed while the seeder is live")
|
||||
}
|
||||
if n := f.opLogRows(t); n != 1 {
|
||||
t.Fatalf("op-log rows after the second open = %d, want 1 (the seed; no second rebuild)", n)
|
||||
}
|
||||
if n := setAside(); n != 1 {
|
||||
t.Fatalf("set-aside rows after the seed and a second open = %d, want 1", n)
|
||||
}
|
||||
if got := f.contentState(t); got != models.ContentStateSetAside {
|
||||
t.Fatalf("content_state after the seed = %q, want %q (set-aside wins over pending)", got, models.ContentStateSetAside)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,188 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/websocket"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/collab"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3244: the first join at a new schema version used to prune the item's
|
||||
// whole op-log, unflushed content-bearing rows included, and the pending
|
||||
// marker went with them, so the stale body read as current afterwards.
|
||||
// Measured on main 3f75c2fe before the fix: rows 1 -> 0, content_state
|
||||
// "applied_pending_flush" -> "".
|
||||
//
|
||||
// Each case runs the SAME fixture at the old version (the control: nothing
|
||||
// may move) and at a bumped one, so an assertion that holds for both legs is
|
||||
// not mistaken for evidence about the rebuild.
|
||||
|
||||
// A Sync Update frame with a payload: content-bearing under the BUG-3124
|
||||
// classifier, the shape a tab's unflushed keystrokes are persisted in.
|
||||
var bug3244Frame = []byte{0x00, 0x02, 0x03, 0x01, 0x02, 0x03}
|
||||
|
||||
type bug3244Fixture struct {
|
||||
srv *Server
|
||||
bus *collab.MemoryOpBus
|
||||
itemID string
|
||||
}
|
||||
|
||||
func newBUG3244Fixture(t *testing.T) *bug3244Fixture {
|
||||
t.Helper()
|
||||
srv := testServer(t)
|
||||
bus := collab.NewMemoryOpBus()
|
||||
t.Cleanup(bus.Close)
|
||||
itemID := seedCollabFixture(t, srv, "Bug3244")
|
||||
body := "stored body"
|
||||
if _, err := srv.store.UpdateItem(itemID, models.ItemUpdate{Content: &body}); err != nil {
|
||||
t.Fatalf("UpdateItem: %v", err)
|
||||
}
|
||||
return &bug3244Fixture{srv: srv, bus: bus, itemID: itemID}
|
||||
}
|
||||
|
||||
func (f *bug3244Fixture) append(t *testing.T, frame []byte, version string) int64 {
|
||||
t.Helper()
|
||||
id, err := f.srv.store.AppendYjsUpdate(f.itemID, frame, version)
|
||||
if err != nil {
|
||||
t.Fatalf("AppendYjsUpdate: %v", err)
|
||||
}
|
||||
return id
|
||||
}
|
||||
|
||||
// joinAt installs a RoomManager at serverVersion (the upgraded binary, or the
|
||||
// same one for the control) and makes one collab join announcing it. It
|
||||
// returns once the join has passed the rebuild: the rebuild runs under the
|
||||
// item's setup lock before the room replays, so the op_log_cursor text frame
|
||||
// the server sends after replay is the barrier.
|
||||
func (f *bug3244Fixture) joinAt(t *testing.T, serverVersion string) {
|
||||
t.Helper()
|
||||
rm := collab.NewRoomManagerWithConfig(f.srv.store, f.bus, collab.RoomManagerConfig{SchemaVersion: serverVersion})
|
||||
t.Cleanup(rm.Close)
|
||||
f.srv.SetCollabRoomManager(rm)
|
||||
ts := httptest.NewServer(f.srv)
|
||||
t.Cleanup(ts.Close)
|
||||
|
||||
u, _ := url.Parse(ts.URL)
|
||||
c, resp, err := (&websocket.Dialer{HandshakeTimeout: 3 * time.Second}).Dial(
|
||||
"ws://"+u.Host+"/api/v1/collab/"+f.itemID+"?schema_version="+serverVersion, nil)
|
||||
if err != nil {
|
||||
st := ""
|
||||
if resp != nil {
|
||||
st = resp.Status
|
||||
}
|
||||
t.Fatalf("dial: %v %s", err, st)
|
||||
}
|
||||
defer c.Close()
|
||||
_ = c.SetReadDeadline(time.Now().Add(3 * time.Second))
|
||||
for {
|
||||
mt, _, err := c.ReadMessage()
|
||||
if err != nil {
|
||||
t.Fatalf("no op_log_cursor frame after join: %v", err)
|
||||
}
|
||||
if mt == websocket.TextMessage {
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (f *bug3244Fixture) contentState(t *testing.T) string {
|
||||
t.Helper()
|
||||
it, err := f.srv.store.GetItem(f.itemID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetItem: %v", err)
|
||||
}
|
||||
if it.Content != "stored body" {
|
||||
t.Fatalf("items.content changed: %q", it.Content)
|
||||
}
|
||||
return it.ContentState
|
||||
}
|
||||
|
||||
func (f *bug3244Fixture) opLogRows(t *testing.T) int {
|
||||
t.Helper()
|
||||
rows, err := f.srv.store.LoadYjsUpdatesSince(f.itemID, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("LoadYjsUpdatesSince: %v", err)
|
||||
}
|
||||
return len(rows)
|
||||
}
|
||||
|
||||
func TestBUG3244SchemaRebuildKeepsUnflushedEditsMarked(t *testing.T) {
|
||||
t.Run("control: same version, nothing moves", func(t *testing.T) {
|
||||
f := newBUG3244Fixture(t)
|
||||
f.append(t, bug3244Frame, "1")
|
||||
f.joinAt(t, "1")
|
||||
if n := f.opLogRows(t); n != 1 {
|
||||
t.Fatalf("op-log rows = %d, want 1", n)
|
||||
}
|
||||
if got := f.contentState(t); got != models.ContentStatePendingFlush {
|
||||
t.Fatalf("content_state = %q, want %q", got, models.ContentStatePendingFlush)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("bumped: the op-log is rebuilt and the item stays marked", func(t *testing.T) {
|
||||
f := newBUG3244Fixture(t)
|
||||
f.append(t, bug3244Frame, "1")
|
||||
f.joinAt(t, "2")
|
||||
if n := f.opLogRows(t); n != 0 {
|
||||
t.Fatalf("op-log rows = %d, want 0: old-era rows must never replay into a new-schema doc", n)
|
||||
}
|
||||
if got := f.contentState(t); got != models.ContentStateSetAside {
|
||||
t.Fatalf("content_state = %q, want %q: the unflushed edit is gone from the op-log "+
|
||||
"and the body is still stale, so it must not read as current", got, models.ContentStateSetAside)
|
||||
}
|
||||
})
|
||||
|
||||
// Only edits the row never received are kept: a row at or below the flush
|
||||
// watermark is already in items.content, and a SyncStep1 frame cannot
|
||||
// change the document. What is kept is the frame byte for byte, with its
|
||||
// era and original op-log id, because those are what recovery needs.
|
||||
t.Run("bumped: only unflushed content-bearing rows are set aside, verbatim", func(t *testing.T) {
|
||||
f := newBUG3244Fixture(t)
|
||||
flushed := f.append(t, []byte{0x00, 0x02, 0x03, 0x09, 0x08, 0x07}, "1")
|
||||
if err := f.srv.store.SetItemContentFlushedOpLogIDForTesting(f.itemID, flushed); err != nil {
|
||||
t.Fatalf("set watermark: %v", err)
|
||||
}
|
||||
f.append(t, []byte{0x00, 0x00, 0x01, 0x00}, "1") // SyncStep1: never content
|
||||
kept := f.append(t, bug3244Frame, "1")
|
||||
f.joinAt(t, "2")
|
||||
|
||||
rows, err := f.srv.store.ListYjsSetAside(f.itemID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListYjsSetAside: %v", err)
|
||||
}
|
||||
if len(rows) != 1 {
|
||||
t.Fatalf("set-aside rows = %d, want 1 (only the unflushed content-bearing frame): %+v", len(rows), rows)
|
||||
}
|
||||
r := rows[0]
|
||||
if r.OpLogID != kept || string(r.UpdateData) != string(bug3244Frame) || r.SchemaVersion != "1" {
|
||||
t.Fatalf("set-aside row = {op_log_id %d, data %v, era %q}, want {%d, %v, %q}",
|
||||
r.OpLogID, r.UpdateData, r.SchemaVersion, kept, bug3244Frame, "1")
|
||||
}
|
||||
if n := f.opLogRows(t); n != 0 {
|
||||
t.Fatalf("op-log rows = %d, want 0", n)
|
||||
}
|
||||
})
|
||||
|
||||
// Nothing unflushed: the rebuild has nothing to keep, and the item must not
|
||||
// be marked, or every item on the instance would read stale after a bump.
|
||||
t.Run("bumped: a fully flushed item is not marked", func(t *testing.T) {
|
||||
f := newBUG3244Fixture(t)
|
||||
id := f.append(t, bug3244Frame, "1")
|
||||
if err := f.srv.store.SetItemContentFlushedOpLogIDForTesting(f.itemID, id); err != nil {
|
||||
t.Fatalf("set watermark: %v", err)
|
||||
}
|
||||
f.joinAt(t, "2")
|
||||
if got := f.contentState(t); got != "" {
|
||||
t.Fatalf("content_state = %q, want empty", got)
|
||||
}
|
||||
rows, err := f.srv.store.ListYjsSetAside(f.itemID)
|
||||
if err != nil || len(rows) != 0 {
|
||||
t.Fatalf("set-aside rows = %d (err %v), want 0", len(rows), err)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,258 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3244 ruling 2: rows a schema rebuild set aside refuse EVERY content
|
||||
// write, token or not, unless it sends overwrite_pending_edits, which discards
|
||||
// them and counts them in warnings.pruned_pending_edits. The refusal must not
|
||||
// offer "open the item" as a remedy, because no tab can restore them.
|
||||
//
|
||||
// The set-aside state is made with the rebuild's own store call; the rebuild
|
||||
// reaching it from a bumped join is TestBUG3244SchemaRebuildKeepsUnflushedEditsMarked.
|
||||
|
||||
func (f bug3133Fixture) seedSetAside(t *testing.T) {
|
||||
t.Helper()
|
||||
f.seedPending(t)
|
||||
moved, _, err := f.srv.store.SetAsideAndClearOpLog(f.item.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("SetAsideAndClearOpLog: %v", err)
|
||||
}
|
||||
if moved != 1 {
|
||||
t.Fatalf("premise: want 1 row set aside, got %d", moved)
|
||||
}
|
||||
if got, _ := f.srv.store.GetItem(f.item.ID); got.ContentState != models.ContentStateSetAside {
|
||||
t.Fatalf("premise: content_state = %q, want %q", got.ContentState, models.ContentStateSetAside)
|
||||
}
|
||||
}
|
||||
|
||||
func (f bug3133Fixture) setAsideRows(t *testing.T) int {
|
||||
t.Helper()
|
||||
rows, err := f.srv.store.ListYjsSetAside(f.item.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListYjsSetAside: %v", err)
|
||||
}
|
||||
return len(rows)
|
||||
}
|
||||
|
||||
func assertSetAsideRefusal(t *testing.T, f bug3133Fixture, before *models.Item, rrCode int, body string, code string, details map[string]any) {
|
||||
t.Helper()
|
||||
if rrCode != http.StatusConflict {
|
||||
t.Fatalf("want 409, got %d: %s", rrCode, body)
|
||||
}
|
||||
if code != "content_pending_flush" {
|
||||
t.Fatalf("code = %q, want content_pending_flush", code)
|
||||
}
|
||||
if details["set_aside_rows"] != float64(1) {
|
||||
t.Errorf("set_aside_rows = %v, want 1", details["set_aside_rows"])
|
||||
}
|
||||
if strings.Contains(body, "browser") || strings.Contains(body, "Wait for the open editor") {
|
||||
t.Errorf("the refusal offers opening the item, which cannot restore set-aside edits: %s", body)
|
||||
}
|
||||
after, _ := f.srv.store.GetItem(f.item.ID)
|
||||
if after.Content != before.Content || after.Seq != before.Seq {
|
||||
t.Errorf("a refused write moved the row: content %q→%q seq %d→%d", before.Content, after.Content, before.Seq, after.Seq)
|
||||
}
|
||||
if n := f.setAsideRows(t); n != 1 {
|
||||
t.Errorf("a refused write changed the set-aside rows: %d, want 1", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetAsideRefusesTokenlessContentWrite(t *testing.T) {
|
||||
// Control: the same write to an item with nothing set aside lands, as it
|
||||
// did before BUG-3244. Without it the refusal below could be anything.
|
||||
t.Run("control: nothing set aside", func(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
rr := doRequest(f.srv, "PATCH", f.path, map[string]any{"content": "replacement body"})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("want 200, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("set aside: refused", func(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
f.seedSetAside(t)
|
||||
before, _ := f.srv.store.GetItem(f.item.ID)
|
||||
rr := doRequest(f.srv, "PATCH", f.path, map[string]any{"content": "replacement body"})
|
||||
code, details := "", map[string]any(nil)
|
||||
if rr.Code == http.StatusConflict {
|
||||
code, details = decodeErrorCode(t, rr)
|
||||
}
|
||||
assertSetAsideRefusal(t, f, before, rr.Code, rr.Body.String(), code, details)
|
||||
})
|
||||
}
|
||||
|
||||
func TestSetAsideRefusesTokenedContentWrite(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
f.seedSetAside(t)
|
||||
before, _ := f.srv.store.GetItem(f.item.ID)
|
||||
rr := doRequest(f.srv, "PATCH", f.path, map[string]any{"content": "replacement body", "expected_seq": before.Seq})
|
||||
code, details := "", map[string]any(nil)
|
||||
if rr.Code == http.StatusConflict {
|
||||
code, details = decodeErrorCode(t, rr)
|
||||
}
|
||||
assertSetAsideRefusal(t, f, before, rr.Code, rr.Body.String(), code, details)
|
||||
}
|
||||
|
||||
func TestSetAsideOverwriteDiscardsAndReports(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
f.seedSetAside(t)
|
||||
rr := doRequest(f.srv, "PATCH", f.path, map[string]any{"content": "replacement body", "overwrite_pending_edits": true})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("want 200, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
got := decodeWrite(t, rr)
|
||||
if got.Warnings == nil || got.Warnings.PrunedPendingEdits != 1 {
|
||||
t.Fatalf("warnings.pruned_pending_edits = %+v, want 1 (the discarded set-aside row)", got.Warnings)
|
||||
}
|
||||
if n := f.setAsideRows(t); n != 0 {
|
||||
t.Fatalf("set-aside rows after the override = %d, want 0", n)
|
||||
}
|
||||
after, _ := f.srv.store.GetItem(f.item.ID)
|
||||
if after.Content != "replacement body" || after.ContentState != "" {
|
||||
t.Fatalf("after the override: content %q, content_state %q", after.Content, after.ContentState)
|
||||
}
|
||||
}
|
||||
|
||||
// The tab's own flush writes the new-era document; it is not replacing the
|
||||
// set-aside edits (the body never held them), so it is not refused, and it
|
||||
// must not clear them either.
|
||||
func TestSetAsideSurvivesCollabSnapshotFlush(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
f.seedSetAside(t)
|
||||
rr := doRequest(f.srv, "PATCH", f.path+"?source=collab-snapshot", map[string]any{"content": "flushed body"})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("want 200, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if n := f.setAsideRows(t); n != 1 {
|
||||
t.Fatalf("set-aside rows after a flush = %d, want 1", n)
|
||||
}
|
||||
after, _ := f.srv.store.GetItem(f.item.ID)
|
||||
if after.ContentState != models.ContentStateSetAside {
|
||||
t.Fatalf("content_state after a flush = %q, want %q", after.ContentState, models.ContentStateSetAside)
|
||||
}
|
||||
}
|
||||
|
||||
// A caught-up tab stamps the flush watermark (BUG-3124 unit B). That clears
|
||||
// the unflushed-rows state by design; it must not clear the set-aside state,
|
||||
// which no tab can have caught up with.
|
||||
func TestSetAsideSurvivesWatermarkStamp(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
f.seedSetAside(t)
|
||||
if _, err := f.srv.store.AppendYjsUpdate(f.item.ID, bug3244Frame, "1"); err != nil {
|
||||
t.Fatalf("AppendYjsUpdate: %v", err)
|
||||
}
|
||||
max, _, err := f.srv.store.MaxOpLogID(f.item.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := f.srv.store.SetItemContentFlushedOpLogIDForTesting(f.item.ID, max); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
after, _ := f.srv.store.GetItem(f.item.ID)
|
||||
if after.ContentState != models.ContentStateSetAside {
|
||||
t.Fatalf("content_state after the watermark caught up = %q, want %q", after.ContentState, models.ContentStateSetAside)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetAsideRefusesRestoreUntilOverwrite(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
// Each write under a different source, so the per-(actor, source) version
|
||||
// throttle cannot swallow one (the BUG-3031 test's recipe).
|
||||
for _, step := range []struct{ content, source string }{{"first body", "web"}, {"second body", "skill"}, {"third body", "cli"}} {
|
||||
body := step.content
|
||||
if _, err := f.srv.store.UpdateItem(f.item.ID, models.ItemUpdate{Content: &body, LastModifiedBy: "user", Source: step.source}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
versions, err := f.srv.store.ListItemVersionsResolved(f.item.ID, "third body")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var firstID string
|
||||
for _, v := range versions {
|
||||
if v.Content == "first body" {
|
||||
firstID = v.ID
|
||||
}
|
||||
}
|
||||
if firstID == "" {
|
||||
t.Fatalf("premise: no version resolving to the first body among %d", len(versions))
|
||||
}
|
||||
f.seedSetAside(t)
|
||||
restore := f.path + "/versions/" + firstID + "/restore"
|
||||
|
||||
rr := doRequest(f.srv, "POST", restore, nil)
|
||||
if rr.Code != http.StatusConflict {
|
||||
t.Fatalf("want 409, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
code, details := decodeErrorCode(t, rr)
|
||||
if code != "content_pending_flush" || details["set_aside_rows"] != float64(1) {
|
||||
t.Fatalf("refusal = %q %v, want content_pending_flush with set_aside_rows 1", code, details)
|
||||
}
|
||||
if strings.Contains(rr.Body.String(), "browser") {
|
||||
t.Errorf("the restore refusal offers opening the item: %s", rr.Body.String())
|
||||
}
|
||||
if n := f.setAsideRows(t); n != 1 {
|
||||
t.Fatalf("a refused restore changed the set-aside rows: %d", n)
|
||||
}
|
||||
|
||||
rr = doRequest(f.srv, "POST", restore, map[string]any{"overwrite_pending_edits": true})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("override: want 200, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if n := f.setAsideRows(t); n != 0 {
|
||||
t.Fatalf("set-aside rows after the override restore = %d, want 0", n)
|
||||
}
|
||||
// BUG-3230 U2 names how many edits a restore deleted; the set-aside rows
|
||||
// it discarded are counted there, as a content write counts them.
|
||||
var restored models.Item
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &restored); err != nil {
|
||||
t.Fatalf("decode restore response: %v", err)
|
||||
}
|
||||
if restored.Warnings == nil || restored.Warnings.PrunedPendingEdits != 1 {
|
||||
t.Fatalf("restore warnings.pruned_pending_edits = %+v, want 1 (the discarded set-aside row)", restored.Warnings)
|
||||
}
|
||||
}
|
||||
|
||||
// Ruling 3: the rows are readable as raw updates and an explicit discard
|
||||
// clears the state without writing the body.
|
||||
func TestSetAsideReadAndDiscardEndpoints(t *testing.T) {
|
||||
f := newBug3133Fixture(t)
|
||||
f.seedSetAside(t)
|
||||
before, _ := f.srv.store.GetItem(f.item.ID)
|
||||
|
||||
rr := doRequest(f.srv, "GET", f.path+"/collab-set-aside", nil)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("GET: want 200, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var got struct {
|
||||
Ref string `json:"ref"`
|
||||
SetAside []models.YjsSetAside `json:"set_aside"`
|
||||
}
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(got.SetAside) != 1 || string(got.SetAside[0].UpdateData) != string(pendingEditFrame) {
|
||||
t.Fatalf("GET set_aside = %+v, want the one seeded frame verbatim", got.SetAside)
|
||||
}
|
||||
|
||||
rr = doRequest(f.srv, "DELETE", f.path+"/collab-set-aside", nil)
|
||||
if rr.Code != http.StatusOK || !strings.Contains(rr.Body.String(), `"discarded":1`) {
|
||||
t.Fatalf("DELETE: want 200 discarded 1, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
after, _ := f.srv.store.GetItem(f.item.ID)
|
||||
if after.ContentState != "" || after.Content != before.Content || after.Seq != before.Seq {
|
||||
t.Fatalf("after discard: content_state %q, content %q→%q, seq %d→%d",
|
||||
after.ContentState, before.Content, after.Content, before.Seq, after.Seq)
|
||||
}
|
||||
rr = doRequest(f.srv, "GET", f.path+"/collab-set-aside", nil)
|
||||
if rr.Code != http.StatusOK || !strings.Contains(rr.Body.String(), `"set_aside":[]`) {
|
||||
t.Fatalf("GET after discard: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,85 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3244: the read and discard doors for edits a collab schema-version
|
||||
// rebuild set aside. Those rows hold the item's content_state at
|
||||
// superseded_set_aside; no tab can replay them, so they leave only by being
|
||||
// recovered (TASK-3246) or discarded here, or by a content write or restore
|
||||
// that sends overwrite_pending_edits.
|
||||
|
||||
type setAsideResponse struct {
|
||||
Ref string `json:"ref"`
|
||||
SetAside []models.YjsSetAside `json:"set_aside"`
|
||||
}
|
||||
|
||||
// handleListCollabSetAside returns the item's set-aside rows as raw updates
|
||||
// (update_data is base64 in JSON), in their original op-log order. Anyone who
|
||||
// can read the item can read them: they are that item's own edits.
|
||||
func (s *Server) handleListCollabSetAside(w http.ResponseWriter, r *http.Request) {
|
||||
_, item, ok := s.resolveSetAsideItem(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
rows, err := s.store.ListYjsSetAside(item.ID)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, setAsideResponse{Ref: itemRefOrSlug(*item), SetAside: rows})
|
||||
}
|
||||
|
||||
// handleDiscardCollabSetAside deletes the item's set-aside rows, which clears
|
||||
// the superseded_set_aside state without writing the body. It needs edit
|
||||
// permission, like the content write that would otherwise discard them.
|
||||
func (s *Server) handleDiscardCollabSetAside(w http.ResponseWriter, r *http.Request) {
|
||||
workspaceID, item, ok := s.resolveSetAsideItem(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if !s.requireEditPermission(w, r, workspaceID, item.ID, item.CollectionID) {
|
||||
return
|
||||
}
|
||||
n, err := s.store.DiscardYjsSetAside(item.ID)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
if n > 0 {
|
||||
// The rows were the only copy of those edits; say who removed them.
|
||||
slog.Info("collab: set-aside edits discarded",
|
||||
"item_id", item.ID,
|
||||
"rows", n,
|
||||
"user_id", currentUserID(r),
|
||||
)
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]int64{"discarded": n})
|
||||
}
|
||||
|
||||
func (s *Server) resolveSetAsideItem(w http.ResponseWriter, r *http.Request) (string, *models.Item, bool) {
|
||||
workspaceID, ok := s.getWorkspaceID(w, r)
|
||||
if !ok {
|
||||
return "", nil, false
|
||||
}
|
||||
itemSlug := chi.URLParam(r, "itemSlug")
|
||||
item, err := s.store.ResolveItem(workspaceID, itemSlug)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return "", nil, false
|
||||
}
|
||||
if item == nil {
|
||||
s.writeItemResolveError(w, r, workspaceID, itemSlug)
|
||||
return "", nil, false
|
||||
}
|
||||
if !s.requireItemVisible(w, r, workspaceID, item) {
|
||||
return "", nil, false
|
||||
}
|
||||
return workspaceID, item, true
|
||||
}
|
||||
@@ -354,15 +354,29 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request
|
||||
// is the restore that DELETES them, and the response names how
|
||||
// many (warnings.pruned_pending_edits), as a content update's
|
||||
// does. Counted here, under the same lock, so it is exact.
|
||||
//
|
||||
// BUG-3244: rows a schema rebuild set aside are edits in no
|
||||
// body either, so they refuse the restore the same way, and
|
||||
// the override discards them with the op-log and counts them
|
||||
// into the same warning.
|
||||
n, perr := s.store.CountPendingContentRowsTx(tx, item.ID)
|
||||
if perr != nil {
|
||||
return perr
|
||||
}
|
||||
if n > 0 && !body.OverwritePendingEdits {
|
||||
pendingRefusal = &store.ContentPendingFlushError{ItemID: item.ID, PendingRows: n}
|
||||
sa, serr := s.store.CountYjsSetAsideTx(tx, item.ID)
|
||||
if serr != nil {
|
||||
return serr
|
||||
}
|
||||
if (n > 0 || sa > 0) && !body.OverwritePendingEdits {
|
||||
pendingRefusal = &store.ContentPendingFlushError{ItemID: item.ID, PendingRows: n, SetAsideRows: sa}
|
||||
return pendingRefusal
|
||||
}
|
||||
prunedPendingEdits = n
|
||||
if sa > 0 {
|
||||
if _, derr := s.store.DeleteYjsSetAsideTx(tx, item.ID); derr != nil {
|
||||
return derr
|
||||
}
|
||||
}
|
||||
prunedPendingEdits = n + sa
|
||||
// Capture the pre-restore seq under the per-item + workspace seq
|
||||
// lock, before any mutation (BUG-2276 P2 — see the baselineSeq note
|
||||
// above). `existing` is the row as read at the top of the update tx.
|
||||
@@ -406,7 +420,7 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request
|
||||
}, reconcile)
|
||||
if werr != nil {
|
||||
if pendingRefusal != nil {
|
||||
writeRestorePendingFlushError(w, itemRefOrSlug(*item), pendingRefusal.PendingRows)
|
||||
writeRestorePendingFlushError(w, itemRefOrSlug(*item), pendingRefusal)
|
||||
return
|
||||
}
|
||||
if errors.Is(werr, errRestoreItemGone) {
|
||||
|
||||
@@ -2214,9 +2214,19 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
|
||||
// collab-snapshot flush is exempt: it is the tab writing those edits.
|
||||
// refuse_pending_edits asks for the same refusal with no token (BUG-3230
|
||||
// U0): the pane's raw saves cannot carry one.
|
||||
if input.Content != nil && !collabSnapshot && !input.OverwritePendingEdits &&
|
||||
(input.ExpectedSeq != nil || input.ExpectedUpdatedAt != "" || input.RefusePendingEdits) {
|
||||
openChildrenPrecheck = composePendingContentGuard(s, item.ID, openChildrenPrecheck)
|
||||
//
|
||||
// BUG-3244: rows a schema rebuild set aside refuse EVERY such write, token
|
||||
// or not, and overwrite_pending_edits discards them on every path, not just
|
||||
// the direct one: no tab holds them, so the applier replacing the live
|
||||
// document does not touch them.
|
||||
var discardedSetAside int
|
||||
if input.Content != nil && !collabSnapshot {
|
||||
if input.OverwritePendingEdits {
|
||||
openChildrenPrecheck = composeSetAsideDiscard(s, item.ID, openChildrenPrecheck, &discardedSetAside)
|
||||
} else {
|
||||
tokenGuarded := input.ExpectedSeq != nil || input.ExpectedUpdatedAt != "" || input.RefusePendingEdits
|
||||
openChildrenPrecheck = composePendingContentGuard(s, item.ID, tokenGuarded, openChildrenPrecheck)
|
||||
}
|
||||
}
|
||||
// BUG-3133 D: how many unflushed edit rows the direct path's prune deleted.
|
||||
var prunedPendingEdits int
|
||||
@@ -2287,7 +2297,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if pending, ok := store.AsContentPendingFlushError(err); ok {
|
||||
writeContentPendingFlushError(w, itemRefOrSlug(*item), pending.PendingRows)
|
||||
writeContentPendingFlushError(w, itemRefOrSlug(*item), pending)
|
||||
return
|
||||
}
|
||||
// BUG-2804: the item rename cascade refuses renames that would process
|
||||
@@ -2531,6 +2541,10 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
|
||||
// Set-aside rows an override discarded are edits the write deleted, like the
|
||||
// prune's, so they are reported in the same count (BUG-3244).
|
||||
prunedPendingEdits += discardedSetAside
|
||||
|
||||
// Advisory, post-write, same as create (BUG-2850).
|
||||
if len(undeclaredFields) > 0 || len(droppedDefaults) > 0 || contentAppliedPendingFlush || prunedPendingEdits > 0 {
|
||||
warnings := &models.ItemWriteWarnings{
|
||||
|
||||
@@ -386,20 +386,55 @@ func composePruneWithPrecheck(s *Server, itemID string, inner func(*sql.Tx, *mod
|
||||
// between that tx and the applier's setContent: the unguarded window shrinks
|
||||
// from read-to-PATCH to one server hop. It is not closed, because appendMu
|
||||
// cannot be held across the applier's network round trip.
|
||||
func composePendingContentGuard(s *Server, itemID string, inner func(*sql.Tx, *models.Item) error) func(*sql.Tx, *models.Item) error {
|
||||
//
|
||||
// tokenGuarded says whether the write asked for the BUG-3133 refusal (a version
|
||||
// token, or refuse_pending_edits). Set-aside rows (BUG-3244) refuse the write
|
||||
// either way, because unlike unflushed rows no tab will ever put them in the
|
||||
// body: a tokenless write over them is not "the latest body wins", it is the
|
||||
// last copy of those edits being dropped. Only overwrite_pending_edits, which
|
||||
// skips this guard, discards them.
|
||||
func composePendingContentGuard(s *Server, itemID string, tokenGuarded bool, inner func(*sql.Tx, *models.Item) error) func(*sql.Tx, *models.Item) error {
|
||||
return func(tx *sql.Tx, existing *models.Item) error {
|
||||
if inner != nil {
|
||||
if err := inner(tx, existing); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
n, err := s.store.CountPendingContentRowsTx(tx, itemID)
|
||||
setAside, err := s.store.CountYjsSetAsideTx(tx, itemID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if n > 0 {
|
||||
return &store.ContentPendingFlushError{ItemID: itemID, PendingRows: n}
|
||||
pending := 0
|
||||
if tokenGuarded || setAside > 0 {
|
||||
if pending, err = s.store.CountPendingContentRowsTx(tx, itemID); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if setAside > 0 || (tokenGuarded && pending > 0) {
|
||||
return &store.ContentPendingFlushError{ItemID: itemID, PendingRows: pending, SetAsideRows: setAside}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
// composeSetAsideDiscard is overwrite_pending_edits' half for set-aside rows
|
||||
// (BUG-3244): the caller said it means to replace edits the body does not
|
||||
// hold, so the rows a schema rebuild set aside are deleted in the write's own
|
||||
// transaction, rolled back with it on any refusal, and counted into
|
||||
// warnings.pruned_pending_edits. Assigned rather than added, like the prune's
|
||||
// count, so a retried transaction does not double it.
|
||||
func composeSetAsideDiscard(s *Server, itemID string, inner func(*sql.Tx, *models.Item) error, discarded *int) func(*sql.Tx, *models.Item) error {
|
||||
return func(tx *sql.Tx, existing *models.Item) error {
|
||||
if inner != nil {
|
||||
if err := inner(tx, existing); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
n, err := s.store.DeleteYjsSetAsideTx(tx, itemID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
*discarded = int(n)
|
||||
return nil
|
||||
}
|
||||
}
|
||||
@@ -462,7 +497,7 @@ func (s *Server) writeTypedItemRefusal(w http.ResponseWriter, item *models.Item,
|
||||
// differently. Kept LAST, after every typed arm, because a substring match can
|
||||
// swallow a typed refusal whose message happens to contain the text.
|
||||
if pending, ok := store.AsContentPendingFlushError(err); ok {
|
||||
writeContentPendingFlushError(w, itemRefOrSlug(*item), pending.PendingRows)
|
||||
writeContentPendingFlushError(w, itemRefOrSlug(*item), pending)
|
||||
return true
|
||||
}
|
||||
if strings.Contains(err.Error(), "UNIQUE constraint") || strings.Contains(err.Error(), "duplicate key") {
|
||||
|
||||
@@ -287,7 +287,8 @@ type ItemCopyResultWarnings struct {
|
||||
|
||||
// SourceContentState says the body this copy carried was BEHIND the
|
||||
// source item's live collaborative document (BUG-3032 / BUG-3000). Takes
|
||||
// the one value models.Item.ContentState takes, and is omitted entirely
|
||||
// the values models.Item.ContentState takes (superseded_set_aside since
|
||||
// BUG-3244, for edits a tab cannot restore), and is omitted entirely
|
||||
// when the source row was current — so a clean copy's response is
|
||||
// byte-identical to one produced before this field existed.
|
||||
//
|
||||
|
||||
@@ -470,21 +470,26 @@ func writeContentNotAppliedError(w http.ResponseWriter, ref string, landedFields
|
||||
// same row and the same seq, so a read-and-retry loop would spin. The message
|
||||
// says what clears it, because nothing server-side does: an open tab flushes
|
||||
// within seconds; with no tab, only opening the item or the override.
|
||||
func writeContentPendingFlushError(w http.ResponseWriter, ref string, pendingRows int) {
|
||||
writeJSON(w, http.StatusConflict, map[string]any{
|
||||
"error": map[string]any{
|
||||
"code": "content_pending_flush",
|
||||
"message": fmt.Sprintf(
|
||||
"%s has unflushed collaborative edits that are not in the body you read; its version token does not cover them. "+
|
||||
"Wait for the open editor to save them and re-read, or open the item in a browser if none is open, "+
|
||||
"or resend with overwrite_pending_edits to replace them.",
|
||||
ref),
|
||||
"details": map[string]any{
|
||||
"ref": ref,
|
||||
"pending_rows": pendingRows,
|
||||
},
|
||||
},
|
||||
})
|
||||
//
|
||||
// BUG-3244: when the item holds rows a schema rebuild set aside, the refusal
|
||||
// covers every content write and the message must NOT offer opening the item,
|
||||
// because no tab can restore those edits. Same code, so one client branch
|
||||
// still handles both; details gain set_aside_rows.
|
||||
func writeContentPendingFlushError(w http.ResponseWriter, ref string, e *store.ContentPendingFlushError) {
|
||||
var msg string
|
||||
if e.SetAsideRows > 0 {
|
||||
msg = fmt.Sprintf(
|
||||
"%s has edits from an earlier editor version that are not in its body, and opening the item will not restore them. "+
|
||||
"Read them with `pad item set-aside %s` if they matter, then resend with overwrite_pending_edits to replace the body and discard them.",
|
||||
ref, ref)
|
||||
} else {
|
||||
msg = fmt.Sprintf(
|
||||
"%s has unflushed collaborative edits that are not in the body you read; its version token does not cover them. "+
|
||||
"Wait for the open editor to save them and re-read, or open the item in a browser if none is open, "+
|
||||
"or resend with overwrite_pending_edits to replace them.",
|
||||
ref)
|
||||
}
|
||||
writePendingFlushConflict(w, ref, msg, e)
|
||||
}
|
||||
|
||||
// writeRestorePendingFlushError answers the BUG-3031 refusal: a version restore
|
||||
@@ -492,20 +497,39 @@ func writeContentPendingFlushError(w http.ResponseWriter, ref string, pendingRow
|
||||
// details as the BUG-3133 refusal, so one client branch handles both; the
|
||||
// message differs because a restore carries no version token. It names the loss
|
||||
// plainly: the undo point a restore leaves is built from the saved body, so
|
||||
// those edits would be in no version afterwards.
|
||||
func writeRestorePendingFlushError(w http.ResponseWriter, ref string, pendingRows int) {
|
||||
// those edits would be in no version afterwards. Set-aside rows (BUG-3244)
|
||||
// branch the remedy for the same reason as above.
|
||||
func writeRestorePendingFlushError(w http.ResponseWriter, ref string, e *store.ContentPendingFlushError) {
|
||||
var msg string
|
||||
if e.SetAsideRows > 0 {
|
||||
msg = fmt.Sprintf(
|
||||
"%s has edits from an earlier editor version that are not in its saved body, and opening the item will not restore them. "+
|
||||
"Restoring now would discard them, and no version would keep them. "+
|
||||
"Read them with `pad item set-aside %s` if they matter, then resend with overwrite_pending_edits to discard them.",
|
||||
ref, ref)
|
||||
} else {
|
||||
msg = fmt.Sprintf(
|
||||
"%s has unsaved edits from another editor session that are not in its saved body. "+
|
||||
"Restoring now would discard them, and no version would keep them. "+
|
||||
"Open the item in a browser to save them first, or resend with overwrite_pending_edits to discard them.",
|
||||
ref)
|
||||
}
|
||||
writePendingFlushConflict(w, ref, msg, e)
|
||||
}
|
||||
|
||||
func writePendingFlushConflict(w http.ResponseWriter, ref, msg string, e *store.ContentPendingFlushError) {
|
||||
details := map[string]any{
|
||||
"ref": ref,
|
||||
"pending_rows": e.PendingRows,
|
||||
}
|
||||
if e.SetAsideRows > 0 {
|
||||
details["set_aside_rows"] = e.SetAsideRows
|
||||
}
|
||||
writeJSON(w, http.StatusConflict, map[string]any{
|
||||
"error": map[string]any{
|
||||
"code": "content_pending_flush",
|
||||
"message": fmt.Sprintf(
|
||||
"%s has unsaved edits from another editor session that are not in its saved body. "+
|
||||
"Restoring now would discard them, and no version would keep them. "+
|
||||
"Open the item in a browser to save them first, or resend with overwrite_pending_edits to discard them.",
|
||||
ref),
|
||||
"details": map[string]any{
|
||||
"ref": ref,
|
||||
"pending_rows": pendingRows,
|
||||
},
|
||||
"code": "content_pending_flush",
|
||||
"message": msg,
|
||||
"details": details,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1896,6 +1896,11 @@ func (s *Server) setupRouter() {
|
||||
// Content-free flush-watermark stamp from a caught-up
|
||||
// tab (BUG-3124 unit B). Web-client only; no CLI/MCP.
|
||||
r.Post("/collab-watermark", s.handleStampCollabWatermark)
|
||||
// Edits a collab schema-version rebuild set aside
|
||||
// (BUG-3244): read them as raw updates, or discard
|
||||
// them. CLI: pad item set-aside. Not on MCP.
|
||||
r.Get("/collab-set-aside", s.handleListCollabSetAside)
|
||||
r.Delete("/collab-set-aside", s.handleDiscardCollabSetAside)
|
||||
// Cross-workspace copy PREFLIGHT (PLAN-2357 /
|
||||
// TASK-2364). Reports what a copy into another
|
||||
// workspace would carry, drop and need, and
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
// BUG-3244 codex r1: the rebuild's setup lock does not stop a connected peer
|
||||
// appending, and on Postgres each statement in the move takes its own
|
||||
// snapshot. A content-bearing row committed after the rows to keep were chosen
|
||||
// but before the op-log was cleared used to be deleted without being set
|
||||
// aside. The rows kept are now the rows the DELETE returned, so a row that
|
||||
// lands in that gap is either left in the op-log or set aside, never lost.
|
||||
//
|
||||
// Postgres only: SQLite serialises writers, so the gap cannot open there (and
|
||||
// an append from another connection inside the transaction would deadlock).
|
||||
func TestSetAsideKeepsARowAppendedInsideTheMove(t *testing.T) {
|
||||
s := testStore(t)
|
||||
if s.dialect.Driver() == DriverSQLite {
|
||||
t.Skip("Postgres only: SQLite serialises writers, so the gap cannot open")
|
||||
}
|
||||
ws := createTestWorkspace(t, s, "SetAsideRace")
|
||||
col := createTestCollection(t, s, ws.ID, "Tasks")
|
||||
item := createTestItem(t, s, ws.ID, col.ID, "Racing", "stored body")
|
||||
|
||||
first := []byte{0x00, 0x02, 0x05, 0x01, 0x44, 0x00, 0x7F, 0x00}
|
||||
late := []byte{0x00, 0x02, 0x05, 0x01, 0x45, 0x00, 0x7F, 0x00}
|
||||
if _, err := s.AppendYjsUpdate(item.ID, first, "1"); err != nil {
|
||||
t.Fatalf("AppendYjsUpdate: %v", err)
|
||||
}
|
||||
|
||||
setAsideBeforeClearHook = func() {
|
||||
// Its own connection, committed before the move's DELETE runs.
|
||||
if _, err := s.AppendYjsUpdate(item.ID, late, "1"); err != nil {
|
||||
t.Errorf("append inside the move: %v", err)
|
||||
}
|
||||
}
|
||||
t.Cleanup(func() { setAsideBeforeClearHook = nil })
|
||||
|
||||
moved, cleared, err := s.SetAsideAndClearOpLog(item.ID)
|
||||
setAsideBeforeClearHook = nil
|
||||
if err != nil {
|
||||
t.Fatalf("SetAsideAndClearOpLog: %v", err)
|
||||
}
|
||||
|
||||
rows, err := s.ListYjsSetAside(item.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ops, err := s.LoadYjsUpdatesSince(item.ID, 0)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Wherever the late row went, it must be somewhere.
|
||||
found := len(ops) == 1 && string(ops[0].UpdateData) == string(late)
|
||||
for _, r := range rows {
|
||||
if string(r.UpdateData) == string(late) {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("the row appended inside the move is in neither table: moved %d, cleared %d, set-aside %d, op-log %d", moved, cleared, len(rows), len(ops))
|
||||
}
|
||||
if int64(len(rows)) != moved {
|
||||
t.Fatalf("moved = %d but %d rows are set aside", moved, len(rows))
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,195 @@
|
||||
package store_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/PerpetualSoftware/pad/internal/store"
|
||||
"github.com/PerpetualSoftware/pad/internal/store/storetest"
|
||||
)
|
||||
|
||||
// BUG-3244, store half, on both dialects (migration 097 / pg 072): the
|
||||
// set-aside move, the content_state value it produces, and the bundle.
|
||||
// Postgres legs skip unless PAD_TEST_POSTGRES_URL is set (make test-pg).
|
||||
|
||||
// A content-bearing sync update, distinct from every other frame here.
|
||||
var setAsideFrame = []byte{0x00, 0x02, 0x05, 0x01, 0x44, 0x00, 0x7F, 0x00}
|
||||
|
||||
func eachBackend(t *testing.T, fn func(t *testing.T, s *store.Store)) {
|
||||
for _, backend := range []struct {
|
||||
name string
|
||||
open func(*testing.T) *store.Store
|
||||
}{
|
||||
{"SQLite", storetest.NewSQLite},
|
||||
{"Postgres", storetest.NewPostgres},
|
||||
} {
|
||||
t.Run(backend.name, func(t *testing.T) { fn(t, backend.open(t)) })
|
||||
}
|
||||
}
|
||||
|
||||
func setAsideOne(t *testing.T, s *store.Store, itemID string) {
|
||||
t.Helper()
|
||||
if _, err := s.AppendYjsUpdate(itemID, setAsideFrame, "1"); err != nil {
|
||||
t.Fatalf("AppendYjsUpdate: %v", err)
|
||||
}
|
||||
moved, cleared, err := s.SetAsideAndClearOpLog(itemID)
|
||||
if err != nil {
|
||||
t.Fatalf("SetAsideAndClearOpLog: %v", err)
|
||||
}
|
||||
if moved != 1 || cleared != 1 {
|
||||
t.Fatalf("SetAsideAndClearOpLog = (%d moved, %d cleared), want (1, 1)", moved, cleared)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetAsideMovesRowsAndMarksTheItem(t *testing.T) {
|
||||
eachBackend(t, func(t *testing.T, s *store.Store) {
|
||||
_, _, item := seedStaleItem(t, s)
|
||||
setAsideOne(t, s, item.ID)
|
||||
|
||||
got, err := s.GetItem(item.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.ContentState != models.ContentStateSetAside || got.Content != "stored body" {
|
||||
t.Fatalf("after set-aside: content_state %q content %q", got.ContentState, got.Content)
|
||||
}
|
||||
rows, err := s.ListYjsSetAside(item.ID)
|
||||
if err != nil || len(rows) != 1 || string(rows[0].UpdateData) != string(setAsideFrame) {
|
||||
t.Fatalf("ListYjsSetAside = %+v (err %v), want the frame verbatim", rows, err)
|
||||
}
|
||||
if ops, _ := s.LoadYjsUpdatesSince(item.ID, 0); len(ops) != 0 {
|
||||
t.Fatalf("op-log rows after set-aside = %d, want 0", len(ops))
|
||||
}
|
||||
|
||||
// Discard clears the state; the body is untouched.
|
||||
n, err := s.DiscardYjsSetAside(item.ID)
|
||||
if err != nil || n != 1 {
|
||||
t.Fatalf("DiscardYjsSetAside = %d, %v", n, err)
|
||||
}
|
||||
got, _ = s.GetItem(item.ID)
|
||||
if got.ContentState != "" || got.Content != "stored body" {
|
||||
t.Fatalf("after discard: content_state %q content %q", got.ContentState, got.Content)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// The bundle carries set-aside rows (condition a) and an import reattaches
|
||||
// them, so the imported item reads the same state as its source.
|
||||
func TestSetAsideRoundTripsThroughTheBundle(t *testing.T) {
|
||||
eachBackend(t, func(t *testing.T, s *store.Store) {
|
||||
_, _, item := seedStaleItem(t, s)
|
||||
setAsideOne(t, s, item.ID)
|
||||
|
||||
exp, err := s.ExportWorkspace("cs")
|
||||
if err != nil {
|
||||
t.Fatalf("ExportWorkspace: %v", err)
|
||||
}
|
||||
var ex *models.ItemExport
|
||||
for i := range exp.Items {
|
||||
if exp.Items[i].ID == item.ID {
|
||||
ex = &exp.Items[i]
|
||||
}
|
||||
}
|
||||
if ex == nil || ex.ContentState != models.ContentStateSetAside || len(ex.CollabSetAside) != 1 ||
|
||||
string(ex.CollabSetAside[0].UpdateData) != string(setAsideFrame) || ex.CollabSetAside[0].SchemaVersion != "1" {
|
||||
t.Fatalf("exported item = %+v, want content_state %q and the one frame", ex, models.ContentStateSetAside)
|
||||
}
|
||||
|
||||
dst, err := s.ImportWorkspace(exp, "Imported set-aside", "", "test")
|
||||
if err != nil {
|
||||
t.Fatalf("ImportWorkspace: %v", err)
|
||||
}
|
||||
items, err := s.ListItems(dst.ID, models.ItemListParams{})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var imported *models.Item
|
||||
for i := range items {
|
||||
if items[i].Title == item.Title {
|
||||
imported = &items[i]
|
||||
}
|
||||
}
|
||||
if imported == nil {
|
||||
t.Fatal("imported item not found")
|
||||
}
|
||||
rows, err := s.ListYjsSetAside(imported.ID)
|
||||
if err != nil || len(rows) != 1 || string(rows[0].UpdateData) != string(setAsideFrame) {
|
||||
t.Fatalf("imported set-aside rows = %+v (err %v)", rows, err)
|
||||
}
|
||||
got, _ := s.GetItem(imported.ID)
|
||||
if got.ContentState != models.ContentStateSetAside {
|
||||
t.Fatalf("imported content_state = %q, want %q", got.ContentState, models.ContentStateSetAside)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// A bundle row that did not come from a server is skipped, not stored and not
|
||||
// fatal: this file's lenient import precedent.
|
||||
func TestSetAsideImportSkipsMalformedRows(t *testing.T) {
|
||||
s := storetest.NewSQLite(t)
|
||||
_, _, item := seedStaleItem(t, s)
|
||||
setAsideOne(t, s, item.ID)
|
||||
exp, err := s.ExportWorkspace("cs")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for i := range exp.Items {
|
||||
if exp.Items[i].ID == item.ID {
|
||||
good := exp.Items[i].CollabSetAside[0]
|
||||
badVersion, badTime, empty := good, good, good
|
||||
badVersion.SchemaVersion = "1\x00; DROP"
|
||||
badTime.CreatedAt = "yesterday"
|
||||
empty.UpdateData = nil
|
||||
exp.Items[i].CollabSetAside = append(exp.Items[i].CollabSetAside, badVersion, badTime, empty)
|
||||
}
|
||||
}
|
||||
dst, err := s.ImportWorkspace(exp, "Imported malformed", "", "test")
|
||||
if err != nil {
|
||||
t.Fatalf("ImportWorkspace: %v", err)
|
||||
}
|
||||
items, _ := s.ListItems(dst.ID, models.ItemListParams{})
|
||||
for _, it := range items {
|
||||
if it.Title != item.Title {
|
||||
continue
|
||||
}
|
||||
rows, err := s.ListYjsSetAside(it.ID)
|
||||
if err != nil || len(rows) != 1 {
|
||||
t.Fatalf("imported rows = %d (err %v), want only the 1 well-formed row", len(rows), err)
|
||||
}
|
||||
return
|
||||
}
|
||||
t.Fatal("imported item not found")
|
||||
}
|
||||
|
||||
// The migrate-to-pg gate asks about the OP-LOG only. Set-aside rows travel in
|
||||
// the bundle, so an item holding only those is not refused; an item holding
|
||||
// both still is, although its content_state reads superseded_set_aside.
|
||||
func TestMigrationGateAsksTheOpLogOnly(t *testing.T) {
|
||||
eachBackend(t, func(t *testing.T, s *store.Store) {
|
||||
wsID, _, item := seedStaleItem(t, s)
|
||||
setAsideOne(t, s, item.ID)
|
||||
|
||||
pending, err := s.ListItemsPendingContentFlush(wsID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(pending) != 0 {
|
||||
t.Fatalf("gate names %+v for an item holding only set-aside rows", pending)
|
||||
}
|
||||
|
||||
if _, err := s.AppendYjsUpdate(item.ID, []byte{0x00, 0x02, 0x03, 0x0A, 0x0B, 0x0C}, "2"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
got, _ := s.GetItem(item.ID)
|
||||
if got.ContentState != models.ContentStateSetAside {
|
||||
t.Fatalf("premise: an item holding both reads %q, want %q", got.ContentState, models.ContentStateSetAside)
|
||||
}
|
||||
pending, err = s.ListItemsPendingContentFlush(wsID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(pending) != 1 {
|
||||
t.Fatalf("gate names %d item(s) for an item holding set-aside AND pending rows, want 1", len(pending))
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -24,12 +24,23 @@ import (
|
||||
// them is not what it asked for. Re-reading does not clear it — the row, and
|
||||
// its seq, are unchanged until a tab flushes — so it is its own error rather
|
||||
// than an UpdateConflictError, which callers answer by re-reading and retrying.
|
||||
//
|
||||
// It also refuses ANY content write, token or not, while the item holds rows a
|
||||
// collab schema-version rebuild set aside (BUG-3244, SetAsideRows): those edits
|
||||
// are in no body and no tab can restore them, so replacing the body without
|
||||
// saying so would lose them for good. Only overwrite_pending_edits discards
|
||||
// them.
|
||||
type ContentPendingFlushError struct {
|
||||
ItemID string
|
||||
PendingRows int
|
||||
ItemID string
|
||||
PendingRows int
|
||||
SetAsideRows int
|
||||
}
|
||||
|
||||
func (e *ContentPendingFlushError) Error() string {
|
||||
if e.SetAsideRows > 0 {
|
||||
return fmt.Sprintf("item %s has %d set-aside edit row(s) from an earlier editor schema and %d unflushed collaborative edit row(s); refusing a content write that would replace them",
|
||||
e.ItemID, e.SetAsideRows, e.PendingRows)
|
||||
}
|
||||
return fmt.Sprintf("item %s has %d unflushed collaborative edit row(s); refusing a content write guarded by a version token",
|
||||
e.ItemID, e.PendingRows)
|
||||
}
|
||||
|
||||
@@ -251,6 +251,37 @@ func (s *Store) ExportWorkspaceQ(q Queryer, slug string) (*models.WorkspaceExpor
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Set-aside edits (BUG-3244), attached to their items. Read through the
|
||||
// same executor as the items, so on the migration's transaction a row and
|
||||
// the content_state that names it come from one snapshot.
|
||||
itemIndex := make(map[string]int, len(export.Items))
|
||||
for i := range export.Items {
|
||||
itemIndex[export.Items[i].ID] = i
|
||||
}
|
||||
setAsideRows, err := q.Query(s.q(`
|
||||
SELECT sa.item_id, sa.op_log_id, sa.update_data, sa.schema_version, sa.created_at, sa.set_aside_at
|
||||
FROM item_yjs_updates_set_aside sa
|
||||
JOIN items i ON i.id = sa.item_id
|
||||
WHERE i.workspace_id = ? AND i.deleted_at IS NULL
|
||||
ORDER BY sa.item_id, sa.op_log_id, sa.id`), ws.ID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("export set-aside edits: %w", err)
|
||||
}
|
||||
defer setAsideRows.Close()
|
||||
for setAsideRows.Next() {
|
||||
var itemID string
|
||||
var r models.YjsSetAsideExport
|
||||
if err := setAsideRows.Scan(&itemID, &r.OpLogID, &r.UpdateData, &r.SchemaVersion, &r.CreatedAt, &r.SetAsideAt); err != nil {
|
||||
return nil, fmt.Errorf("scan set-aside edit: %w", err)
|
||||
}
|
||||
if i, ok := itemIndex[itemID]; ok {
|
||||
export.Items[i].CollabSetAside = append(export.Items[i].CollabSetAside, r)
|
||||
}
|
||||
}
|
||||
if err := setAsideRows.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Comments
|
||||
commentRows, err := q.Query(s.q(`
|
||||
SELECT c.id, c.item_id, c.author, c.body, c.created_by, c.source, c.created_at, c.updated_at
|
||||
@@ -1236,6 +1267,39 @@ func (s *Store) importWorkspace(data *models.WorkspaceExport, newName string, ow
|
||||
}
|
||||
}
|
||||
|
||||
// Import set-aside edits (BUG-3244), so the imported item keeps them and
|
||||
// reads superseded_set_aside as its source did. Every value is checked
|
||||
// BEFORE the insert rather than skipped on a failed one: on Postgres a
|
||||
// failed statement poisons the transaction and loses the whole import (the
|
||||
// BUG-2884 lesson recorded on the reminders loop above). A row that fails
|
||||
// the check is skipped with a warning, matching this file's lenient
|
||||
// precedent; a bundle this server wrote never fails it.
|
||||
for _, it := range data.Items {
|
||||
if len(it.CollabSetAside) == 0 {
|
||||
continue
|
||||
}
|
||||
newItemID := itemMap[it.ID]
|
||||
if !insertedItems[newItemID] {
|
||||
continue
|
||||
}
|
||||
for _, sa := range it.CollabSetAside {
|
||||
createdAt, cerr := normalizeImportedInstant(sa.CreatedAt)
|
||||
setAsideAt, serr := normalizeImportedInstant(sa.SetAsideAt)
|
||||
if len(sa.UpdateData) == 0 || !importSchemaVersionRe.MatchString(sa.SchemaVersion) || cerr != nil || serr != nil {
|
||||
slog.Warn("workspace import: skipping a malformed set-aside edit row",
|
||||
"workspace_id", ws.ID, "item_id", newItemID, "op_log_id", sa.OpLogID)
|
||||
continue
|
||||
}
|
||||
if _, err := tx.Exec(s.q(`
|
||||
INSERT INTO item_yjs_updates_set_aside
|
||||
(item_id, op_log_id, update_data, schema_version, created_at, set_aside_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?)`),
|
||||
newItemID, sa.OpLogID, sa.UpdateData, sa.SchemaVersion, createdAt, setAsideAt); err != nil {
|
||||
return nil, fmt.Errorf("import set-aside edit: %w", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Import item versions
|
||||
for _, ver := range data.ItemVersions {
|
||||
newItemID := itemMap[ver.ItemID]
|
||||
@@ -1449,6 +1513,16 @@ type PendingFlushItem struct {
|
||||
// hand-written spellings of it would be two chances to drift, and a gate that
|
||||
// refused a different set than the bundle marks would be worse than no gate.
|
||||
//
|
||||
// It asks about the OP-LOG half only (pendingFlushExistsSQLFor), not about
|
||||
// set-aside rows (BUG-3244). Those travel in the bundle
|
||||
// (ItemExport.CollabSetAside), so migrating an item that holds them loses
|
||||
// nothing, and refusing it would send the operator to open a tab that cannot
|
||||
// recover them. The op-log half is asked on its own because contentStateSQL
|
||||
// reports set-aside FIRST: an item holding both would read superseded_set_aside
|
||||
// and hide the pending rows this gate exists for. So the gate refuses a
|
||||
// superset of the items the bundle marks applied_pending_flush, and it runs
|
||||
// first, on the same snapshot.
|
||||
//
|
||||
// Why a query rather than a scan of an already-built bundle: the migration's
|
||||
// refusal has to be able to say "nothing has been migrated", which means the
|
||||
// check must cover EVERY workspace before the first import runs. Exporting them
|
||||
@@ -1494,7 +1568,7 @@ func (s *Store) ListItemsPendingContentFlushQ(q Queryer, workspaceID string) ([]
|
||||
FROM items i
|
||||
LEFT JOIN collections c ON c.id = i.collection_id
|
||||
WHERE i.workspace_id = ? AND i.deleted_at IS NULL
|
||||
AND `+contentStateSQL+` <> ''
|
||||
AND `+pendingFlushExistsSQLFor("i")+`
|
||||
ORDER BY COALESCE(c.prefix, ''), i.item_number`), workspaceID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list items pending content flush: %w", err)
|
||||
|
||||
+23
-2
@@ -741,12 +741,33 @@ var contentStateSQL = contentStateSQLFor("i")
|
||||
// predicate would be two chances to drift, and a predicate that disagreed with
|
||||
// itself between doors is the shape of the defect this whole family is about.
|
||||
func contentStateSQLFor(alias string) string {
|
||||
return `CASE WHEN EXISTS (
|
||||
// Set-aside rows first (BUG-3244): when an item has both, the edits no tab
|
||||
// can recover are the claim a reader must not miss, and set-aside rows do
|
||||
// not depend on the watermark, so a tab stamping it cannot clear them.
|
||||
return `CASE WHEN ` + setAsideExistsSQLFor(alias) +
|
||||
` THEN 'superseded_set_aside' WHEN ` + pendingFlushExistsSQLFor(alias) +
|
||||
` THEN 'applied_pending_flush' ELSE '' END`
|
||||
}
|
||||
|
||||
// pendingFlushExistsSQLFor is the BUG-3000 half of contentStateSQLFor on its
|
||||
// own: unflushed content-bearing op-log rows. A door that must ask about the
|
||||
// op-log alone uses it, because contentStateSQLFor reports set-aside first and
|
||||
// so cannot say whether an item holding both ALSO has op-log rows pending.
|
||||
func pendingFlushExistsSQLFor(alias string) string {
|
||||
return `EXISTS (
|
||||
SELECT 1 FROM item_yjs_updates u
|
||||
WHERE u.item_id = ` + alias + `.id
|
||||
AND u.id > COALESCE(` + alias + `.content_flushed_op_log_id, 0)
|
||||
AND u.content_bearing = TRUE
|
||||
) THEN 'applied_pending_flush' ELSE '' END`
|
||||
)`
|
||||
}
|
||||
|
||||
// setAsideExistsSQLFor is the BUG-3244 half: rows a schema rebuild set aside.
|
||||
func setAsideExistsSQLFor(alias string) string {
|
||||
return `EXISTS (
|
||||
SELECT 1 FROM item_yjs_updates_set_aside sa
|
||||
WHERE sa.item_id = ` + alias + `.id
|
||||
)`
|
||||
}
|
||||
|
||||
// getItemScanQ is the one item-row scan behind GetItem, getItemTx and
|
||||
|
||||
@@ -120,6 +120,9 @@ var migratedRefusalTables = []string{
|
||||
// has not been flushed to items.content, and it is the one BUG-3032 could
|
||||
// detect but not save.
|
||||
"item_yjs_updates",
|
||||
// Edits a schema rebuild set aside (BUG-3244): the only copy of edits no
|
||||
// body holds, the same claim the op-log makes above.
|
||||
"item_yjs_updates_set_aside",
|
||||
|
||||
// Carried by models.WorkspaceExport: these rows now live in PostgreSQL, so
|
||||
// a write here edits a copy that is gone.
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
-- Migration 097: keep the unflushed edits a schema-version rebuild used to delete (BUG-3244).
|
||||
--
|
||||
-- A collab join at a new editor schema version rebuilds the item's op-log
|
||||
-- (collab.RoomManager.maybeRebuildOnSchemaMismatch): rows written under the old
|
||||
-- schema cannot replay into a new-schema document, so they leave the op-log.
|
||||
-- Before this table they were DELETED, including the content-bearing rows above
|
||||
-- items.content_flushed_op_log_id, which are edits items.content never
|
||||
-- received. content_state then read clean, so the stale body looked current.
|
||||
--
|
||||
-- Those rows are now MOVED here first. Nothing replays them: this table is not
|
||||
-- item_yjs_updates, so no op-log reader (replay, cursors, GC, applier, watermark
|
||||
-- stamp) can see them. They are kept for recovery (TASK-3246), are readable as
|
||||
-- raw updates, and hold the item's content_state at superseded_set_aside until
|
||||
-- someone recovers or explicitly discards them.
|
||||
--
|
||||
-- op_log_id the row's id in item_yjs_updates before the move, so the
|
||||
-- original order and provenance survive.
|
||||
-- update_data the frame, byte for byte (BLOB/BYTEA, like the op-log's).
|
||||
-- schema_version the era the frame was written under; the decoder that
|
||||
-- recovers it is chosen by this.
|
||||
-- created_at when the frame was first persisted.
|
||||
-- set_aside_at when the rebuild moved it.
|
||||
|
||||
CREATE TABLE IF NOT EXISTS item_yjs_updates_set_aside (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
item_id TEXT NOT NULL REFERENCES items(id) ON DELETE CASCADE,
|
||||
op_log_id INTEGER NOT NULL,
|
||||
update_data BLOB NOT NULL,
|
||||
schema_version TEXT NOT NULL,
|
||||
created_at TEXT NOT NULL,
|
||||
set_aside_at TEXT NOT NULL
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_yjs_set_aside_item_id
|
||||
ON item_yjs_updates_set_aside(item_id, id);
|
||||
@@ -146,6 +146,10 @@ item_yjs_updates.content_hash
|
||||
item_yjs_updates.created_at
|
||||
item_yjs_updates.item_id
|
||||
item_yjs_updates.schema_version
|
||||
item_yjs_updates_set_aside.created_at
|
||||
item_yjs_updates_set_aside.item_id
|
||||
item_yjs_updates_set_aside.schema_version
|
||||
item_yjs_updates_set_aside.set_aside_at
|
||||
items.agent_role_id
|
||||
items.assigned_user_id
|
||||
items.collection_id
|
||||
|
||||
@@ -366,19 +366,20 @@ func NULProtectedColumns() []nulColumn {
|
||||
// on an unlisted column are to fail on every known exclusion or to ignore the
|
||||
// class entirely.
|
||||
var nulExcluded = map[string]string{
|
||||
"activities.action": "fixed enum, models.ValidActions",
|
||||
"event_outbox.last_error": "Go error string, server-composed",
|
||||
"mcp_audit_log.tool_name": "server enum, mcp_audit.go",
|
||||
"mcp_audit_log.error_kind": "server enum, mcp_audit.go",
|
||||
"mcp_audit_log.tool_name_source": "server enum (models.MCPToolNameSource), CHECK-constrained by migration 096; never request-derived",
|
||||
"attachments.filename_source": "server enum (attachments.FilenameSource), CHECK-constrained by migration 096; a bundle import may carry the bundle's value, but only after attachments.ValidFilenameSource accepts it",
|
||||
"users.recovery_codes": "newline-joined bcrypt hashes of server-generated codes; looks like JSON, is not",
|
||||
"workspace_members.collection_access": "validated enum all/selected",
|
||||
"decision_jobs.claimed_by": "runner id minted by the server's decision tick, never request-derived",
|
||||
"decision_jobs.last_error": "Go error string, server-composed",
|
||||
"item_decisions.answer": "json.Marshal of decision.Answer: a NUL anywhere in it is written as the six-byte escape, never a raw byte, and the column is TEXT on both dialects so no jsonb parser decodes it (migration 090)",
|
||||
"item_relation_links.source_field_key": "only written by replaceRelationLinks, as a field definition's key value read from collections.schema in the same transaction; that column's classJSON trigger checks every decoded value; migration 088 states the derivation (BUG-3108 writer list)",
|
||||
"item_yjs_updates.update_data": "BINARY (BLOB/BYTEA), the only such column in either schema. Raw Yjs updates legitimately contain NUL bytes; Layer A exempts it for the same reason and TestBinaryColumnCensus pins that. Surfaced here when the census's type filter was widened to include BLOB affinity, which is correct — the decision to exclude it is a judgement, not an oversight.",
|
||||
"activities.action": "fixed enum, models.ValidActions",
|
||||
"event_outbox.last_error": "Go error string, server-composed",
|
||||
"mcp_audit_log.tool_name": "server enum, mcp_audit.go",
|
||||
"mcp_audit_log.error_kind": "server enum, mcp_audit.go",
|
||||
"mcp_audit_log.tool_name_source": "server enum (models.MCPToolNameSource), CHECK-constrained by migration 096; never request-derived",
|
||||
"attachments.filename_source": "server enum (attachments.FilenameSource), CHECK-constrained by migration 096; a bundle import may carry the bundle's value, but only after attachments.ValidFilenameSource accepts it",
|
||||
"users.recovery_codes": "newline-joined bcrypt hashes of server-generated codes; looks like JSON, is not",
|
||||
"workspace_members.collection_access": "validated enum all/selected",
|
||||
"decision_jobs.claimed_by": "runner id minted by the server's decision tick, never request-derived",
|
||||
"decision_jobs.last_error": "Go error string, server-composed",
|
||||
"item_decisions.answer": "json.Marshal of decision.Answer: a NUL anywhere in it is written as the six-byte escape, never a raw byte, and the column is TEXT on both dialects so no jsonb parser decodes it (migration 090)",
|
||||
"item_relation_links.source_field_key": "only written by replaceRelationLinks, as a field definition's key value read from collections.schema in the same transaction; that column's classJSON trigger checks every decoded value; migration 088 states the derivation (BUG-3108 writer list)",
|
||||
"item_yjs_updates_set_aside.update_data": "BINARY (BLOB/BYTEA): op-log frames a schema rebuild moved here verbatim (BUG-3244), so the same raw Yjs bytes as item_yjs_updates.update_data, NULs included, and exempt for the same reason. A bundle import writes it only after base64-decoding the bundle's bytes.",
|
||||
"item_yjs_updates.update_data": "BINARY (BLOB/BYTEA), one of two such columns in either schema (the other is its set-aside twin, BUG-3244). Raw Yjs updates legitimately contain NUL bytes; Layer A exempts it for the same reason and TestBinaryColumnCensus pins that. Surfaced here when the census's type filter was widened to include BLOB affinity, which is correct — the decision to exclude it is a judgement, not an oversight.",
|
||||
}
|
||||
|
||||
// ensureNULTriggers re-applies the applied Layer B trigger migrations if any of
|
||||
|
||||
@@ -116,8 +116,9 @@ func (e *InvalidTextParameterError) Unwrap() error { return ErrInvalidTextParame
|
||||
//
|
||||
// BINARY is exempt, and that is measured rather than assumed. The invariant is
|
||||
// about text and JSON columns; in this store []byte binds BINARY, and
|
||||
// item_yjs_updates.update_data — the only BLOB/BYTEA column in either schema —
|
||||
// legitimately contains NUL bytes. The first version of this guard checked
|
||||
// item_yjs_updates.update_data and its set-aside twin (BUG-3244) — the only
|
||||
// BLOB/BYTEA columns in either schema, both raw Yjs frames —
|
||||
// legitimately contain NUL bytes. The first version of this guard checked
|
||||
// []byte and refused every Yjs op-log append; the existing collab suite caught
|
||||
// it immediately. TestBinaryColumnCensus fails when a new binary column
|
||||
// appears, which is when that exemption must be re-examined.
|
||||
|
||||
@@ -32,7 +32,9 @@ import (
|
||||
// then add it below.
|
||||
func TestBinaryColumnCensus(t *testing.T) {
|
||||
t.Parallel()
|
||||
want := map[string]bool{"item_yjs_updates.update_data": true}
|
||||
// item_yjs_updates_set_aside.update_data holds op-log frames moved verbatim
|
||||
// by a schema rebuild (BUG-3244): the same raw Yjs bytes, NULs included.
|
||||
want := map[string]bool{"item_yjs_updates.update_data": true, "item_yjs_updates_set_aside.update_data": true}
|
||||
|
||||
// One pattern per dialect spelling, applied to the raw migration text so
|
||||
// nothing depends on a Go-side model of the schema.
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
-- Migration 072 (Postgres twin of SQLite 097): keep the unflushed edits a schema-version rebuild used to delete (BUG-3244).
|
||||
--
|
||||
-- A collab join at a new editor schema version rebuilds the item's op-log
|
||||
-- (collab.RoomManager.maybeRebuildOnSchemaMismatch): rows written under the old
|
||||
-- schema cannot replay into a new-schema document, so they leave the op-log.
|
||||
-- Before this table they were DELETED, including the content-bearing rows above
|
||||
-- items.content_flushed_op_log_id, which are edits items.content never
|
||||
-- received. content_state then read clean, so the stale body looked current.
|
||||
--
|
||||
-- Those rows are now MOVED here first. Nothing replays them: this table is not
|
||||
-- item_yjs_updates, so no op-log reader (replay, cursors, GC, applier, watermark
|
||||
-- stamp) can see them. They are kept for recovery (TASK-3246), are readable as
|
||||
-- raw updates, and hold the item's content_state at superseded_set_aside until
|
||||
-- someone recovers or explicitly discards them.
|
||||
--
|
||||
-- op_log_id the row's id in item_yjs_updates before the move, so the
|
||||
-- original order and provenance survive.
|
||||
-- update_data the frame, byte for byte (BLOB/BYTEA, like the op-log's).
|
||||
-- schema_version the era the frame was written under; the decoder that
|
||||
-- recovers it is chosen by this.
|
||||
-- created_at when the frame was first persisted.
|
||||
-- set_aside_at when the rebuild moved it.
|
||||
|
||||
CREATE TABLE IF NOT EXISTS item_yjs_updates_set_aside (
|
||||
id BIGSERIAL PRIMARY KEY,
|
||||
item_id TEXT NOT NULL REFERENCES items(id) ON DELETE CASCADE,
|
||||
op_log_id BIGINT NOT NULL,
|
||||
update_data BYTEA NOT NULL,
|
||||
schema_version TEXT NOT NULL,
|
||||
created_at TEXT NOT NULL,
|
||||
set_aside_at TEXT NOT NULL
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_yjs_set_aside_item_id
|
||||
ON item_yjs_updates_set_aside(item_id, id);
|
||||
@@ -0,0 +1,240 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"sort"
|
||||
"time"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3244: edits a collab schema-version rebuild takes out of the op-log.
|
||||
//
|
||||
// A join at a new editor schema version cannot replay rows written under the
|
||||
// old one, so the rebuild empties the item's op-log. The content-bearing rows
|
||||
// above the flush watermark are edits items.content never received; they are
|
||||
// MOVED to item_yjs_updates_set_aside first (migration 097 / pg 072), where no
|
||||
// op-log reader sees them, and they hold content_state at
|
||||
// models.ContentStateSetAside until they are recovered or explicitly
|
||||
// discarded. Rows at or below the watermark are already in items.content, and
|
||||
// non-content-bearing rows cannot change the document (BUG-3124), so neither
|
||||
// is kept.
|
||||
|
||||
// setAsideBeforeClearHook, when set by a test, runs inside the transaction
|
||||
// before the DELETE: the gap a concurrent append would land in.
|
||||
var setAsideBeforeClearHook func()
|
||||
|
||||
// SetAsideAndClearOpLog moves the item's unflushed content-bearing op-log rows
|
||||
// to the set-aside table and deletes its whole op-log, in one transaction, so
|
||||
// a failure leaves the op-log untouched rather than half moved. It returns how
|
||||
// many rows were set aside and how many op-log rows were deleted.
|
||||
//
|
||||
// The rows set aside are chosen from the rows the DELETE itself returned, not
|
||||
// by a separate read before it. The setup lock the rebuild holds does not stop
|
||||
// a connected peer's readLoop appending, and on Postgres (READ COMMITTED) each
|
||||
// statement takes its own snapshot, so a row committed between a copy and a
|
||||
// delete would be deleted without being considered.
|
||||
func (s *Store) SetAsideAndClearOpLog(itemID string) (setAside, cleared int64, err error) {
|
||||
if itemID == "" {
|
||||
return 0, 0, errors.New("SetAsideAndClearOpLog: itemID is required")
|
||||
}
|
||||
tx, err := s.db.Begin()
|
||||
if err != nil {
|
||||
return 0, 0, fmt.Errorf("set aside op-log (begin): %w", err)
|
||||
}
|
||||
defer func() {
|
||||
if err != nil {
|
||||
_ = tx.Rollback()
|
||||
}
|
||||
}()
|
||||
|
||||
if setAsideBeforeClearHook != nil {
|
||||
setAsideBeforeClearHook()
|
||||
}
|
||||
|
||||
// The DELETE is the transaction's first statement, as the INSERT it
|
||||
// replaces was: on SQLite a read first would take a snapshot that a
|
||||
// concurrent append invalidates, and the write that follows would then
|
||||
// fail with SQLITE_BUSY_SNAPSHOT instead of waiting.
|
||||
type deletedRow struct {
|
||||
id int64
|
||||
data []byte
|
||||
schemaVersion string
|
||||
createdAt any
|
||||
contentBearing bool
|
||||
}
|
||||
rows, err := tx.Query(s.dialect.Rebind(`
|
||||
DELETE FROM item_yjs_updates WHERE item_id = ?
|
||||
RETURNING id, update_data, schema_version, created_at, content_bearing`), itemID)
|
||||
if err != nil {
|
||||
return 0, 0, fmt.Errorf("set aside op-log (clear): %w", err)
|
||||
}
|
||||
var deleted []deletedRow
|
||||
for rows.Next() {
|
||||
var r deletedRow
|
||||
if err = rows.Scan(&r.id, &r.data, &r.schemaVersion, &r.createdAt, &r.contentBearing); err != nil {
|
||||
rows.Close()
|
||||
return 0, 0, fmt.Errorf("set aside op-log (clear scan): %w", err)
|
||||
}
|
||||
deleted = append(deleted, r)
|
||||
}
|
||||
if err = rows.Err(); err != nil {
|
||||
rows.Close()
|
||||
return 0, 0, fmt.Errorf("set aside op-log (clear rows): %w", err)
|
||||
}
|
||||
rows.Close()
|
||||
cleared = int64(len(deleted))
|
||||
|
||||
// Read after the delete. A flush that has advanced it covers rows that are
|
||||
// in items.content, so a later value is only more accurate.
|
||||
var watermark int64
|
||||
if err = tx.QueryRow(s.dialect.Rebind(
|
||||
`SELECT COALESCE(content_flushed_op_log_id, 0) FROM items WHERE id = ?`), itemID).Scan(&watermark); err != nil && !errors.Is(err, sql.ErrNoRows) {
|
||||
return 0, 0, fmt.Errorf("set aside op-log (watermark): %w", err)
|
||||
}
|
||||
err = nil
|
||||
var keep []deletedRow
|
||||
for _, r := range deleted {
|
||||
if r.contentBearing && r.id > watermark {
|
||||
keep = append(keep, r)
|
||||
}
|
||||
}
|
||||
sort.Slice(keep, func(i, j int) bool { return keep[i].id < keep[j].id })
|
||||
|
||||
now := time.Now().UTC().Format(time.RFC3339)
|
||||
for _, r := range keep {
|
||||
if _, err = tx.Exec(s.dialect.Rebind(`
|
||||
INSERT INTO item_yjs_updates_set_aside
|
||||
(item_id, op_log_id, update_data, schema_version, created_at, set_aside_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?)`), itemID, r.id, r.data, r.schemaVersion, r.createdAt, now); err != nil {
|
||||
return 0, 0, fmt.Errorf("set aside op-log (copy): %w", err)
|
||||
}
|
||||
setAside++
|
||||
}
|
||||
if err = tx.Commit(); err != nil {
|
||||
return 0, 0, fmt.Errorf("set aside op-log (commit): %w", err)
|
||||
}
|
||||
return setAside, cleared, nil
|
||||
}
|
||||
|
||||
// ListYjsSetAside returns the item's set-aside rows in their original op-log
|
||||
// order, raw bytes included. Empty when there are none.
|
||||
func (s *Store) ListYjsSetAside(itemID string) ([]models.YjsSetAside, error) {
|
||||
if itemID == "" {
|
||||
return nil, errors.New("ListYjsSetAside: itemID is required")
|
||||
}
|
||||
rows, err := s.db.Query(s.dialect.Rebind(`
|
||||
SELECT id, item_id, op_log_id, update_data, schema_version, created_at, set_aside_at
|
||||
FROM item_yjs_updates_set_aside
|
||||
WHERE item_id = ?
|
||||
ORDER BY op_log_id ASC, id ASC`), itemID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list set-aside updates: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
out := []models.YjsSetAside{}
|
||||
for rows.Next() {
|
||||
var (
|
||||
r models.YjsSetAside
|
||||
createdAt, movedAt string
|
||||
)
|
||||
if err := rows.Scan(&r.ID, &r.ItemID, &r.OpLogID, &r.UpdateData, &r.SchemaVersion, &createdAt, &movedAt); err != nil {
|
||||
return nil, fmt.Errorf("scan set-aside update: %w", err)
|
||||
}
|
||||
r.CreatedAt = parseOpLogTime(createdAt)
|
||||
r.SetAsideAt = parseOpLogTime(movedAt)
|
||||
out = append(out, r)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, fmt.Errorf("iterate set-aside updates: %w", err)
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// CountYjsSetAsideTx counts the item's set-aside rows inside tx.
|
||||
func (s *Store) CountYjsSetAsideTx(tx *sql.Tx, itemID string) (int, error) {
|
||||
if tx == nil {
|
||||
return 0, errors.New("CountYjsSetAsideTx: tx is required")
|
||||
}
|
||||
var n int
|
||||
if err := tx.QueryRow(s.dialect.Rebind(
|
||||
`SELECT COUNT(*) FROM item_yjs_updates_set_aside WHERE item_id = ?`), itemID).Scan(&n); err != nil {
|
||||
return 0, fmt.Errorf("count set-aside updates: %w", err)
|
||||
}
|
||||
return n, nil
|
||||
}
|
||||
|
||||
// DeleteYjsSetAsideTx discards the item's set-aside rows inside tx and returns
|
||||
// how many it deleted. Only an explicit discard may call it: the rows are the
|
||||
// only copy of those edits.
|
||||
func (s *Store) DeleteYjsSetAsideTx(tx *sql.Tx, itemID string) (int64, error) {
|
||||
if tx == nil {
|
||||
return 0, errors.New("DeleteYjsSetAsideTx: tx is required")
|
||||
}
|
||||
res, err := tx.Exec(s.dialect.Rebind(`DELETE FROM item_yjs_updates_set_aside WHERE item_id = ?`), itemID)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("discard set-aside updates: %w", err)
|
||||
}
|
||||
n, err := res.RowsAffected()
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("discard set-aside updates (count): %w", err)
|
||||
}
|
||||
return n, nil
|
||||
}
|
||||
|
||||
// DiscardYjsSetAside is DeleteYjsSetAsideTx in its own transaction, for the
|
||||
// explicit discard door, which writes nothing else.
|
||||
func (s *Store) DiscardYjsSetAside(itemID string) (int64, error) {
|
||||
if itemID == "" {
|
||||
return 0, errors.New("DiscardYjsSetAside: itemID is required")
|
||||
}
|
||||
tx, err := s.db.Begin()
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("discard set-aside updates (begin): %w", err)
|
||||
}
|
||||
n, err := s.DeleteYjsSetAsideTx(tx, itemID)
|
||||
if err != nil {
|
||||
_ = tx.Rollback()
|
||||
return 0, err
|
||||
}
|
||||
if err := tx.Commit(); err != nil {
|
||||
return 0, fmt.Errorf("discard set-aside updates (commit): %w", err)
|
||||
}
|
||||
return n, nil
|
||||
}
|
||||
|
||||
// importSchemaVersionRe is what a bundle's set-aside schema_version must look
|
||||
// like to be imported (BUG-3244). The server stamps short opaque tags ("1",
|
||||
// "2-rc1"); anything else did not come from a server and is not stored.
|
||||
var importSchemaVersionRe = regexp.MustCompile(`^[A-Za-z0-9._-]{1,32}$`)
|
||||
|
||||
// normalizeImportedInstant parses a bundle timestamp in either form the
|
||||
// op-log is written in and returns it as the RFC3339 UTC the store writes, so
|
||||
// an imported row is indistinguishable from a native one.
|
||||
func normalizeImportedInstant(v string) (string, error) {
|
||||
if t, err := time.Parse(time.RFC3339, v); err == nil {
|
||||
return t.UTC().Format(time.RFC3339), nil
|
||||
}
|
||||
t, err := time.Parse("2006-01-02 15:04:05", v)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("not an instant: %q", v)
|
||||
}
|
||||
return t.UTC().Format(time.RFC3339), nil
|
||||
}
|
||||
|
||||
// parseOpLogTime reads an op-log timestamp: RFC3339 as written, or SQLite's
|
||||
// CURRENT_TIMESTAMP form a fixture may use (the same tolerance
|
||||
// LoadYjsUpdatesSince applies).
|
||||
func parseOpLogTime(v string) time.Time {
|
||||
if t, err := time.Parse(time.RFC3339, v); err == nil {
|
||||
return t
|
||||
}
|
||||
if t, err := time.Parse("2006-01-02 15:04:05", v); err == nil {
|
||||
return t.UTC()
|
||||
}
|
||||
return time.Time{}
|
||||
}
|
||||
@@ -241,14 +241,12 @@ func (s *Store) LoadYjsUpdatesSince(itemID string, sinceID int64) ([]models.YjsU
|
||||
//
|
||||
// Used by the schema-mismatch rebuild flow (TASK-1268, PLAN-1248):
|
||||
// if the latest persisted version differs from the server's current
|
||||
// SCHEMA_VERSION, the room manager prunes the op-log before
|
||||
// SCHEMA_VERSION, the room manager empties the op-log before
|
||||
// replaying so the new-schema client doesn't replay old-schema ops
|
||||
// that may be incompatible. The id is also returned so the rebuild
|
||||
// can compare against items.content_flushed_op_log_id and log
|
||||
// loudly when the prune is dropping unflushed edits — the user
|
||||
// can't recover them (old-schema ops can't replay in new schema)
|
||||
// but operators should see when this happens. Per Codex review of
|
||||
// TASK-1309 round 4 [P2].
|
||||
// that may be incompatible, setting the unflushed content-bearing
|
||||
// rows aside first (SetAsideAndClearOpLog, BUG-3244). The id is
|
||||
// returned too; the rebuild no longer needs it, since the set-aside
|
||||
// query reads the watermark itself.
|
||||
//
|
||||
// We pick "most recent" rather than "any row" so a server that
|
||||
// wrote some old-version rows then was rolled back, then forward
|
||||
@@ -422,9 +420,10 @@ func (s *Store) ItemRestoreBoundaryOpID(itemID string) (int64, bool, error) {
|
||||
// id known to be reflected in items.content, or (0, false) if the
|
||||
// item has never been flushed (NULL column value or item missing).
|
||||
//
|
||||
// Used by the schema-mismatch rebuild path (TASK-1268) to detect
|
||||
// when an unsafe prune is about to drop unflushed edits. Per Codex
|
||||
// review of TASK-1309 round 4 [P2].
|
||||
// The schema-mismatch rebuild used to read it to warn that its prune
|
||||
// was about to drop unflushed edits (TASK-1309 round 4). Since BUG-3244
|
||||
// the rebuild sets those rows aside instead, in SQL that reads the
|
||||
// watermark itself, so no production path calls this today.
|
||||
func (s *Store) GetItemContentFlushedOpLogID(itemID string) (int64, bool, error) {
|
||||
if itemID == "" {
|
||||
return 0, false, errors.New("GetItemContentFlushedOpLogID: itemID is required")
|
||||
@@ -565,10 +564,10 @@ func (s *Store) PruneItemOpLogIfDormantBefore(itemID string, before time.Time) (
|
||||
// can't be resolved). This method is retained ONLY for callers that
|
||||
// guarantee the suffix doesn't depend on the prefix:
|
||||
//
|
||||
// - The schema-mismatch rebuild path (TASK-1268) calls it with a
|
||||
// far-future cutoff to wipe the entire op-log on a version
|
||||
// change — same effective behaviour as the GC sweeper, just
|
||||
// reached via a different signal.
|
||||
// - The schema-mismatch rebuild path (TASK-1268) used to call it
|
||||
// with a far-future cutoff to wipe the entire op-log on a version
|
||||
// change. Since BUG-3244 it calls SetAsideAndClearOpLog, which
|
||||
// keeps the unflushed rows; tests still wipe op-logs with this.
|
||||
// - PruneAndApply's direct-write fallback (TASK-1257) — the
|
||||
// caller guarantees no live readLoop is appending while the
|
||||
// prune+items.content-write runs.
|
||||
|
||||
@@ -4,12 +4,14 @@
|
||||
* The CollabProvider sends this on every WebSocket connect via a
|
||||
* `?schema_version=...` query parameter. The server (1) rejects the
|
||||
* upgrade outright if the value doesn't match its own
|
||||
* `DefaultSchemaVersion`, and (2) prunes the op-log on first
|
||||
* `DefaultSchemaVersion`, and (2) empties the op-log on first
|
||||
* mismatched-vs-persisted connect so the new client doesn't replay
|
||||
* old-schema ops that may be incompatible.
|
||||
* old-schema ops that may be incompatible, setting unflushed edits
|
||||
* aside first (BUG-3244).
|
||||
*
|
||||
* **Frozen (BUG-3244).** The server's mismatch prune also deletes every
|
||||
* item's unflushed edits, so a bump is refused until that bug is closed:
|
||||
* **Frozen (BUG-3244).** The server's mismatch rebuild sets every item's
|
||||
* unflushed edits aside, where no editor can restore them until TASK-3246
|
||||
* ships a decoder, so a bump is refused until then:
|
||||
* `internal/collab/schema_version_guard_test.go` fails on any change here
|
||||
* or to the server constant. The rule below says when one would be owed.
|
||||
*
|
||||
|
||||
@@ -12,11 +12,31 @@
|
||||
<Modal open={!!active} onclose={() => pendingEditsDialog.keep()} labelledby="pending-edits-title" maxWidth="500px">
|
||||
{#if active}
|
||||
<div class="modal-header">
|
||||
<h2 id="pending-edits-title">Unsaved edits in an open tab</h2>
|
||||
<h2 id="pending-edits-title">{active.reason === 'set_aside' ? 'Edits from an earlier editor version' : 'Unsaved edits in an open tab'}</h2>
|
||||
<button class="close-btn" type="button" onclick={() => pendingEditsDialog.keep()} aria-label="Close, keeping the tab's edits">✕</button>
|
||||
</div>
|
||||
<div class="modal-body">
|
||||
{#if active.kind === 'excerpt'}
|
||||
{#if active.reason === 'set_aside'}
|
||||
<!-- BUG-3244: these edits are in no body and opening the item will
|
||||
not store them, so no branch here offers that. -->
|
||||
{#if active.kind === 'excerpt'}
|
||||
<p>
|
||||
This prompt includes an excerpt of <strong>{active.itemRef}</strong>'s body, which is
|
||||
missing edits an editor upgrade set aside. Opening the item will not restore them.
|
||||
</p>
|
||||
{:else if active.kind === 'duplicate'}
|
||||
<p>
|
||||
<strong>{active.itemRef}</strong> has edits from an earlier editor version that are not in
|
||||
its stored body, and opening it will not restore them. A copy holds the stored body,
|
||||
without them. The original keeps them.
|
||||
</p>
|
||||
{:else}
|
||||
<p>
|
||||
<strong>{active.itemRef}</strong> has edits from an earlier editor version that are not in
|
||||
its stored body, and opening it will not restore them. Saving your body would discard them.
|
||||
</p>
|
||||
{/if}
|
||||
{:else if active.kind === 'excerpt'}
|
||||
<p>
|
||||
This prompt includes an excerpt of <strong>{active.itemRef}</strong>'s body, and the stored
|
||||
body is behind edits in an open tab, so the excerpt may be out of date. Open the item so
|
||||
@@ -37,10 +57,10 @@
|
||||
</div>
|
||||
<div class="modal-footer">
|
||||
<Button variant="secondary" onclick={() => pendingEditsDialog.keep()}>
|
||||
{active.kind === 'save' ? "Keep the tab's edits" : 'Cancel'}
|
||||
{active.kind === 'save' ? (active.reason === 'set_aside' ? 'Keep those edits' : "Keep the tab's edits") : 'Cancel'}
|
||||
</Button>
|
||||
<Button variant="primary" onclick={() => pendingEditsDialog.overwrite()}>
|
||||
{active.kind === 'duplicate' ? 'Copy the stored body' : active.kind === 'excerpt' ? 'Use it anyway' : 'Overwrite them'}
|
||||
{active.kind === 'duplicate' ? 'Copy the stored body' : active.kind === 'excerpt' ? 'Use it anyway' : active.reason === 'set_aside' ? 'Discard them' : 'Overwrite them'}
|
||||
</Button>
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { pendingEditsDialog } from '$lib/stores/pendingEditsDialog.svelte';
|
||||
import { isBodyStale, isSetAside } from '$lib/items/staleBody';
|
||||
import { copyToClipboard } from '$lib/utils/clipboard';
|
||||
import { collapsePushMessage } from '$lib/push/message';
|
||||
import {
|
||||
@@ -367,8 +368,8 @@
|
||||
// BUG-3050 U2: `{content}` is an excerpt of the STORED body. When that
|
||||
// body is behind a tab's unsaved edits, the excerpt is out of date.
|
||||
// Captured here, before any await, like everything above.
|
||||
const staleExcerpt =
|
||||
item?.content_state === 'applied_pending_flush' && action.prompt.includes('{content}');
|
||||
const staleExcerpt = isBodyStale(item) && action.prompt.includes('{content}');
|
||||
const excerptSetAside = isSetAside(item?.content_state);
|
||||
const itemLabel = item ? (formatItemRef(item) ?? item.title) : '';
|
||||
open = false;
|
||||
resetCreateForm();
|
||||
@@ -378,7 +379,9 @@
|
||||
// is reported rather than asked about here.
|
||||
if (staleExcerpt) {
|
||||
toastStore.show(
|
||||
`The copied excerpt of ${itemLabel} may be out of date: it has edits in an open tab that are not stored yet.`,
|
||||
excerptSetAside
|
||||
? `The copied excerpt of ${itemLabel} is missing edits an editor upgrade set aside.`
|
||||
: `The copied excerpt of ${itemLabel} may be out of date: it has edits in an open tab that are not stored yet.`,
|
||||
'info'
|
||||
);
|
||||
}
|
||||
@@ -393,7 +396,7 @@
|
||||
// A push is not tied to the gesture, so a stale excerpt is ASKED about.
|
||||
// Only captured values are used after this await, and the identity
|
||||
// fence is re-checked: the push has no idempotency key.
|
||||
if (staleExcerpt && !(await pendingEditsDialog.request(itemLabel, 'excerpt'))) return;
|
||||
if (staleExcerpt && !(await pendingEditsDialog.request(itemLabel, 'excerpt', excerptSetAside ? 'set_aside' : 'pending'))) return;
|
||||
if (!isSameIdentity()) return;
|
||||
|
||||
dispatching = true;
|
||||
|
||||
@@ -5,7 +5,8 @@
|
||||
import { api, PadApiError, isUpdateConflictError, type ImportURLResponse } from '$lib/api/client';
|
||||
// Its own statement, so units that only use `api` keep their reviewed hash.
|
||||
import { isSupersededWriteError } from '$lib/api/client';
|
||||
import { isContentPendingFlush, prunedEditsNotice } from '$lib/items/contentWrite';
|
||||
import { isContentPendingFlush, pendingEditsReason, prunedEditsNotice } from '$lib/items/contentWrite';
|
||||
import type { PendingEditsReason } from '$lib/stores/pendingEditsDialog.svelte';
|
||||
import { pendingEditsDialog } from '$lib/stores/pendingEditsDialog.svelte';
|
||||
import {
|
||||
keepRefusedRawDraft,
|
||||
@@ -34,7 +35,7 @@
|
||||
import Editor from '$lib/components/editor/Editor.svelte';
|
||||
import StaleBodyNotice from '$lib/components/common/StaleBodyNotice.svelte';
|
||||
import RefusedRawDraftNotice from './RefusedRawDraftNotice.svelte';
|
||||
import { isBodyStale } from '$lib/items/staleBody';
|
||||
import { isBodyStale, isSetAside } from '$lib/items/staleBody';
|
||||
import EditorBubbleMenu from '$lib/components/editor/EditorBubbleMenu.svelte';
|
||||
import EditorLinkPopover from '$lib/components/editor/EditorLinkPopover.svelte';
|
||||
import RawMarkdownEditor from '$lib/components/editor/RawMarkdownEditor.svelte';
|
||||
@@ -4223,7 +4224,7 @@
|
||||
});
|
||||
send(false).catch(async (e) => {
|
||||
if (switchedAway(reqItem, gen) || !isContentPendingFlush(e)) throw e;
|
||||
const overwrite = await askToOverwritePendingEdits(reqItem.id, gen);
|
||||
const overwrite = await askToOverwritePendingEdits(reqItem.id, gen, pendingEditsReason(e));
|
||||
if (!overwrite || switchedAway(reqItem, gen)) return null;
|
||||
// Newer typing since this save was armed has its own debounced
|
||||
// save, which asks again; resending this older text would
|
||||
@@ -4573,7 +4574,9 @@
|
||||
if (isSupersededWriteError(e)) clearRefusedRawDraft(draftUser, reqItemId, markdown);
|
||||
else if (isContentPendingFlush(e)) {
|
||||
toastStore.show(
|
||||
'Your markdown edits were not saved: another tab has edits to this item that are not stored yet. They are kept in this browser, and opening the item offers them back.',
|
||||
pendingEditsReason(e) === 'set_aside'
|
||||
? 'Your markdown edits were not saved: this item has edits an editor upgrade set aside. They are kept in this browser, and opening the item offers them back.'
|
||||
: 'Your markdown edits were not saved: another tab has edits to this item that are not stored yet. They are kept in this browser, and opening the item offers them back.',
|
||||
'error',
|
||||
);
|
||||
}
|
||||
@@ -4634,7 +4637,7 @@
|
||||
// BUG-3230 U0: the text stays pending (and the pane dirty) unless
|
||||
// the user chooses to overwrite, which resends it once.
|
||||
if (isContentPendingFlush(e)) {
|
||||
void askToOverwritePendingEdits(reqItemId, genAtSave).then((ok) => {
|
||||
void askToOverwritePendingEdits(reqItemId, genAtSave, pendingEditsReason(e)).then((ok) => {
|
||||
if (!ok || !item || item.id !== reqItemId || genAtSave !== loadGeneration) return;
|
||||
rawOverwriteArmed = true;
|
||||
if (!rawContentSaver.flushNow()) rawOverwriteArmed = false;
|
||||
@@ -4658,11 +4661,11 @@
|
||||
* edits (BUG-3230 U0). False when a question is already open, when the user
|
||||
* keeps the edits, or when the pane moved to another item or load meanwhile.
|
||||
*/
|
||||
async function askToOverwritePendingEdits(reqItemId: string, gen: number): Promise<boolean> {
|
||||
async function askToOverwritePendingEdits(reqItemId: string, gen: number, reason: PendingEditsReason): Promise<boolean> {
|
||||
if (rawPendingPromptOpen || !item) return false;
|
||||
rawPendingPromptOpen = true;
|
||||
try {
|
||||
const overwrite = await pendingEditsDialog.request(formatItemRef(item) || item.title);
|
||||
const overwrite = await pendingEditsDialog.request(formatItemRef(item) || item.title, 'save', reason);
|
||||
if (!item || item.id !== reqItemId || gen !== loadGeneration) return false;
|
||||
return overwrite;
|
||||
} finally {
|
||||
@@ -4747,7 +4750,7 @@
|
||||
} catch (e) {
|
||||
if (!item || item.id !== reqItemId || gen !== loadGeneration) return;
|
||||
if (!overwrite && isContentPendingFlush(e)) {
|
||||
const answer = await askToOverwritePendingEdits(reqItemId, gen);
|
||||
const answer = await askToOverwritePendingEdits(reqItemId, gen, pendingEditsReason(e));
|
||||
if (!answer || !item || item.id !== reqItemId || gen !== loadGeneration) return;
|
||||
overwrite = true;
|
||||
continue;
|
||||
@@ -4904,7 +4907,7 @@
|
||||
// resends; keeping them leaves this text pending and the
|
||||
// pane in raw mode.
|
||||
if (isContentPendingFlush(e)) {
|
||||
const overwrite = await askToOverwritePendingEdits(reqItemId, genAtFlush);
|
||||
const overwrite = await askToOverwritePendingEdits(reqItemId, genAtFlush, pendingEditsReason(e));
|
||||
if (genAtFlush !== loadGeneration || !item || item.id !== reqItemId) return false;
|
||||
if (overwrite) {
|
||||
overwriteNext = true;
|
||||
@@ -6958,7 +6961,9 @@
|
||||
// instead of seeding it.
|
||||
if (item && rawSeedDecision({ liveNow, lastFlushed: lastFlushedOut, stored: item.content ?? '', contentState: item.content_state }).refuse) {
|
||||
toastStore.show(
|
||||
"Raw mode isn't available yet: this item has edits in an open tab that are not stored, and the raw editor would start from the older body.",
|
||||
isSetAside(item.content_state)
|
||||
? "Raw mode isn't available: this item has edits an editor upgrade set aside, and every raw save would be refused until they are discarded."
|
||||
: "Raw mode isn't available yet: this item has edits in an open tab that are not stored, and the raw editor would start from the older body.",
|
||||
'error'
|
||||
);
|
||||
return;
|
||||
|
||||
@@ -88,7 +88,7 @@ const ASYNC_FUNCTIONS: Record<string, Row> = {
|
||||
jumpToSection: { reviewed: '17407aeb431b', why: 'switches this instance\'s tab and scrolls to an anchor', may: ['document.getElementById', 'document.getElementById(anchorId).scrollIntoView'] },
|
||||
ensureGraphComp: { reviewed: 'c1565cfb8a18', why: 'lazy-loads a component module into this instance', may: ['ItemGraphComp', 'graphLoadError'] },
|
||||
handleCopyRef: { reviewed: 'a5e0a0394fcc', why: 'switchedAway before the copied flag' },
|
||||
loadData: { reviewed: '3ac8a3aef331', why: 'IS the load: myGen against loadGeneration after every await; the BUG-3198 re-read of a changed item runs after the install, keyed on the installed item id, and fences itself on itemGen' },
|
||||
loadData: { reviewed: 'e3ff5fea0461', why: 'IS the load: myGen against loadGeneration after every await; the BUG-3198 re-read of a changed item runs after the install, keyed on the installed item id, and fences itself on itemGen' },
|
||||
startEditTitle: { reviewed: '17f04352d420', why: 'focuses and sizes the input it opened synchronously', may: ['el', 'titleInputEl.focus', 'titleInputEl.setSelectionRange'] },
|
||||
// Re-reviewed for BUG-2836: saveTitle's own lines are unchanged (the diff
|
||||
// touches only a $derived beside titleDraft and the textarea's aria
|
||||
@@ -137,7 +137,7 @@ const ASYNC_FUNCTIONS: Record<string, Row> = {
|
||||
updateAssignedUser: { reviewed: '5e4128e45ae7', why: 'gen against loadGeneration on both arms' + SETTLE_NOTE, may: ['saves.settle'] },
|
||||
updateAgentRole: { reviewed: '9b0a6af92cc2', why: 'gen against loadGeneration on both arms' + SETTLE_NOTE, may: ['saves.settle'] },
|
||||
flushRawIfPending: {
|
||||
reviewed: '9030c207f9eb',
|
||||
reviewed: '623e9aa4f426',
|
||||
why: 'genAtFlush against loadGeneration after each PATCH; the re-entrancy waiter returns state; the finally clears this drain\'s own in-flight flag' + SETTLE_NOTE,
|
||||
may: ['saves.settle', 'rawFlushInFlight'],
|
||||
bareAwaits: ['await new Promise((r) => setTimeout(r, 50));'],
|
||||
@@ -167,13 +167,13 @@ const ASYNC_FUNCTIONS: Record<string, Row> = {
|
||||
// BUG-3230 U0: the pending-edits question for the pane's raw and fallback
|
||||
// saves, and the recovery offer for markdown an unload save could not store.
|
||||
askToOverwritePendingEdits: {
|
||||
reviewed: '17ca7bfd9b4d',
|
||||
reviewed: '4163d3fdc1c1',
|
||||
why: 'item id and gen against loadGeneration after the dialog, before answering; the finally clears this pane\'s own one-question-at-a-time flag',
|
||||
may: ['rawPendingPromptOpen'],
|
||||
},
|
||||
copyRefusedDraft: { reviewed: '839b51b59798', why: 'the copied text was captured before the await; the toast after it describes that copy, whichever item is shown', may: ['toastStore.show'] },
|
||||
restoreRefusedDraft: {
|
||||
reviewed: 'd42be5def01e',
|
||||
reviewed: 'c6fcb77cf248',
|
||||
why: 'item id and gen against loadGeneration after every await; the finally clears this pane\'s own busy flag',
|
||||
may: ['refusedDraftBusy'],
|
||||
},
|
||||
@@ -193,7 +193,7 @@ const NESTED: SignedRow[] = [
|
||||
may: ['saves.settle'],
|
||||
},
|
||||
// BUG-3230 U0: the legacy content save's refusal handler.
|
||||
{ body: /askToOverwritePendingEdits\(reqItem\.id, gen\)/, why: 'fallback refusal: switchedAway before the question, and again after it before the resend', reviewed: '272399d98b0e' },
|
||||
{ body: /askToOverwritePendingEdits\(reqItem\.id, gen, pendingEditsReason\(e\)\)/, why: 'fallback refusal: switchedAway before the question, and again after it before the resend', reviewed: 'a6be81eb257a' },
|
||||
];
|
||||
|
||||
/** Async functions in the markup. */
|
||||
@@ -202,7 +202,7 @@ const MARKUP: SignedRow[] = [
|
||||
// Re-reviewed for BUG-3050 U1 (door A4): the raw-seed refusal adds no await;
|
||||
// its post-loop editor read and the rawSeedDecision check run after the
|
||||
// existing genAtToggle fence and write only locals and a toast.
|
||||
{ body: /genAtToggle/, why: 'Markdown toggle: genAtToggle against loadGeneration after each await', reviewed: 'dea496abe16e' },
|
||||
{ body: /genAtToggle/, why: 'Markdown toggle: genAtToggle against loadGeneration after each await', reviewed: 'c863d1b87a53' },
|
||||
];
|
||||
|
||||
|
||||
@@ -217,7 +217,7 @@ const CONTINUATIONS: SignedRow[] = [
|
||||
may: ['renameOverride'],
|
||||
},
|
||||
{ call: /^setTimeout\($/, body: /copied = false/, why: 'copy-flag reset: switchedAway', reviewed: '8a8d4754017f' },
|
||||
{ call: /api\.items\.get\(wsSlug, itemSlug\)\.catch\($/, body: /./, why: 'loadData item fetch: sets a flag local to that load and re-throws', reviewed: 'faaa27d23182' },
|
||||
{ call: /api\.items\.get\(wsSlug, itemSlug\)\.catch\($/, body: /./, why: 'loadData item fetch: sets a flag local to that load and re-throws', reviewed: '76d69526cf05' },
|
||||
{
|
||||
call: /^setTimeout\($/,
|
||||
body: /staleConnecting = true/,
|
||||
@@ -233,7 +233,7 @@ const CONTINUATIONS: SignedRow[] = [
|
||||
body: /teardownFlushed/,
|
||||
in: 'onBeforeUnload',
|
||||
code: '() => { teardownFlushed = false; }',
|
||||
why: 're-arms the BUG-3005 teardown latch, itself identity-checked', reviewed: '07a11fd90d0b',
|
||||
why: 're-arms the BUG-3005 teardown latch, itself identity-checked', reviewed: '747b66cd731f',
|
||||
may: ['teardownFlushed'],
|
||||
},
|
||||
{ call: /^queueMicrotask\($/, body: /./, why: 'collab lazy seed: refuses a retired or re-identified context first', reviewed: '12e84b8a47be' },
|
||||
@@ -250,22 +250,22 @@ const CONTINUATIONS: SignedRow[] = [
|
||||
call: /^setTimeout\($/,
|
||||
body: /send\(false\)\.catch\(/,
|
||||
in: 'handleContentUpdate',
|
||||
why: 'content debounce: loadData clears this timer before its first await, so the callback never runs across a load', reviewed: '094f908dfba9',
|
||||
why: 'content debounce: loadData clears this timer before its first await, so the callback never runs across a load', reviewed: 'd4fe195f88b7',
|
||||
startSafe: true,
|
||||
pin: (src, unit) => clearsBeforeFirstAwait(src, 'loadData', 'contentDebounceTimer') ?? assignedTo(src, unit, 'contentDebounceTimer'),
|
||||
},
|
||||
// BUG-3230 U0: the fallback save is refused rather than replacing another
|
||||
// tab's unstored edits; the refusal asks, then resends once on overwrite.
|
||||
{ call: /^send\(false\)\.catch\($/, body: /./, why: 'content save refusal: switchedAway before the question, and again after it before the resend', reviewed: '272399d98b0e' },
|
||||
{ call: /return send\(true\); \}\)\.then\($/, body: /^\(sent\) =>/, why: 'content save: switchedAway', reviewed: '99cbdc519986' },
|
||||
{ call: /showSaved\(saveTok\); \}\)\.catch\($/, body: /./, why: 'content save failure: switchedAway', reviewed: '56f6fd2ab1f8' },
|
||||
{ call: /^send\(false\)\.catch\($/, body: /./, why: 'content save refusal: switchedAway before the question, and again after it before the resend', reviewed: 'a6be81eb257a' },
|
||||
{ call: /return send\(true\); \}\)\.then\($/, body: /^\(sent\) =>/, why: 'content save: switchedAway', reviewed: '44370fb4224d' },
|
||||
{ call: /showSaved\(saveTok\); \}\)\.catch\($/, body: /./, why: 'content save failure: switchedAway', reviewed: '12150813f4ca' },
|
||||
{
|
||||
call: /\.finally\($/,
|
||||
body: /^\(\) => saves\.settle\(saveTok\)$/,
|
||||
in: 'setTimeout(…)',
|
||||
code: '() => saves.settle(saveTok)',
|
||||
why: SETTLE_WHY,
|
||||
reviewed: '670d366fd798',
|
||||
reviewed: '49164b6cc800',
|
||||
may: ['saves.settle'],
|
||||
},
|
||||
// BUG-3230 U0: the unload save keeps its text in this browser before it goes
|
||||
@@ -279,29 +279,29 @@ const CONTINUATIONS: SignedRow[] = [
|
||||
in: '{save}',
|
||||
code: "() => { clearRefusedRawDraft(draftUser, reqItemId, markdown); if (item && item.id === reqItemId && genAtSave === loadGeneration && rawContentSaver.pending === markdown) { rawContentSaver.clearPending(); editorStore.setDirty(false); localDirty = false; } }",
|
||||
why: 'raw keepalive save: removes the kept text by the captured user and item, then genAtSave against loadGeneration',
|
||||
reviewed: '435849657502',
|
||||
reviewed: 'dca1fdaf45f5',
|
||||
may: ['clearRefusedRawDraft'],
|
||||
},
|
||||
{
|
||||
call: /localDirty = false; \} \}\) \.catch\($/,
|
||||
body: /^\(e\) =>/,
|
||||
in: '{save}',
|
||||
code: "(e) => { if (isSupersededWriteError(e)) clearRefusedRawDraft(draftUser, reqItemId, markdown); else if (isContentPendingFlush(e)) { toastStore.show( 'Your markdown edits were not saved: another tab has edits to this item that are not stored yet. They are kept in this browser, and opening the item offers them back.', 'error', ); } }",
|
||||
code: "(e) => { if (isSupersededWriteError(e)) clearRefusedRawDraft(draftUser, reqItemId, markdown); else if (isContentPendingFlush(e)) { toastStore.show( pendingEditsReason(e) === 'set_aside' ? 'Your markdown edits were not saved: this item has edits an editor upgrade set aside. They are kept in this browser, and opening the item offers them back.' : 'Your markdown edits were not saved: another tab has edits to this item that are not stored yet. They are kept in this browser, and opening the item offers them back.', 'error', ); } }",
|
||||
why: 'raw keepalive failure: a superseded answer removes the kept text by the captured user and item; a refusal is reported whichever item is shown',
|
||||
reviewed: 'cd76a2dbd33c',
|
||||
may: ['isSupersededWriteError', 'clearRefusedRawDraft', 'isContentPendingFlush', 'toastStore.show'],
|
||||
reviewed: '01ab82a9219a',
|
||||
may: ['isSupersededWriteError', 'clearRefusedRawDraft', 'isContentPendingFlush', 'pendingEditsReason', 'toastStore.show'],
|
||||
},
|
||||
{ call: /overwrite_pending_edits: true \} : \{\}\), \}\)\.then\($/, body: /^\(updated\) =>/, why: 'raw foreground save: genAtSave against loadGeneration', reviewed: 'c968ffa55c7f' },
|
||||
{ call: /overwrite_pending_edits: true \} : \{\}\), \}\)\.then\($/, body: /^\(updated\) =>/, why: 'raw foreground save: genAtSave against loadGeneration', reviewed: 'beffa7ca1c70' },
|
||||
// BUG-3230 U0: the answer to a refused raw save arms one resend.
|
||||
{ call: /askToOverwritePendingEdits\(reqItemId, genAtSave\)\.then\($/, body: /./, why: 'raw refusal answer: genAtSave against loadGeneration before arming the resend', reviewed: '9f2fa38e1fb0' },
|
||||
{ call: /content: item\.content, content_state: item\.content_state \}\); \} \}\)\.catch\($/, body: /./, why: 'raw foreground failure: genAtSave against loadGeneration', reviewed: 'd1a5314e65f5' },
|
||||
{ call: /askToOverwritePendingEdits\(reqItemId, genAtSave, pendingEditsReason\(e\)\)\.then\($/, body: /./, why: 'raw refusal answer: genAtSave against loadGeneration before arming the resend', reviewed: 'ff237680d905' },
|
||||
{ call: /content: item\.content, content_state: item\.content_state \}\); \} \}\)\.catch\($/, body: /./, why: 'raw foreground failure: genAtSave against loadGeneration', reviewed: '32f893e4fe98' },
|
||||
{
|
||||
call: /\.finally\($/,
|
||||
body: /^\(\) => saves\.settle\(saveTok\)$/,
|
||||
in: '{save}',
|
||||
code: '() => saves.settle(saveTok)',
|
||||
why: SETTLE_WHY,
|
||||
reviewed: '7f0b66871d51',
|
||||
reviewed: '91d8e94192f9',
|
||||
may: ['saves.settle'],
|
||||
},
|
||||
];
|
||||
@@ -326,7 +326,7 @@ const HELPERS: Record<string, string> = {
|
||||
identityHeld: '1c5505d51f73',
|
||||
navigateToCollectionRoot: '64dce75693d3',
|
||||
refreshPrintMeta: '2312cc481ca5',
|
||||
runTeardownFlush: '28bce9a45cc2',
|
||||
runTeardownFlush: 'd424dba1db8a',
|
||||
// BUG-3124 unit B: the cursor-advance settle. Synchronous; arms the flusher's
|
||||
// single timer only for the current, identity-held context.
|
||||
showSaved: '456dd972dcf5',
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
// BUG-3244 ruling 1: the pending-edits dialog is the main door that tells a
|
||||
// user what to do about edits their write would replace. For edits an editor
|
||||
// upgrade SET ASIDE, opening the item does not store them, so the set-aside
|
||||
// copy must never say it will. Every kind is rendered for BOTH reasons, so a
|
||||
// branch that never fires (or fires for both) fails.
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({
|
||||
authStore: { onIdentityChange: () => {}, identityFence: () => () => true, user: null },
|
||||
}));
|
||||
|
||||
const { pendingEditsDialog } = await import('$lib/stores/pendingEditsDialog.svelte');
|
||||
const { default: PendingEditsDialog } = await import('./PendingEditsDialog.svelte');
|
||||
|
||||
let root: HTMLElement | null = null;
|
||||
let instance: ReturnType<typeof mount> | null = null;
|
||||
|
||||
afterEach(() => {
|
||||
pendingEditsDialog.abandonAll();
|
||||
flushSync();
|
||||
if (instance) unmount(instance);
|
||||
root?.remove();
|
||||
root = null;
|
||||
instance = null;
|
||||
});
|
||||
|
||||
function renderFor(kind: 'save' | 'duplicate' | 'excerpt', reason: 'pending' | 'set_aside'): string {
|
||||
root = document.body.appendChild(document.createElement('div'));
|
||||
instance = mount(PendingEditsDialog, { target: root });
|
||||
void pendingEditsDialog.request('TASK-7', kind, reason);
|
||||
flushSync();
|
||||
return document.body.textContent ?? '';
|
||||
}
|
||||
|
||||
describe('PendingEditsDialog', () => {
|
||||
for (const kind of ['save', 'duplicate', 'excerpt'] as const) {
|
||||
it(`${kind}: set-aside copy names the upgrade and never offers opening the item`, () => {
|
||||
const text = renderFor(kind, 'set_aside');
|
||||
expect(text).toMatch(/earlier editor version|editor upgrade/);
|
||||
expect(text).toMatch(/TASK-7/);
|
||||
expect(text).not.toMatch(/open tab|Open the item|opening the item so/i);
|
||||
});
|
||||
|
||||
it(`${kind}: pending copy is unchanged`, () => {
|
||||
const text = renderFor(kind, 'pending');
|
||||
expect(text).toMatch(/open tab/);
|
||||
expect(text).not.toMatch(/earlier editor version/);
|
||||
});
|
||||
}
|
||||
|
||||
it('save: the set-aside choice is to discard, not to overwrite a tab', () => {
|
||||
const text = renderFor('save', 'set_aside');
|
||||
expect(text).toMatch(/Discard them/);
|
||||
expect(text).toMatch(/Keep those edits/);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,7 @@
|
||||
<script lang="ts">
|
||||
import type { Version, Item } from '$lib/types';
|
||||
import { api, isContentPendingFlushError } from '$lib/api/client';
|
||||
import { pendingEditsReason } from '$lib/items/contentWrite';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import DiffView from '$lib/components/versions/DiffView.svelte';
|
||||
import StaleBodyNotice from '$lib/components/common/StaleBodyNotice.svelte';
|
||||
@@ -49,6 +50,8 @@
|
||||
// another editor session has not saved. The confirm then asks again, naming
|
||||
// what the restore would discard, and a second yes sends the override.
|
||||
let pendingEdits = $state(false);
|
||||
// BUG-3244: the pending edits were set aside by an editor upgrade, not held by a tab.
|
||||
let pendingSetAside = $state(false);
|
||||
|
||||
// The timeline endpoint serves raw reverse-patch text for diff versions
|
||||
// (is_diff), so version.content is unreadable patch data, not real content.
|
||||
@@ -97,6 +100,7 @@
|
||||
if (!expanded) {
|
||||
confirming = false;
|
||||
pendingEdits = false;
|
||||
pendingSetAside = false;
|
||||
} else {
|
||||
ensureResolved();
|
||||
}
|
||||
@@ -109,6 +113,7 @@
|
||||
function cancelRestore() {
|
||||
confirming = false;
|
||||
pendingEdits = false;
|
||||
pendingSetAside = false;
|
||||
}
|
||||
|
||||
async function confirmRestore(overwritePendingEdits: boolean) {
|
||||
@@ -167,6 +172,7 @@
|
||||
if (isContentPendingFlushError(err)) {
|
||||
if (reqSlug !== itemSlug || reqWs !== wsSlug) return;
|
||||
pendingEdits = true;
|
||||
pendingSetAside = pendingEditsReason(err) === 'set_aside';
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
@@ -175,6 +181,7 @@
|
||||
if (reqSlug !== itemSlug || reqWs !== wsSlug) return;
|
||||
confirming = false;
|
||||
pendingEdits = false;
|
||||
pendingSetAside = false;
|
||||
onRestore?.(updatedItem);
|
||||
} finally {
|
||||
restoring = false;
|
||||
@@ -242,7 +249,9 @@
|
||||
<div class="confirm-prompt">
|
||||
{#if pendingEdits}
|
||||
<span class="confirm-text confirm-warning" role="alert">
|
||||
This item has unsaved edits from another tab or session. Restoring will discard them, and no version will keep them.
|
||||
{pendingSetAside
|
||||
? 'This item has edits from an earlier editor version that are not in its saved body. Restoring will discard them, and no version will keep them.'
|
||||
: 'This item has unsaved edits from another tab or session. Restoring will discard them, and no version will keep them.'}
|
||||
</span>
|
||||
{:else}
|
||||
<span class="confirm-text">Restore to this version?</span>
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
// BUG-3244 ruling 1: edits an editor upgrade SET ASIDE are marked like any
|
||||
// stale body, but no copy may tell the user that opening the item restores
|
||||
// them. Each describe pairs the set-aside leg with the pending leg it must
|
||||
// differ from, so a branch that never fires cannot pass.
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { PadApiError } from '$lib/api/client';
|
||||
import type { ItemCopyResult } from '$lib/types';
|
||||
import { pendingEditsReason } from './contentWrite';
|
||||
import { copyResultToast } from './copyResultToast';
|
||||
import { rawSeedDecision } from './rawSeed';
|
||||
import { isBodyStale, isSetAside } from './staleBody';
|
||||
|
||||
const refusal = (details?: Record<string, unknown>) =>
|
||||
Object.assign(Object.create(PadApiError.prototype), { code: 'content_pending_flush', message: 'x', details });
|
||||
|
||||
function copyResult(state: string, archived = false): ItemCopyResult {
|
||||
return {
|
||||
source: { archived },
|
||||
destination: { workspace_name: 'Other', ref: 'TASK-9', slug: 'x' },
|
||||
archive_source: archived,
|
||||
warnings: {
|
||||
dropped_fields: [],
|
||||
dropped_assignee: false,
|
||||
dropped_agent_role: false,
|
||||
attachment_count: 0,
|
||||
attachment_bytes: 0,
|
||||
unresolvable_ref_count: 0,
|
||||
source_content_state: state,
|
||||
},
|
||||
} as unknown as ItemCopyResult;
|
||||
}
|
||||
|
||||
describe('the stale-body mark covers both values', () => {
|
||||
it('marks pending and set-aside, not a current row', () => {
|
||||
expect(isBodyStale({ content_state: 'applied_pending_flush' })).toBe(true);
|
||||
expect(isBodyStale({ content_state: 'superseded_set_aside' })).toBe(true);
|
||||
expect(isBodyStale({})).toBe(false);
|
||||
expect(isSetAside('superseded_set_aside')).toBe(true);
|
||||
expect(isSetAside('applied_pending_flush')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('pendingEditsReason', () => {
|
||||
it('reads set_aside_rows from the refusal', () => {
|
||||
expect(pendingEditsReason(refusal({ ref: 'TASK-1', pending_rows: 0, set_aside_rows: 2 }))).toBe('set_aside');
|
||||
});
|
||||
it('a refusal about unflushed edits only, or from an older server, is pending', () => {
|
||||
expect(pendingEditsReason(refusal({ ref: 'TASK-1', pending_rows: 3 }))).toBe('pending');
|
||||
expect(pendingEditsReason(refusal())).toBe('pending');
|
||||
expect(pendingEditsReason(new Error('x'))).toBe('pending');
|
||||
});
|
||||
});
|
||||
|
||||
describe('copy and move toast', () => {
|
||||
for (const archived of [false, true]) {
|
||||
it(`${archived ? 'move' : 'copy'}: a set-aside source never says to open it or wait for a save`, () => {
|
||||
const setAside = copyResultToast(copyResult('superseded_set_aside', archived));
|
||||
const pending = copyResultToast(copyResult('applied_pending_flush', archived));
|
||||
expect(setAside.type).toBe('info');
|
||||
expect(setAside.message).toMatch(/earlier editor version/);
|
||||
expect(setAside.message).not.toMatch(/open it|once they are saved|open tab/);
|
||||
expect(pending.message).toMatch(/open tab/);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('raw editor switch', () => {
|
||||
it('is refused on a set-aside item even when the live read equals the seed', () => {
|
||||
const inputs = { liveNow: 'same', lastFlushed: 'same', stored: 'same' };
|
||||
expect(rawSeedDecision({ ...inputs, contentState: 'superseded_set_aside' }).refuse).toBe(true);
|
||||
expect(rawSeedDecision({ ...inputs, contentState: 'applied_pending_flush' }).refuse).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -17,6 +17,7 @@
|
||||
import { PadApiError } from '$lib/api/client';
|
||||
import type { Item } from '$lib/types';
|
||||
import { occTokenFor } from '$lib/items/occToken';
|
||||
import type { PendingEditsReason } from '$lib/stores/pendingEditsDialog.svelte';
|
||||
|
||||
/**
|
||||
* The body half of an update: nothing when the body is unchanged since load,
|
||||
@@ -66,3 +67,17 @@ export function contentOutcomeNotice(resp: Pick<Item, 'warnings'> | null | undef
|
||||
export function isContentPendingFlush(err: unknown): boolean {
|
||||
return err instanceof PadApiError && err.code === 'content_pending_flush';
|
||||
}
|
||||
|
||||
/**
|
||||
* Why a content_pending_flush refusal happened (BUG-3244): 'set_aside' when the
|
||||
* server counted edits an editor upgrade set aside (details.set_aside_rows),
|
||||
* which no tab will ever store, else 'pending'. Copy that tells the user what to
|
||||
* do branches on it.
|
||||
*/
|
||||
export function pendingEditsReason(err: unknown): PendingEditsReason {
|
||||
if (err instanceof PadApiError) {
|
||||
const n = err.details?.set_aside_rows;
|
||||
if (typeof n === 'number' && n > 0) return 'set_aside';
|
||||
}
|
||||
return 'pending';
|
||||
}
|
||||
|
||||
@@ -40,7 +40,7 @@ describe('copyResultToast', () => {
|
||||
expect(t.type).toBe('info');
|
||||
expect(t.message).toMatch(/^Moved to Other as TASK-9\. /);
|
||||
expect(t.message).toMatch(/archived original may still hold them/);
|
||||
// Never a promise: a purge or a schema-mismatch rebuild can delete them.
|
||||
// Never a promise: a workspace purge can delete them.
|
||||
expect(t.message).not.toMatch(/still has them/);
|
||||
});
|
||||
|
||||
|
||||
@@ -10,9 +10,12 @@
|
||||
// On a MOVE the source is archived. Archiving is a soft delete that leaves
|
||||
// the op-log alone, and the dormant GC keeps unflushed rows, so the edits
|
||||
// are USUALLY still with the archived original. Not always: a workspace
|
||||
// purge deletes them, and so does the schema-mismatch rebuild when the item
|
||||
// is next opened after an editor schema bump (collab manager
|
||||
// maybeRebuildOnSchemaMismatch). So the wording says "may", never "has".
|
||||
// purge deletes them. (An editor schema bump no longer does: since BUG-3244
|
||||
// the rebuild sets them aside, still on the original.) So the wording says
|
||||
// "may", never "has".
|
||||
// - superseded_set_aside (BUG-3244): the source's missing edits were set
|
||||
// aside by an editor upgrade. They stay with the original, and opening it
|
||||
// will not store them, so the wording offers no "open it, then again".
|
||||
import type { ItemCopyResult } from '$lib/types';
|
||||
|
||||
export interface CopyResultToast {
|
||||
@@ -33,7 +36,13 @@ export function copyResultToast(result: ItemCopyResult): CopyResultToast {
|
||||
if (notUnique.length > 0) parts[0] += `, without: ${notUnique.map((d) => d.message).join('; ')}`;
|
||||
|
||||
const stale = !!result.warnings.source_content_state;
|
||||
if (stale) {
|
||||
if (stale && result.warnings.source_content_state === 'superseded_set_aside') {
|
||||
parts.push(
|
||||
moved
|
||||
? 'This item has edits from an earlier editor version that are not in its body, so the moved copy does not include them. They stay with the archived original.'
|
||||
: 'This item has edits from an earlier editor version that are not in its body, so the copy does not include them. They stay with the original.',
|
||||
);
|
||||
} else if (stale) {
|
||||
parts.push(
|
||||
moved
|
||||
? 'An open tab had edits to this item that were not saved yet, so the moved copy may be missing them. The archived original may still hold them: restore it and open it, then move it again once they are saved.'
|
||||
|
||||
@@ -28,6 +28,10 @@ export interface RawSeedInputs {
|
||||
|
||||
export function rawSeedDecision(inputs: RawSeedInputs): { seed: string; refuse: boolean } {
|
||||
const seed = inputs.lastFlushed ?? inputs.stored;
|
||||
// Set-aside edits (BUG-3244) are in no document a live read can see, and the
|
||||
// server refuses every raw save while they exist (the raw saves send
|
||||
// refuse_pending_edits), so the switch is refused up front instead.
|
||||
if (inputs.contentState === 'superseded_set_aside') return { seed, refuse: true };
|
||||
if (typeof inputs.liveNow === 'string') {
|
||||
// A live read exists: the seed must BE it. This tab's own unflushed
|
||||
// typing marks the item too, which is why the marker is not consulted
|
||||
|
||||
@@ -17,7 +17,17 @@
|
||||
/** The one wording every treatment uses. */
|
||||
export const STALE_BODY_NOTICE = 'This may not include the latest edits.';
|
||||
|
||||
/** True when the server marked this row's body as behind the live document. */
|
||||
/** True when the server marked this row's body as behind edits that exist
|
||||
* elsewhere: unflushed tab edits, or edits an editor upgrade set aside
|
||||
* (BUG-3244). The notice and dot wording names no remedy, so it fits both;
|
||||
* copy that DOES name one must branch on the value (isSetAside). */
|
||||
export function isBodyStale(row: { content_state?: string | null } | null | undefined): boolean {
|
||||
return row?.content_state === 'applied_pending_flush';
|
||||
return row?.content_state === 'applied_pending_flush' || row?.content_state === 'superseded_set_aside';
|
||||
}
|
||||
|
||||
/** True when the row's missing edits were set aside by an editor upgrade
|
||||
* (BUG-3244): opening the item does NOT restore them, so no copy may say it
|
||||
* will. */
|
||||
export function isSetAside(state: string | null | undefined): boolean {
|
||||
return state === 'superseded_set_aside';
|
||||
}
|
||||
|
||||
@@ -6,10 +6,14 @@
|
||||
import { authStore } from './auth.svelte';
|
||||
|
||||
export type PendingEditsKind = 'save' | 'duplicate' | 'excerpt';
|
||||
/** 'set_aside': the edits were set aside by an editor upgrade (BUG-3244) and
|
||||
* opening the item will not store them, so the dialog must not say it will. */
|
||||
export type PendingEditsReason = 'pending' | 'set_aside';
|
||||
|
||||
interface PendingRequest {
|
||||
itemRef: string;
|
||||
kind: PendingEditsKind;
|
||||
reason: PendingEditsReason;
|
||||
resolve: (overwrite: boolean) => void;
|
||||
}
|
||||
|
||||
@@ -21,9 +25,9 @@ function advanceQueue(): void {
|
||||
}
|
||||
|
||||
/** Resolves true to OVERWRITE (or, for a duplicate, copy the stored body anyway), false to keep. */
|
||||
function request(itemRef: string, kind: PendingEditsKind = 'save'): Promise<boolean> {
|
||||
function request(itemRef: string, kind: PendingEditsKind = 'save', reason: PendingEditsReason = 'pending'): Promise<boolean> {
|
||||
return new Promise<boolean>((resolve) => {
|
||||
const entry: PendingRequest = { itemRef, kind, resolve };
|
||||
const entry: PendingRequest = { itemRef, kind, reason, resolve };
|
||||
if (active === null) active = entry;
|
||||
else queue.push(entry);
|
||||
});
|
||||
|
||||
@@ -115,7 +115,7 @@ export interface PublicShareItem {
|
||||
fields?: string;
|
||||
content?: string;
|
||||
/** BUG-3000 marker: `content` is behind the live document (BUG-3050 U3). */
|
||||
content_state?: 'applied_pending_flush';
|
||||
content_state?: 'applied_pending_flush' | 'superseded_set_aside';
|
||||
}
|
||||
|
||||
/** The shape returned by GET /api/v1/s/{token}. Auth/password gates short-circuit
|
||||
@@ -136,7 +136,7 @@ export interface SharePayload {
|
||||
collection_name?: string;
|
||||
collection_icon?: string;
|
||||
/** BUG-3000 marker: `content` is behind the live document (BUG-3050 U3). */
|
||||
content_state?: 'applied_pending_flush';
|
||||
content_state?: 'applied_pending_flush' | 'superseded_set_aside';
|
||||
};
|
||||
/** Single-item shares only (TASK-2248 U3): defs for the fields the item
|
||||
* shows, in schema order. Deliberately NOT the schema: key, label, type,
|
||||
@@ -696,7 +696,7 @@ export interface Item {
|
||||
* field. It matters for surfaces that render `content` straight from the
|
||||
* API without joining the room.
|
||||
*/
|
||||
content_state?: 'applied_pending_flush';
|
||||
content_state?: 'applied_pending_flush' | 'superseded_set_aside';
|
||||
fields: string;
|
||||
tags: string;
|
||||
pinned: boolean;
|
||||
@@ -1541,7 +1541,7 @@ export interface Backlink {
|
||||
* BacklinksPanel renders it as a muted dot on the marked row only, beside
|
||||
* the snippet (BUG-3050 U3, `lib/items/staleBody.ts`).
|
||||
*/
|
||||
content_state?: 'applied_pending_flush';
|
||||
content_state?: 'applied_pending_flush' | 'superseded_set_aside';
|
||||
updated_at: string;
|
||||
/**
|
||||
* Optional `[[X|display]]` override. `null` (omitted from JSON) when
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
import { parseFields, parseSchema, itemUrlId, formatItemRef } from '$lib/types';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { titleLimitError } from '$lib/items/titleLimit';
|
||||
import { contentOutcomeNotice, contentWriteFor, isContentPendingFlush, prunedEditsNotice } from '$lib/items/contentWrite';
|
||||
import { contentOutcomeNotice, contentWriteFor, isContentPendingFlush, pendingEditsReason, prunedEditsNotice } from '$lib/items/contentWrite';
|
||||
import { pendingEditsDialog } from '$lib/stores/pendingEditsDialog.svelte';
|
||||
import { createScrollRestoration } from '$lib/scroll/restore.svelte';
|
||||
import { exportAndDownloadArtifact, importArtifactFile } from '$lib/utils/artifacts';
|
||||
@@ -335,8 +335,14 @@
|
||||
updated = await api.items.update(workspace, item.slug, write);
|
||||
} catch (err) {
|
||||
if (!isContentPendingFlush(err)) throw err;
|
||||
if (!(await pendingEditsDialog.request(formatItemRef(item) ?? item.title))) {
|
||||
toastStore.show("Not saved: the open tab's edits were kept. Your text is still here.", 'info');
|
||||
const reason = pendingEditsReason(err);
|
||||
if (!(await pendingEditsDialog.request(formatItemRef(item) ?? item.title, 'save', reason))) {
|
||||
toastStore.show(
|
||||
reason === 'set_aside'
|
||||
? "Not saved: the edits an editor upgrade set aside were kept. Your text is still here."
|
||||
: "Not saved: the open tab's edits were kept. Your text is still here.",
|
||||
'info'
|
||||
);
|
||||
return;
|
||||
}
|
||||
updated = await api.items.update(workspace, item.slug, { ...write, overwrite_pending_edits: true });
|
||||
|
||||
@@ -23,7 +23,7 @@
|
||||
import EmptyState from '$lib/components/common/EmptyState.svelte';
|
||||
import PlaybookFormFields from '$lib/components/playbooks/PlaybookFormFields.svelte';
|
||||
import StaleBodyDot from '$lib/components/common/StaleBodyDot.svelte';
|
||||
import { isBodyStale } from '$lib/items/staleBody';
|
||||
import { isBodyStale, isSetAside } from '$lib/items/staleBody';
|
||||
import {
|
||||
PLAYBOOK_SKELETON_BODY,
|
||||
argumentsToJSON,
|
||||
@@ -336,8 +336,9 @@
|
||||
// BUG-3050 U1: a source whose stored body is behind an open tab's
|
||||
// edits would be copied WITHOUT them. A create has no token for the
|
||||
// server to refuse on, so the user is asked here instead.
|
||||
if (item.content_state === 'applied_pending_flush' &&
|
||||
!(await pendingEditsDialog.request(formatItemRef(item) ?? item.title, 'duplicate'))) {
|
||||
if (isBodyStale(item) &&
|
||||
!(await pendingEditsDialog.request(formatItemRef(item) ?? item.title, 'duplicate',
|
||||
isSetAside(item.content_state) ? 'set_aside' : 'pending'))) {
|
||||
return;
|
||||
}
|
||||
await api.items.create(wsSlug, 'playbooks', {
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import { titleEditError } from '$lib/items/titleLimit';
|
||||
import { contentOutcomeNotice, contentWriteFor, isContentPendingFlush, prunedEditsNotice } from '$lib/items/contentWrite';
|
||||
import { contentOutcomeNotice, contentWriteFor, isContentPendingFlush, pendingEditsReason, prunedEditsNotice } from '$lib/items/contentWrite';
|
||||
import { pendingEditsDialog } from '$lib/stores/pendingEditsDialog.svelte';
|
||||
import { createScrollRestoration } from '$lib/scroll/restore.svelte';
|
||||
import { exportAndDownloadArtifact } from '$lib/utils/artifacts';
|
||||
@@ -267,10 +267,16 @@
|
||||
} catch (err) {
|
||||
if (!isContentPendingFlush(err)) throw err;
|
||||
if (!isSameIdentity()) return;
|
||||
const overwrite = await pendingEditsDialog.request(formatItemRef(item) ?? item.title);
|
||||
const reason = pendingEditsReason(err);
|
||||
const overwrite = await pendingEditsDialog.request(formatItemRef(item) ?? item.title, 'save', reason);
|
||||
if (!isSameIdentity()) return;
|
||||
if (!overwrite) {
|
||||
toastStore.show("Not saved: the open tab's edits were kept. Your changes are still here.", 'info');
|
||||
toastStore.show(
|
||||
reason === 'set_aside'
|
||||
? "Not saved: the edits an editor upgrade set aside were kept. Your changes are still here."
|
||||
: "Not saved: the open tab's edits were kept. Your changes are still here.",
|
||||
'info'
|
||||
);
|
||||
return;
|
||||
}
|
||||
const resent = await api.items.update(wsSlug, item.slug, { ...payload, overwrite_pending_edits: true });
|
||||
|
||||
@@ -17,7 +17,7 @@ identityGateSuite({
|
||||
loadItem: { reviewed: '1d65e4a33dc3', why: 'workspace, ref AND the entry identity fence after the fetch, on both arms and in the finally' },
|
||||
loadPlaybooks: { reviewed: '6825e2ad3d9b', why: 'workspace AND the entry identity fence after the fetch, on both arms' },
|
||||
loadCollection: { reviewed: '5e49f16c7659', why: 'workspace AND the entry identity fence after the fetch, on both arms' },
|
||||
save: { reviewed: '9a71c395f37e', why: 'user-initiated save; the entry identity fence before the dialog, after its answer (so the overwrite re-send never goes under another identity), before the success report and navigation, on the failure report, and on the finally that clears saving' },
|
||||
save: { reviewed: 'e3044b77107a', why: 'user-initiated save; the entry identity fence before the dialog, after its answer (so the overwrite re-send never goes under another identity), before the success report and navigation, on the failure report, and on the finally that clears saving' },
|
||||
handleExport: { reviewed: 'bd5b6bebfde2', why: 'user-initiated export; the entry identity fence before either toast and on the finally that clears exporting' },
|
||||
},
|
||||
nested: [],
|
||||
|
||||
Reference in New Issue
Block a user