mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 06:19:49 +00:00
fix: a convention's stored metadata copy follows its ordinary fields (TASK-2257) (#1964)
A convention keeps its metadata twice: in the ordinary fields (trigger, scope/surfaces, enforcement/priority, category, commands), which agents and bootstrap read, and in the reserved `convention` key, which item.convention prefers. Since BUG-3163 nothing but the create member can write the reserved copy, so a fields_patch changing `trigger` left the copy behind and the two disagreed. UpdateItem now mirrors every mirrored key the write CHANGED into the reserved copy, under the write lock, after the BUG-3163 carry guard. A convention with no reserved copy is untouched. The merged copy is validated with models.ValidateConventionMetadata; a write that would leave it invalid is refused 400 validation_error (new, deliberate: storing an invalid copy is what the reserved key exists to prevent). The web Conventions page's edit form gains trigger / surface / enforcement selects (sent as fields_patch with both spellings) and an "Open item" link to the full item. Drift measured before the change: 194 conventions, 5 with a reserved copy, all 5 agreeing, so no reconcile follow-up was filed.
This commit is contained in:
@@ -24,6 +24,7 @@ REST API at `/api/v1/`. Key endpoints:
|
||||
- **READ responses may carry `content_state`** (BUG-3000 / BUG-3033) — the read-side counterpart to `warnings.content_outcome` above, and its pending value is the same, `applied_pending_flush`. It has a second value no write produces, `superseded_set_aside` (BUG-3244): edits a collab schema-version rebuild set aside, which no tab can restore; see the set-aside bullet below. It wins when both apply, and a door that only marks staleness treats the two alike (`models.IsContentStateStale`, web `isBodyStale`) while any copy that names a remedy branches on it. A write warning says what a REQUEST did; this says what the ROW is: the item's op-log holds CONTENT-BEARING updates above `items.content_flushed_op_log_id`, so the body being served is BEHIND the live collaborative document. Since BUG-3124 rows that provably cannot change the document — y-protocols SyncStep1 frames, empty updates, and byte-identical re-sends of an earlier row (a SyncStep2 and an update differing only in their subtype byte count as identical since BUG-3136, which is what an open tab's answer to a joining tab is) — are persisted but marked `content_bearing = false` and not counted; they used to keep an item "pending" forever after its tab closed (measured: 470 pending items → 189 on the live DB). The classifier is envelope-only (no Yjs parse) and strict: a frame that does not parse exactly stays content-bearing. Additive and `omitempty`, and emitted only where a real body is — a summary or metadata projection that serves no content never claims its absent body is stale. The window is **bounded by server-side recovery** (TASK-2198 U4), not by a tab: when the item's collab room closes (60s after its last tab leaves) with content-bearing rows above the watermark, or when a one-minute sweep finds such an item dormant for 2 minutes with no room, a background worker materializes the WHOLE content-bearing op-log in a capped child process (`pad __materialize-worker`, internal/materialize) and `store.MaterializeFlush` lands it: cursor == MAX(op-log id), rows pending and no set-aside rows, checked in the write's own transaction under the collab item lock with no room open; watermark to the cursor only, `seq` bumped, one version row attributed `system` / `recovery` ("recovered from an unsaved editor session"), op-log NOT pruned. It never runs on a request path, and it is NOT a guarantee: a job that fails 3 times in a row is not retried until the op-log changes, an item with set-aside rows or rows from another schema version is skipped, and with `PAD_MATERIALIZE=off` the window is unbounded as before (the op-log GC's flush-coverage guard still RETAINS unflushed rows), so the row catches up only when a browser tab next opens the item. Opening it is now enough even when nothing needs flushing (BUG-3124 unit B): a caught-up tab whose document still renders to the stored body (byte for byte, or, since BUG-3197, as the editor's own serialization of it) calls `POST /workspaces/{ws}/items/{slug}/collab-watermark {op_log_cursor, content_sha256}`, and the server advances the flush watermark iff `cursor == MAX(op-log id)` and the sha256 matches `items.content`, atomically, writing no content, version, `seq` or `updated_at` — before this, a view that changed nothing was deduped and left the item "pending" forever. Web-client only; no CLI/MCP surface. The content is not lost — the op-log is the durability layer — but the marker is a signal to RE-READ LATER, never to re-send. The predicate is broader than "an API write is pending flush" and deliberately so: a user typing in an open tab produces the same stale body, and the row does not record WHY the op-log is ahead. Carried by every door that serialises `models.Item`, and by the hand-built ones too — the playbook `run` response, bootstrap convention and bodies-mode include payloads, the exported artifact's `provenance` block, and a line above the body in the MCP item resource. The CLI prints it to **stderr** (`pad item show --format markdown`, `pad playbook show`, `pad playbook run`), like the write warnings — except `pad item show`'s default TABLE output, which since TASK-3132 opens with it on **stdout**; markdown stays stderr because its stdout is the body verbatim for `update --stdin`
|
||||
- **`github_pr` is written only through typed update members** (BUG-2696): `github_pr: {number, url, title?, state?, branch?, repo?, updated_at?}` (number > 0 and an absolute http(s) url required, else 400) and `clear_github_pr: true`, mutually exclusive with each other and with a full `fields` write. A `fields_patch` entry naming it is refused like the other three reserved keys. The point is VALIDITY, not sender identity: any caller may send the member, but only a well-formed PR object gets in. `pad github link` / `unlink` / `project reconcile` use it and verify from the response that it landed; a server older than the CLI ignores the member and answers 200, which the CLI reports as an error naming the likely skew. Not on the MCP catalog
|
||||
- **Reserved metadata keys are refused in create's `fields`, and a full `fields` update may only CARRY them** (BUG-3163): `implementation_notes`, `decision_log`, `github_pr` and `convention` in an item CREATE's `fields` answer 400 `validation_error` and nothing is created. Convention metadata goes in through a typed create member, `convention: {category?, trigger?, surfaces?, enforcement?, commands?}` (an empty object is refused), which library activation uses on every surface (CLI, remote MCP, web library and Conventions pages); the CLI verifies from the stored fields that it landed, because an older server ignores the member. A FULL `fields` blob on update may carry a stored reserved value unchanged (compared as decoded JSON, not bytes), but a differing value or an OMITTED stored key (which would delete it) is refused with 400, checked against the row under the write lock. Use `fields_patch` for ordinary writes; it leaves unnamed keys untouched
|
||||
- **A convention's reserved `convention` copy follows its ordinary metadata keys** (TASK-2257). A convention stores its metadata twice: the ordinary fields `trigger`, `scope` / `surfaces`, `enforcement` / `priority`, `category` and `commands` (what agents obey; bootstrap and `--field` filters read them) and the reserved `convention` key (what `item.convention` prefers). When an update CHANGES one of those ordinary keys on an item that HAS a reserved copy, the server rebuilds the copy from the merged fields in the same transaction, after the carry check above: `surfaces` from `surfaces`, else `[scope]`; `enforcement` from `enforcement`, else `priority`; untouched keys keep their stored value. An item without a reserved copy is left as it was. The one new refusal is deliberate: a write whose merged values would leave the copy empty answers 400 `validation_error` and writes nothing. Before this, `pad item update --field trigger=X` left `item.convention` on the old trigger
|
||||
- **`refuse_undeclared_fields: true` is a strict opt-in on item update** (BUG-3156): a write that would store a field key the collection does not declare is REFUSED with 400 `validation_error`, and nothing is written, instead of being accepted and named in `warnings.undeclared_fields`. It refuses EXACTLY the keys that warning would have named: on a `fields_patch` write the patched keys only; on a full `fields` write the whole blob, stray stored keys included. It is additive and omitted by default, so a write without it is unchanged. **Skew:** a server that predates it ignores the member, which is the accept-and-warn, so a strict caller confirms by the ABSENCE of the warning rather than trusting a 200. `pad item bulk-update` and the remote bulk-update dispatcher set it; `pad item update` / `pad_item.update` do not
|
||||
- **A content write carrying a version token is REFUSED while a tab holds unflushed edits** (BUG-3133): 409 `content_pending_flush`, lifted by `overwrite_pending_edits: true`. The token guards the row, and those edits live only in the op-log until a tab flushes, so re-reading does not clear it. A tokenless content write still replaces them; with no tab open that deletes them from the op-log, and the response names how many in `warnings.pruned_pending_edits` (additive, omitempty; the CLI prints it to stderr). Rows a schema rebuild SET ASIDE (BUG-3244) refuse EVERY content write, token or not, with the same code plus `details.set_aside_rows`, because no tab will ever store them; `overwrite_pending_edits` deletes them in the write's own transaction and counts them into `warnings.pruned_pending_edits`. A collab-snapshot flush is exempt and leaves them alone. A writer that cannot carry a token asks for the same refusal with `refuse_pending_edits: true` (BUG-3230 U0): no row version is compared, and `overwrite_pending_edits` still lifts it. The web item pane's raw-markdown saves send it, because their debounced saves overlap and their unload flush is a teardown write (BUG-3080). Web-only: not on the CLI or the MCP catalog, so no tool-surface bump, and a server that predates it ignores it and replaces
|
||||
- **Write responses may also carry `warnings.dropped_fields`** (TASK-2878, widened by BUG-3079) — schema-declared keys the write DISCARDED. The general case is an injected schema DEFAULT that fails the same `validateFieldType` a caller-supplied value takes: it used to be assigned and skipped past that check, so the SAME BYTES were refused when supplied and stored when injected, decided only by who put them there — a `status` retyped to `multi_select` whose scalar default survived answered 400 to `{"status":"open"}` and 201 to `{}`, storing `{"status":"open"}`. It is now dropped and named here. The relation case keeps its own later pass, because "is this a string" and "does this string name a live, visible item" are different questions and only the first is answerable in the DB-free validator. Dropped rather than refused because nobody in the request typed it — EXCEPT on a `required` field, where dropping leaves it absent and the write is refused as required, with a message naming the default as the cause. Additive and `omitempty`
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// TASK-2257: editing a convention's ordinary metadata keys (what agents obey)
|
||||
// keeps item.convention (what the web reads) in step. Measured before the fix:
|
||||
// `fields_patch {trigger: on-commit}` answered 200 and item.convention still
|
||||
// read "always".
|
||||
func TestTASK2257_FieldsPatchKeepsTheConventionCopyInStep(t *testing.T) {
|
||||
t.Parallel()
|
||||
srv := testServer(t)
|
||||
ws := createWSWithCollections(t, srv)
|
||||
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+ws+"/collections/conventions/items", map[string]any{
|
||||
"title": "Mirror rule",
|
||||
"fields": `{"status":"active","trigger":"always","scope":"all","priority":"should","enforcement":"should"}`,
|
||||
"convention": map[string]any{"trigger": "always", "surfaces": []string{"all"}, "enforcement": "should"},
|
||||
})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("create: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var it models.Item
|
||||
parseJSON(t, rr, &it)
|
||||
|
||||
rr = doRequest(srv, "PATCH", "/api/v1/workspaces/"+ws+"/items/"+it.Slug, map[string]any{
|
||||
"fields_patch": map[string]any{"trigger": "on-commit", "priority": "must", "enforcement": "must"},
|
||||
})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("patch: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
rr = doRequest(srv, "GET", "/api/v1/workspaces/"+ws+"/items/"+it.Slug, nil)
|
||||
var got models.Item
|
||||
parseJSON(t, rr, &got)
|
||||
if got.Convention == nil || got.Convention.Trigger != "on-commit" || got.Convention.Enforcement != "must" {
|
||||
t.Fatalf("item.convention = %+v, want trigger on-commit, enforcement must", got.Convention)
|
||||
}
|
||||
if len(got.Convention.Surfaces) != 1 || got.Convention.Surfaces[0] != "all" {
|
||||
t.Fatalf("untouched surfaces changed: %v", got.Convention.Surfaces)
|
||||
}
|
||||
}
|
||||
|
||||
// A full `fields` write that carries the stored reserved copy unchanged
|
||||
// (BUG-3163's rule) and changes `trigger` passes the carry guard, and the copy
|
||||
// follows.
|
||||
func TestTASK2257_FullFieldsCarryStillPassesAndTheCopyFollows(t *testing.T) {
|
||||
t.Parallel()
|
||||
srv := testServer(t)
|
||||
ws := createWSWithCollections(t, srv)
|
||||
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+ws+"/collections/conventions/items", map[string]any{
|
||||
"title": "Full carry",
|
||||
"fields": `{"status":"active","trigger":"always","scope":"all","priority":"should","enforcement":"should"}`,
|
||||
"convention": map[string]any{"trigger": "always", "surfaces": []string{"all"}, "enforcement": "should"},
|
||||
})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("create: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var it models.Item
|
||||
parseJSON(t, rr, &it)
|
||||
|
||||
full := `{"status":"active","trigger":"on-implement","scope":"all","priority":"should","enforcement":"should","convention":{"enforcement":"should","surfaces":["all"],"trigger":"always"}}`
|
||||
rr = doRequest(srv, "PATCH", "/api/v1/workspaces/"+ws+"/items/"+it.Slug, map[string]any{"fields": full})
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("full fields carrying the reserved copy: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
rr = doRequest(srv, "GET", "/api/v1/workspaces/"+ws+"/items/"+it.Slug, nil)
|
||||
var got models.Item
|
||||
parseJSON(t, rr, &got)
|
||||
if got.Convention == nil || got.Convention.Trigger != "on-implement" {
|
||||
t.Fatalf("item.convention = %+v, want trigger on-implement", got.Convention)
|
||||
}
|
||||
}
|
||||
@@ -2864,6 +2864,17 @@ func (s *Store) updateItemWithParentLinkOnce(
|
||||
input.Fields = &base
|
||||
}
|
||||
|
||||
// TASK-2257: keep a convention's reserved metadata copy in step with the
|
||||
// ordinary keys this write changed. After the BUG-3163 carry guard (the
|
||||
// precheck above), on the merged blob, under the same lock.
|
||||
if input.Fields != nil {
|
||||
mirrored, mErr := mirrorConventionMetadata(existing.Fields, *input.Fields)
|
||||
if mErr != nil {
|
||||
return nil, mErr
|
||||
}
|
||||
input.Fields = &mirrored
|
||||
}
|
||||
|
||||
// BUG-3407: if the schema the caller validated against has moved since,
|
||||
// re-check the keys this write SETS against the schema under the seq lock
|
||||
// held above. A patch sets its own keys; a full `fields` write sets the
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"strings"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// A convention keeps its metadata twice (TASK-2257). The ordinary fields
|
||||
// (`trigger`, `scope` / `surfaces`, `enforcement` / `priority`, `category`,
|
||||
// `commands`) are what agents obey: bootstrap and `--field trigger=X` read
|
||||
// them. The reserved `convention` key (BUG-3163) is what the web reads,
|
||||
// through item.convention, which prefers it. Only the create door wrote the
|
||||
// reserved copy, so an edit of an ordinary key (`pad item update --field
|
||||
// trigger=on-commit`, measured) left it stale, and the conventions page went
|
||||
// on showing the old trigger while agents had moved on.
|
||||
//
|
||||
// The store now keeps the copy in step: when a write changes a mirrored
|
||||
// ordinary key on an item that HAS a reserved copy, the copy is rebuilt from
|
||||
// the merged fields in the same transaction. It runs after the BUG-3163
|
||||
// carry guard (the update's precheck), and rewrites only the reserved key
|
||||
// itself, so a caller still cannot set it directly. An item without a
|
||||
// reserved copy (a legacy convention, or anything that is not a convention)
|
||||
// is left exactly as it was.
|
||||
|
||||
// mirrorConventionMetadata returns `after` with its reserved `convention`
|
||||
// copy brought in line with the ordinary keys that changed since `before`.
|
||||
func mirrorConventionMetadata(before, after string) (string, error) {
|
||||
// UseNumber: the blob is re-encoded when the copy changes, and a float64
|
||||
// round trip would round any large integer elsewhere in it (BUG-3217).
|
||||
next, err := decodeNumbers(after)
|
||||
if err != nil {
|
||||
return after, nil // not ours to judge; the caller's own decode refuses it
|
||||
}
|
||||
rawCopy, ok := next[models.ItemFieldConvention].(map[string]any)
|
||||
if !ok {
|
||||
return after, nil // no reserved copy: nothing to keep in step
|
||||
}
|
||||
prev, err := decodeNumbers(before)
|
||||
if err != nil {
|
||||
prev = map[string]any{}
|
||||
}
|
||||
changed := func(key string) bool {
|
||||
_, in := next[key]
|
||||
return in && !reflect.DeepEqual(prev[key], next[key])
|
||||
}
|
||||
|
||||
b, _ := json.Marshal(rawCopy)
|
||||
var meta models.ItemConventionMetadata
|
||||
if err := json.Unmarshal(b, &meta); err != nil {
|
||||
return after, nil // an undecodable copy is BUG-3163's to refuse, not ours
|
||||
}
|
||||
|
||||
touched := false
|
||||
if changed("trigger") {
|
||||
if s, ok := next["trigger"].(string); ok {
|
||||
meta.Trigger, touched = s, true
|
||||
}
|
||||
}
|
||||
if changed("category") {
|
||||
if s, ok := next["category"].(string); ok {
|
||||
meta.Category, touched = s, true
|
||||
}
|
||||
}
|
||||
if changed("commands") {
|
||||
if list, ok := stringList(next["commands"]); ok {
|
||||
meta.Commands, touched = list, true
|
||||
}
|
||||
}
|
||||
if changed("surfaces") {
|
||||
if list, ok := stringList(next["surfaces"]); ok {
|
||||
meta.Surfaces, touched = list, true
|
||||
}
|
||||
} else if changed("scope") {
|
||||
if s, ok := next["scope"].(string); ok && s != "" {
|
||||
meta.Surfaces, touched = []string{s}, true
|
||||
}
|
||||
}
|
||||
if changed("enforcement") {
|
||||
if s, ok := next["enforcement"].(string); ok {
|
||||
meta.Enforcement, touched = s, true
|
||||
}
|
||||
} else if changed("priority") {
|
||||
if s, ok := next["priority"].(string); ok {
|
||||
meta.Enforcement, touched = s, true
|
||||
}
|
||||
}
|
||||
if !touched {
|
||||
return after, nil
|
||||
}
|
||||
|
||||
normalized, err := models.ValidateConventionMetadata(&meta)
|
||||
if err != nil {
|
||||
// A ValidationError: every door answers it 400 validation_error with
|
||||
// the Reason, and nothing is written.
|
||||
return "", &ValidationError{Reason: "this write would leave the convention's stored metadata invalid: " + err.Error()}
|
||||
}
|
||||
next[models.ItemFieldConvention] = normalized
|
||||
out, err := json.Marshal(next)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("marshal mirrored convention: %w", err)
|
||||
}
|
||||
return string(out), nil
|
||||
}
|
||||
|
||||
func decodeNumbers(blob string) (map[string]any, error) {
|
||||
dec := json.NewDecoder(strings.NewReader(blob))
|
||||
dec.UseNumber()
|
||||
var m map[string]any
|
||||
if err := dec.Decode(&m); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if m == nil {
|
||||
m = map[string]any{}
|
||||
}
|
||||
return m, nil
|
||||
}
|
||||
|
||||
func stringList(v any) ([]string, bool) {
|
||||
arr, ok := v.([]any)
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
out := make([]string, 0, len(arr))
|
||||
for _, e := range arr {
|
||||
s, ok := e.(string)
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
out = append(out, s)
|
||||
}
|
||||
return out, true
|
||||
}
|
||||
@@ -0,0 +1,77 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TASK-2257: the reserved convention copy follows the ordinary keys a write
|
||||
// changed.
|
||||
|
||||
func mirrorOf(t *testing.T, before, after string) map[string]any {
|
||||
t.Helper()
|
||||
out, err := mirrorConventionMetadata(before, after)
|
||||
if err != nil {
|
||||
t.Fatalf("mirror: %v", err)
|
||||
}
|
||||
var m map[string]any
|
||||
if err := json.Unmarshal([]byte(out), &m); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
c, _ := m["convention"].(map[string]any)
|
||||
return c
|
||||
}
|
||||
|
||||
const stored = `{"trigger":"always","scope":"all","priority":"should","enforcement":"should","convention":{"trigger":"always","surfaces":["all"],"enforcement":"should"}}`
|
||||
|
||||
func TestMirrorConvention_ChangedKeysFlowIntoTheCopy(t *testing.T) {
|
||||
after := `{"trigger":"on-commit","scope":"all","priority":"must","enforcement":"must","convention":{"trigger":"always","surfaces":["all"],"enforcement":"should"}}`
|
||||
c := mirrorOf(t, stored, after)
|
||||
if c["trigger"] != "on-commit" || c["enforcement"] != "must" {
|
||||
t.Fatalf("copy = %v, want trigger on-commit, enforcement must", c)
|
||||
}
|
||||
if s, _ := c["surfaces"].([]any); len(s) != 1 || s[0] != "all" {
|
||||
t.Fatalf("untouched surfaces changed: %v", c["surfaces"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestMirrorConvention_ScopeAndPriorityAreTheFallbackKeys(t *testing.T) {
|
||||
after := `{"trigger":"always","scope":"cli","priority":"nice-to-have","enforcement":"should","convention":{"trigger":"always","surfaces":["all"],"enforcement":"should"}}`
|
||||
c := mirrorOf(t, stored, after)
|
||||
if s, _ := c["surfaces"].([]any); len(s) != 1 || s[0] != "cli" {
|
||||
t.Fatalf("scope-only write: surfaces = %v, want [cli]", c["surfaces"])
|
||||
}
|
||||
if c["enforcement"] != "nice-to-have" {
|
||||
t.Fatalf("priority-only write: enforcement = %v, want nice-to-have", c["enforcement"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestMirrorConvention_NoCopyNoChange(t *testing.T) {
|
||||
before := `{"trigger":"always","n":12345678901234567890}`
|
||||
after := `{"trigger":"on-commit","n":12345678901234567890}`
|
||||
out, err := mirrorConventionMetadata(before, after)
|
||||
if err != nil || out != after {
|
||||
t.Fatalf("an item without a reserved copy must pass through byte for byte: %q, %v", out, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMirrorConvention_UnmirroredChangeLeavesBytesAlone(t *testing.T) {
|
||||
after := `{"trigger":"always","scope":"all","priority":"should","enforcement":"should","status":"draft","convention":{"trigger":"always","surfaces":["all"],"enforcement":"should"}}`
|
||||
out, err := mirrorConventionMetadata(stored, after)
|
||||
if err != nil || out != after {
|
||||
t.Fatalf("a write touching no mirrored key must not re-encode the blob: %q, %v", out, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMirrorConvention_KeepsLargeIntegersExact(t *testing.T) {
|
||||
before := `{"trigger":"always","n":12345678901234567890,"convention":{"trigger":"always"}}`
|
||||
after := `{"trigger":"on-commit","n":12345678901234567890,"convention":{"trigger":"always"}}`
|
||||
out, err := mirrorConventionMetadata(before, after)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if want := `12345678901234567890`; !strings.Contains(out, `"n":`+want) {
|
||||
t.Fatalf("re-encoding rounded a large integer: %s", out)
|
||||
}
|
||||
}
|
||||
@@ -345,10 +345,49 @@
|
||||
// current list row's, which a live refresh may have replaced while editing.
|
||||
let editBase: Pick<Item, 'content' | 'seq' | 'updated_at'> | null = null;
|
||||
|
||||
// TASK-2257 (C69): trigger, surface and enforcement were settable only at
|
||||
// creation; changing on-commit to on-pr-create meant delete and recreate.
|
||||
// The edit form carries them, and a change is sent as a plain fields_patch
|
||||
// of the ORDINARY keys (what agents obey). The server keeps the reserved
|
||||
// `convention` copy (what this page reads) in step in the same write.
|
||||
let editTrigger = $state('');
|
||||
let editSurface = $state('');
|
||||
let editEnforcement = $state('');
|
||||
let editMetaBase = $state({ trigger: '', surface: '', enforcement: '' });
|
||||
|
||||
function startEditing(item: Item) {
|
||||
editingSlug = item.slug;
|
||||
editContent = item.content ?? '';
|
||||
editBase = { content: item.content, seq: item.seq, updated_at: item.updated_at };
|
||||
const meta = getConvention(item);
|
||||
editMetaBase = {
|
||||
trigger: meta.trigger || 'always',
|
||||
surface: getPrimarySurface(item),
|
||||
enforcement: getEnforcement(item)
|
||||
};
|
||||
editTrigger = editMetaBase.trigger;
|
||||
editSurface = editMetaBase.surface;
|
||||
editEnforcement = editMetaBase.enforcement;
|
||||
}
|
||||
|
||||
// The options a select offers: the schema's, plus the stored value when the
|
||||
// schema no longer lists it, so opening the editor never changes a value.
|
||||
function withCurrent(options: readonly string[], current: string): string[] {
|
||||
return options.includes(current) || !current ? [...options] : [current, ...options];
|
||||
}
|
||||
|
||||
function metadataPatch(): Record<string, unknown> | null {
|
||||
const patch: Record<string, unknown> = {};
|
||||
if (editTrigger !== editMetaBase.trigger) patch.trigger = editTrigger;
|
||||
if (editSurface !== editMetaBase.surface) {
|
||||
patch.scope = editSurface;
|
||||
patch.surfaces = [editSurface];
|
||||
}
|
||||
if (editEnforcement !== editMetaBase.enforcement) {
|
||||
patch.enforcement = editEnforcement;
|
||||
patch.priority = editEnforcement;
|
||||
}
|
||||
return Object.keys(patch).length > 0 ? patch : null;
|
||||
}
|
||||
|
||||
async function saveEditing(item: Item) {
|
||||
@@ -358,11 +397,13 @@
|
||||
// BUG-3050 U1: the body goes only when it CHANGED, and then with the
|
||||
// token of the row the edit started from, so edits an open tab has not
|
||||
// stored yet are refused (409 content_pending_flush) rather than replaced.
|
||||
const write = contentWriteFor(editContent, editBase ?? item);
|
||||
if (!('content' in write)) {
|
||||
const contentWrite = contentWriteFor(editContent, editBase ?? item);
|
||||
const metaPatch = metadataPatch();
|
||||
if (!('content' in contentWrite) && !metaPatch) {
|
||||
editingSlug = null;
|
||||
return;
|
||||
}
|
||||
const write = metaPatch ? { ...contentWrite, fields_patch: metaPatch } : contentWrite;
|
||||
saving = true;
|
||||
try {
|
||||
let updated: Item;
|
||||
@@ -790,6 +831,33 @@
|
||||
{#if expanded}
|
||||
<div class="row-expanded">
|
||||
{#if editingSlug === item.slug}
|
||||
<div class="edit-meta">
|
||||
<label class="form-field">
|
||||
<span>Trigger</span>
|
||||
<select bind:value={editTrigger}>
|
||||
{#each withCurrent(createTriggers, editMetaBase.trigger) as t (t)}
|
||||
{@const meta = triggerMeta(t)}
|
||||
<option value={t}>{meta.icon} {meta.label}</option>
|
||||
{/each}
|
||||
</select>
|
||||
</label>
|
||||
<label class="form-field">
|
||||
<span>Surface</span>
|
||||
<select bind:value={editSurface}>
|
||||
{#each withCurrent(createSurfaces, editMetaBase.surface) as sv (sv)}
|
||||
<option value={sv}>{sv}</option>
|
||||
{/each}
|
||||
</select>
|
||||
</label>
|
||||
<label class="form-field">
|
||||
<span>Enforcement</span>
|
||||
<select bind:value={editEnforcement}>
|
||||
{#each withCurrent(ENFORCEMENT_LEVELS, editMetaBase.enforcement) as lv (lv)}
|
||||
<option value={lv}>{lv}</option>
|
||||
{/each}
|
||||
</select>
|
||||
</label>
|
||||
</div>
|
||||
<textarea
|
||||
class="edit-textarea"
|
||||
bind:value={editContent}
|
||||
@@ -839,6 +907,9 @@
|
||||
{#if workspaceStore.canEditItem(item)}
|
||||
<Button variant="secondary" size="sm" onclick={() => startEditing(item)}>Edit</Button>
|
||||
{/if}
|
||||
<!-- TASK-2257 (C69): the item's own page (history, comments,
|
||||
wiki-links) was unreachable from here. -->
|
||||
<a class="btn btn-secondary open-item" href="/{username}/{workspace}/{conventionsSlug}/{itemUrlId(item)}">Open item</a>
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="sm"
|
||||
@@ -883,6 +954,7 @@
|
||||
.btn { padding: var(--space-1) var(--space-4); border-radius: var(--radius); font-size: 0.85em; font-weight: 600; cursor: pointer; border: none; white-space: nowrap; text-decoration: none; display: inline-flex; align-items: center; }
|
||||
.btn-secondary { background: var(--bg-tertiary); color: var(--text-primary); border: 1px solid var(--border); }
|
||||
.btn-secondary:hover { background: var(--bg-hover); text-decoration: none; }
|
||||
.open-item { padding: 2px var(--space-3); font-size: 0.8em; font-weight: 500; }
|
||||
|
||||
/* Create form */
|
||||
.create-form { background: var(--bg-secondary); border: 1px solid var(--border); border-radius: var(--radius-lg); padding: var(--space-4); margin-bottom: var(--space-6); display: flex; flex-direction: column; gap: var(--space-3); }
|
||||
@@ -954,6 +1026,7 @@
|
||||
.command-list { display: flex; flex-direction: column; gap: var(--space-2); margin-bottom: var(--space-3); }
|
||||
.command-list code { display: inline-block; width: fit-content; max-width: 100%; padding: var(--space-1) var(--space-2); background: var(--bg-tertiary); border: 1px solid var(--border); border-radius: var(--radius); font-size: 0.8em; overflow-wrap: anywhere; }
|
||||
.expanded-actions { display: flex; gap: var(--space-2); align-items: center; }
|
||||
.edit-meta { display: flex; flex-wrap: wrap; gap: var(--space-2); margin-bottom: var(--space-2); }
|
||||
.edit-textarea { width: 100%; padding: var(--space-2) var(--space-3); background: var(--bg-tertiary); border: 1px solid var(--border); border-radius: var(--radius); color: var(--text-primary); font-size: 0.85em; font-family: inherit; line-height: 1.6; resize: vertical; margin-bottom: var(--space-3); box-sizing: border-box; }
|
||||
.edit-textarea:focus { border-color: var(--accent-blue); }
|
||||
.edit-hint { font-size: 0.75em; color: var(--text-muted); margin-left: auto; }
|
||||
|
||||
Reference in New Issue
Block a user