From 662bfbdbf9ba5eb1326f97b3be12548e454720df Mon Sep 17 00:00:00 2001 From: ZDDC Date: Thu, 21 May 2026 14:48:52 -0500 Subject: [PATCH] refactor(records): converge all record-write paths on WriteWithHistory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-dir form create/update (serveFormCreate/serveFormUpdate) wrote records with plain WriteAtomic + date+email naming — no audit stamping, no filename composition, no field_codes/folder_fields. So "+ Add row" from a per-party mdl/rsk table produced un-stamped, mis-named rows that the tables tool's own PUT-update path (which composes) would then 422 on. Only PUT and the project rollup honored the record machinery. Now every record-write entry point converges on WriteWithHistory: - Extract the shared field_defaults + folder_fields + row-assign + compose step into recordCreatePrep (history.go); the rollup uses it too, replacing its inline copy. - serveFormCreate: when a records: rule with a filename_format applies in the target dir, compose the name + route through WriteWithHistory; otherwise keep the generic date+email submission write. - serveFormUpdate: route through WriteWithHistory unconditionally — it stamps/historizes records and plain-writes non-records. Editing a tracking-number component in place now 422s (identity is the filename; renames are delete+create). - Drop originator from required: in the per-party mdl/rsk forms and mark it readOnly, matching the rollup forms — it's server-derived from the party folder, so a create needn't send it. Docs (AGENTS.md, ARCHITECTURE.md) updated for the converged wire surface. Tests: in-dir record create composes + stamps audit + folder-binds originator; in-dir update bumps revision and rejects an in-place component edit. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 3 +- ARCHITECTURE.md | 2 +- zddc/internal/handler/default-mdl.form.yaml | 7 +- zddc/internal/handler/default-rsk.form.yaml | 7 +- zddc/internal/handler/formhandler.go | 95 +++++++++++++++++---- zddc/internal/handler/history.go | 39 +++++++++ zddc/internal/handler/history_test.go | 75 ++++++++++++++++ zddc/internal/handler/ssrhandler.go | 38 +++------ 8 files changed, 217 insertions(+), 49 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cc8993b..d7f57da 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -439,8 +439,9 @@ Defaults are baked into `defaults.zddc.yaml`; `field_codes:` ships empty (every **Strip-and-stamp policy**: clients can't forge audit fields. `WriteWithHistory` strips all six keys from the incoming body BEFORE schema validation runs, then injects authoritative values from request context. A client that sends `created_by: eve@evil` finds it silently overwritten with the request principal. -**Wire surface**: +**Wire surface** — every record-write entry point converges on `WriteWithHistory`: - `PUT /.yaml` — routed through `WriteWithHistory` automatically when the basename matches a `records:` rule. Response echoes the stamped YAML as the body (Content-Type: application/yaml) so the tables client can update its row state without a re-GET. +- `POST //form.html` (in-dir create) and `POST //.yaml.html` (in-dir update), plus the project rollup `POST //(mdl|rsk)/form.html` — when a `records:` rule with a `filename_format` applies in the target directory, these compose the filename (shared `recordCreatePrep`: field_defaults + folder_fields + row-assign + compose), then route through `WriteWithHistory` for audit + history + the composed-name match check. So "+ Add row" from a per-party table no longer drops un-stamped, date+email-named rows. Directories with no record rule keep the generic date+email submission write. - `GET /.yaml?history=1` — JSON list of prior revisions: `[{revision, ts, by, sha, path}, …]`. ACL gates against the live record (read it → read its history). **Record-vs-config distinction**: `WriteWithHistory` fires only for genuine record paths. The gate (`isRecordPath` in `fileapi.go`) excludes `table.yaml`, `form.yaml`, `.zddc`, and the spec naming variants `*.table.yaml` / `*.form.yaml`. Those bypass audit stamping (they're configuration, not data) and go through plain `WriteAtomic`. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 0490724..aa3b181 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -470,7 +470,7 @@ app.state.subscribe((property, newValue) => { **Server-side counterpart:** `zddc/internal/handler/formhandler.go` recognizes `*.form.html` and `*.yaml.html` URLs, parses the spec, validates submissions via `zddc/internal/jsonschema/`, writes via `zddc.WriteAtomic` (plain submissions) or `zddc/internal/handler/history.go` `WriteWithHistory` (record-typed YAML — mdl rows, rsk rows, ssr.yaml). Existence of `.form.yaml` is the trigger; without it, the URL falls through to static-file serving. -**Record-vs-submission distinction.** "Records" are the three table-store types (mdl/rsk/ssr); everything else is a "submission." Records get server-stamped audit fields (`created_at`/`_by`, `updated_at`/`_by`, `revision`, `previous_sha`), an immutable per-record history at `/.history//-.`, cascade-driven filename composition (via the `records:` + `field_codes:` `.zddc` keys), and per-folder field locking (e.g. type=RSK in rsk/). The mechanism intercepts at the file-API write path (`serveFilePut`): if `isRecordPath` matches, the call routes through `WriteWithHistory`; otherwise the historical `WriteAtomic` path is used. See AGENTS.md "Records, audit, and history" for the operator surface; `zddc/internal/handler/history.go` for the orchestration. +**Record-vs-submission distinction.** "Records" are the three table-store types (mdl/rsk/ssr); everything else is a "submission." Records get server-stamped audit fields (`created_at`/`_by`, `updated_at`/`_by`, `revision`, `previous_sha`), an immutable per-record history at `/.history//-.`, cascade-driven filename composition (via the `records:` + `field_codes:` `.zddc` keys), per-folder field locking (e.g. type=RSK in rsk/), and folder-bound fields (`folder_fields`, e.g. originator = party-folder name). The mechanism intercepts at every write entry point — the file-API `serveFilePut` (if `isRecordPath` matches → `WriteWithHistory`, else `WriteAtomic`), the in-dir form create/update (`serveFormCreate`/`serveFormUpdate`), and the project rollup (`serveFormCreateRollup`). Each resolves the `records:` rule for the target directory and, when one with a `filename_format` applies, composes the name via the shared `recordCreatePrep` and routes through `WriteWithHistory`; non-record paths keep the historical date+email `WriteAtomic` write. The convergence means there's no back door that writes an un-stamped, un-composed record. See AGENTS.md "Records, audit, and history" for the operator surface; `zddc/internal/handler/history.go` for the orchestration. **Round-trip philosophy:** v0 is "form-as-truth" — submission YAML is regenerated from form state on every save. Hand-edits to submission files are not preserved across re-edit→re-submit. v1 will add an opt-in "file-as-truth" mode (eemeli/yaml Document API) for forms like `.zddc` itself where users hand-edit and comments must survive. diff --git a/zddc/internal/handler/default-mdl.form.yaml b/zddc/internal/handler/default-mdl.form.yaml index 8b38752..429019e 100644 --- a/zddc/internal/handler/default-mdl.form.yaml +++ b/zddc/internal/handler/default-mdl.form.yaml @@ -38,7 +38,10 @@ description: One planned or in-flight deliverable. The first fields are componen schema: type: object - required: [originator, project, discipline, type, sequence, title] + # originator is omitted from required: it's folder-bound — the server + # derives it from the party-folder name (folder_fields) and the form + # renders it read-only. + required: [project, discipline, type, sequence, title] additionalProperties: false properties: # --- Tracking-number components (matches the reference doc's @@ -52,7 +55,7 @@ schema: type: string title: Originator description: Bound to the party-folder name — the folder is the source of truth for the originator code. Server-set and read-only; you don't edit it here. - minLength: 1 + readOnly: true # phase: # project-wide; sits between originator and project # type: string # title: Phase diff --git a/zddc/internal/handler/default-rsk.form.yaml b/zddc/internal/handler/default-rsk.form.yaml index dd286e3..901ad0f 100644 --- a/zddc/internal/handler/default-rsk.form.yaml +++ b/zddc/internal/handler/default-rsk.form.yaml @@ -39,7 +39,10 @@ schema: # `type` is intentionally absent from required: — the cascade's # field_defaults inject type=RSK after schema validation, and the # form renderer surfaces it as a locked readOnly field. - required: [originator, project, discipline, sequence, title] + # originator is omitted from required: it's folder-bound — the server + # derives it from the party-folder name (folder_fields) and the form + # renders it read-only. + required: [project, discipline, sequence, title] additionalProperties: false properties: # --- Table-tracking components: identify which RSK deliverable @@ -49,7 +52,7 @@ schema: type: string title: Originator description: Bound to the party-folder name — the folder is the source of truth for the originator code. Server-set and read-only; you don't edit it here. - minLength: 1 + readOnly: true # phase: # project-wide; sits between originator and project # type: string # title: Phase diff --git a/zddc/internal/handler/formhandler.go b/zddc/internal/handler/formhandler.go index a4fd61b..1aa6624 100644 --- a/zddc/internal/handler/formhandler.go +++ b/zddc/internal/handler/formhandler.go @@ -408,23 +408,74 @@ func serveFormCreate(cfg config.Config, req *FormRequest, w http.ResponseWriter, return } - dateStr := time.Now().UTC().Format("2006-01-02") - emailSan := sanitizeEmail(email) - base := dateStr + "-" + emailSan - target, fname, ok := pickAvailableFilename(submissionsDir, base, ".yaml") - if !ok { - http.Error(w, "could not pick a free filename (>100 collisions)", http.StatusConflict) - return - } + var target, fname string - yamlBytes, err := yaml.Marshal(data) - if err != nil { - http.Error(w, "marshal yaml: "+err.Error(), http.StatusInternalServerError) + // Record path: when a records: rule with a filename_format applies + // in this directory, route the create through the same compose + + // folder_fields + audit + history machinery as a PUT or the project + // rollup — NOT the generic date+email submission write. This keeps + // in-dir "+ Add row" on a per-party mdl/rsk table consistent with + // every other record-write entry point (no un-stamped, un-composed + // rows leaking in through this door). + rowChain, perr := zddc.EffectivePolicy(cfg.Root, submissionsDir) + if perr != nil { + http.Error(w, "cascade resolve: "+perr.Error(), http.StatusInternalServerError) return } - if err := zddc.WriteAtomic(target, yamlBytes); err != nil { - http.Error(w, "write: "+err.Error(), http.StatusInternalServerError) - return + if _, rule, hasRule := rowChain.EffectiveRecordRule("placeholder.yaml"); hasRule && rule.FilenameFormat != "" { + dataMap, ok := data.(map[string]interface{}) + if !ok { + http.Error(w, "request body must be a YAML/JSON object", http.StatusBadRequest) + return + } + var composeErr *jsonschema.Error + fname, composeErr, err = recordCreatePrep(cfg, submissionsDir, rule, dataMap) + if err != nil { + http.Error(w, "record prep: "+err.Error(), http.StatusInternalServerError) + return + } + if composeErr != nil { + writeValidationErrors(w, []jsonschema.Error{*composeErr}) + return + } + target = filepath.Join(submissionsDir, fname) + if _, statErr := os.Stat(target); statErr == nil { + http.Error(w, "Conflict — a row with this composed tracking number already exists", http.StatusConflict) + return + } + yamlBytes, mErr := yaml.Marshal(dataMap) + if mErr != nil { + http.Error(w, "marshal yaml: "+mErr.Error(), http.StatusInternalServerError) + return + } + if _, verrs, herr := WriteWithHistory(cfg, target, req.SubmitURL, yamlBytes, email); herr != nil { + http.Error(w, "write: "+herr.Error(), http.StatusInternalServerError) + return + } else if len(verrs) > 0 { + writeValidationErrors(w, verrs) + return + } + } else { + // Generic submission: server-dated, email-tagged filename + a + // plain write (no audit stamping — these aren't records). + dateStr := time.Now().UTC().Format("2006-01-02") + emailSan := sanitizeEmail(email) + base := dateStr + "-" + emailSan + var ok bool + target, fname, ok = pickAvailableFilename(submissionsDir, base, ".yaml") + if !ok { + http.Error(w, "could not pick a free filename (>100 collisions)", http.StatusConflict) + return + } + yamlBytes, mErr := yaml.Marshal(data) + if mErr != nil { + http.Error(w, "marshal yaml: "+mErr.Error(), http.StatusInternalServerError) + return + } + if wErr := zddc.WriteAtomic(target, yamlBytes); wErr != nil { + http.Error(w, "write: "+wErr.Error(), http.StatusInternalServerError) + return + } } // Capability URL: the path to the new submission file. The renderer @@ -491,8 +542,20 @@ func serveFormUpdate(cfg config.Config, req *FormRequest, w http.ResponseWriter, http.Error(w, "marshal yaml: "+err.Error(), http.StatusInternalServerError) return } - if err := zddc.WriteAtomic(req.DataPath, yamlBytes); err != nil { - http.Error(w, "write: "+err.Error(), http.StatusInternalServerError) + // Route through WriteWithHistory: for record paths (matching a + // records: rule) this stamps audit fields, captures prior bytes into + // .history/, re-derives folder_fields, and enforces that the body + // still composes to the existing filename — a tracking-number + // component can't be edited in place (that's a delete + create). For + // non-record submissions WriteWithHistory falls through to a plain + // atomic write of the body. + _, verrs, herr := WriteWithHistory(cfg, req.DataPath, req.SubmitURL, yamlBytes, email) + if herr != nil { + http.Error(w, "write: "+herr.Error(), http.StatusInternalServerError) + return + } + if len(verrs) > 0 { + writeValidationErrors(w, verrs) return } diff --git a/zddc/internal/handler/history.go b/zddc/internal/handler/history.go index 15726a6..5320057 100644 --- a/zddc/internal/handler/history.go +++ b/zddc/internal/handler/history.go @@ -427,6 +427,45 @@ func applyFolderFields(rule zddc.RecordRule, recordDir, root string, bodyMap map return nil } +// recordCreatePrep applies a record rule's field_defaults + +// folder_fields and, for row-based rules, the auto-assigned row +// sequence to dataMap, then composes the row's filename. dir is the +// slot directory the new row will land in. The returned fname carries +// the .yaml extension. +// +// It centralizes the dataMap mutation + name composition shared by the +// in-dir form create (serveFormCreate) and the project rollup +// (serveFormCreateRollup). Callers still own collision-checking and the +// WriteWithHistory call (which re-applies these same steps as the +// authority and enforces the composed name matches the path). +// +// composeErr is a 422-worthy validation error (body can't compose a +// filename); err is a 500-worthy internal error (folder-field +// misconfig, row-assign failure). Only call when rule.FilenameFormat +// is non-empty. +func recordCreatePrep(cfg config.Config, dir string, rule zddc.RecordRule, dataMap map[string]any) (fname string, composeErr *jsonschema.Error, err error) { + for k, want := range rule.FieldDefaults { + if _, present := dataMap[k]; !present { + dataMap[k] = want + } + } + if ferr := applyFolderFields(rule, dir, cfg.Root, dataMap); ferr != nil { + return "", nil, ferr + } + if rule.RowField != "" { + rowVal, rerr := AssignNextRow(dir, rule.RowField, rule.RowScopeFields, dataMap) + if rerr != nil { + return "", nil, rerr + } + dataMap[rule.RowField] = rowVal + } + composed, cerr := composeFilename(rule.FilenameFormat, dataMap) + if cerr != nil { + return "", &jsonschema.Error{Path: "/", Message: cerr.Error()}, nil + } + return composed + ".yaml", nil, nil +} + // AssignNextRow finds the next free row sequence within the // row-scope group identified by scopeFields. Used by POST-create // handlers (rsk row creation) before invoking WriteWithHistory. diff --git a/zddc/internal/handler/history_test.go b/zddc/internal/handler/history_test.go index 19a262f..673b10b 100644 --- a/zddc/internal/handler/history_test.go +++ b/zddc/internal/handler/history_test.go @@ -451,6 +451,81 @@ func TestRollupCreate_AssignsRowAndComposesFilename(t *testing.T) { // RecognizeFormRequest → ServeForm (the rollup/SSR create entry // point). Lets the history tests share the same harness without // pulling in the full ssrTestSetup helper. +// TestInDirCreate_RecordComposesAndStampsAudit: "+ Add row" on a +// per-party mdl table posts to the in-dir form.html create endpoint +// (serveFormCreate). Convergence requires it to compose the +// tracking-number filename, fold in the folder-bound originator, and +// stamp audit fields — i.e. behave like the rollup / PUT, NOT drop a +// date+email submission file. +func TestInDirCreate_RecordComposesAndStampsAudit(t *testing.T) { + cfg, _ := historyTestSetup(t) + // originator is omitted on purpose — it's folder-bound to ACM. + body := `{"project":"PRJ","discipline":"EL","type":"SPC","sequence":"0001","title":"Switchgear spec"}` + rec := doForm(t, cfg, "POST", "/Project/archive/ACM/mdl/form.html", "alice@example.com", body) + if rec.Code != http.StatusCreated { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + loc := rec.Result().Header.Get("Location") + if !strings.Contains(loc, "ACM-PRJ-EL-SPC-0001.yaml") { + t.Errorf("location=%q want composed ACM-PRJ-EL-SPC-0001.yaml (not a date+email name)", loc) + } + abs := filepath.Join(cfg.Root, "Project", "archive", "ACM", "mdl", "ACM-PRJ-EL-SPC-0001.yaml") + disk, err := os.ReadFile(abs) + if err != nil { + t.Fatalf("read disk: %v", err) + } + out := map[string]any{} + if err := yaml.Unmarshal(disk, &out); err != nil { + t.Fatal(err) + } + if out["originator"] != "ACM" { + t.Errorf("originator=%v want ACM (folder-bound)", out["originator"]) + } + if out["created_by"] != "alice@example.com" || out["revision"] != 1 { + t.Errorf("audit not stamped on in-dir create: %+v", out) + } +} + +// TestInDirUpdate_RecordStampsAuditAndRejectsRename: the in-dir +// form.html update endpoint (serveFormUpdate) must route records +// through WriteWithHistory — incrementing revision and refusing an +// in-place tracking-number change (identity is the filename). +func TestInDirUpdate_RecordStampsAuditAndRejectsRename(t *testing.T) { + cfg, do := historyTestSetup(t) + url := "/Project/archive/ACM/mdl/ACM-PRJ-EL-SPC-0001.yaml" + seed := []byte("originator: ACM\nproject: PRJ\ndiscipline: EL\ntype: SPC\nsequence: '0001'\ntitle: V1\n") + if rec := do(http.MethodPut, url, "alice@example.com", seed, nil); rec.Code != http.StatusCreated { + t.Fatalf("seed status=%d body=%s", rec.Code, rec.Body.String()) + } + + // Same components, new title → revision bumps to 2 (proves the form + // update went through WriteWithHistory, not a plain WriteAtomic). + upd := `{"originator":"ACM","project":"PRJ","discipline":"EL","type":"SPC","sequence":"0001","title":"V2"}` + rec := doForm(t, cfg, "POST", "/Project/archive/ACM/mdl/ACM-PRJ-EL-SPC-0001.yaml.html", "bob@example.com", upd) + if rec.Code != http.StatusOK { + t.Fatalf("update status=%d body=%s", rec.Code, rec.Body.String()) + } + disk, _ := os.ReadFile(filepath.Join(cfg.Root, "Project", "archive", "ACM", "mdl", "ACM-PRJ-EL-SPC-0001.yaml")) + out := map[string]any{} + if err := yaml.Unmarshal(disk, &out); err != nil { + t.Fatal(err) + } + if out["revision"] != 2 { + t.Errorf("revision=%v want 2 (form update must route through WriteWithHistory)", out["revision"]) + } + if out["updated_by"] != "bob@example.com" { + t.Errorf("updated_by=%v want bob", out["updated_by"]) + } + + // Editing a tracking-number component in place → 422 (composed name + // would differ from the file's name). + rename := `{"originator":"ACM","project":"PRJ","discipline":"EL","type":"SPC","sequence":"0099","title":"V3"}` + rec = doForm(t, cfg, "POST", "/Project/archive/ACM/mdl/ACM-PRJ-EL-SPC-0001.yaml.html", "bob@example.com", rename) + if rec.Code != http.StatusUnprocessableEntity { + t.Fatalf("expected 422 for in-place component edit, got %d body=%s", rec.Code, rec.Body.String()) + } +} + func doForm(t *testing.T, cfg config.Config, method, target, email, body string) *httptest.ResponseRecorder { t.Helper() req := httptest.NewRequest(method, target, bytes.NewReader([]byte(body))) diff --git a/zddc/internal/handler/ssrhandler.go b/zddc/internal/handler/ssrhandler.go index d4ba591..1bd6a98 100644 --- a/zddc/internal/handler/ssrhandler.go +++ b/zddc/internal/handler/ssrhandler.go @@ -306,37 +306,21 @@ func serveFormCreateRollup(cfg config.Config, req *FormRequest, w http.ResponseW var target, fname string if hasRule && rule.FilenameFormat != "" { - // Apply field_defaults (e.g. type=RSK for rsk/). - for k, want := range rule.FieldDefaults { - if _, present := dataMap[k]; !present { - dataMap[k] = want - } - } - // Bind folder-derived fields (e.g. originator = party-folder - // name) BEFORE composing the filename so the composed name and - // the value WriteWithHistory will re-derive agree. slotAbs is - // /, so originator: 1 resolves to the party folder. - if ferr := applyFolderFields(rule, slotAbs, cfg.Root, dataMap); ferr != nil { - auditFile(r, "rollup-create", req.SubmitURL, http.StatusInternalServerError, 0, ferr) - http.Error(w, "folder fields: "+ferr.Error(), http.StatusInternalServerError) + // Shared record-create prep: field_defaults + folder_fields + // (originator = party folder, since slotAbs is /) + // + per-row sequence + filename composition. WriteWithHistory + // below re-applies these as the authority. + var composeErr *jsonschema.Error + fname, composeErr, err = recordCreatePrep(cfg, slotAbs, rule, dataMap) + if err != nil { + auditFile(r, "rollup-create", req.SubmitURL, http.StatusInternalServerError, 0, err) + http.Error(w, "record prep: "+err.Error(), http.StatusInternalServerError) return } - // Auto-assign the per-row sequence for RSK-style rules. - if rule.RowField != "" { - rowVal, rerr := AssignNextRow(slotAbs, rule.RowField, rule.RowScopeFields, dataMap) - if rerr != nil { - auditFile(r, "rollup-create", req.SubmitURL, http.StatusInternalServerError, 0, rerr) - http.Error(w, "row assign: "+rerr.Error(), http.StatusInternalServerError) - return - } - dataMap[rule.RowField] = rowVal - } - composed, cerr := composeFilename(rule.FilenameFormat, dataMap) - if cerr != nil { - writeValidationErrors(w, []jsonschema.Error{{Path: "/", Message: cerr.Error()}}) + if composeErr != nil { + writeValidationErrors(w, []jsonschema.Error{*composeErr}) return } - fname = composed + ".yaml" target = filepath.Join(slotAbs, fname) if _, err := os.Stat(target); err == nil { http.Error(w, "Conflict — a row with this composed tracking number already exists", http.StatusConflict)