[judy] feat(refs): editable .md refs + ETag + danger banner + paper-trail#48
Merged
Conversation
…em-post paper trail PRD: alice/PRD.md v1.1; Design: designer/design-v0.1.md Thread: https://github.com/SymbolStar/OpenForge (th_19e6d36bf3b_7be92c) Backend (forge_refs.py / server.py): - .md refs are now editable regardless of writable flag (per PRD). - GET /api/refs/<id>/content returns ETag (sha256(content)[:16]) in both 'ETag' and 'X-Ref-Etag' headers. - PUT /api/refs/<id>/content now expects JSON body {content, actor?, thread_id?} and honours 'If-Match' header. - 409 returns {current_etag, current_content} so the UI can decide. - atomic write (tmp + fsync + os.replace), diff (+N / -M) returned. - if thread_id supplied: appends a system 'paper-trail' post by speaker='__editor__' to the thread (bypasses post router to avoid fan-out on @mentions inside the file). - Non-.md still 403 (read-only) unless ref.writable=True (legacy). - New RefConflictError class. Frontend (web/app.js): - Ref viewer wires Edit tab for .md (greyed for non-md). - Tracks state.refEtag across GET/PUT; sends If-Match. - 409 -> confirm() with both etags; choose 'force' or 'discard mine'. - Danger banner above textarea for STATUS/MEMORY/AGENTS/SOUL/IDENTITY/USER.md (basename match); not blocking, only warns. Resolves agent id from workspace-<agent> in abs_path. - IME guard: Cmd/Ctrl+S during composition does NOT trigger Save. Tests: - Replaced legacy 'requires writable' test with .md-always-editable + non-md-still-readonly + etag-conflict (unit + HTTP).
🤖 bot-review (comment-only · phase 1)Diff: Red-line checks:
Needs human review — these paths are not eligible for future auto-approve:
Phase 1: this bot leaves comments only. Auto-approve will be enabled per-path after 1–2 weeks of clean runs. Promotion plan: judy PR #42 follow-up. |
Codex review (PR #48) flagged two 🔴 data-corruption risks + one 🔴 semantics drift. Fixed all three: 1. **Frontend stale ref state** (web/app.js): selectFile() now clears state.currentRef / refEtag / isMdRef. Without this, opening a .md ref then switching to a workspace file would make saveCurrent() wrongly route to saveRef() and PUT workspace content into the old ref id. 2. **Backend tmp-file collision + ETag TOCTOU** (forge_refs.py): The atomic write used a fixed '<name>.tmp' that two concurrent PUTs in the same dir could clobber, AND the If-Match check was not under any lock — two writers holding the same ETag could both pass the check and the loser silently overwrote. Now: - per-ref in-process Lock wraps re-read + check + replace - tmp name is unique (pid + 4 bytes random) and opened O_EXCL - file-size re-check after lock (avoid OOM on external growth) - best-effort parent-dir fsync for durable rename - tmp cleanup on failure 3. **Non-md semantics** (forge_refs.py + tests): Removed the dead 'non-md still honours writable' branch. v1 PRD is .md only — both writable=False and writable=True non-md now reject as RefValidationError (HTTP 400). Test updated. Other nits from codex review picked up: - Danger-banner regex now matches workspace-<agent> at path-segment boundary, includes '.' (mirrors server agent-id allowlist). - Cmd/Ctrl+S in edit view now ALWAYS preventDefault (so browser 'save page' never escapes), then skips on IME composition. - New unit + HTTP tests assert: exact sha256[:16] ETag, file unchanged on conflict, quoted If-Match strip works, writable=True non-md still rejected. All 511 tests green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements PRD v1.1 + design v0.1 from thread
th_19e6d36bf3b_7be92c.Scope (per Alice PRD / Judy tech plan / Dora design):
.mdrefs only — all editable, no per-file writable flag needed.sha256(content)[:16]. BothETagandX-Ref-Etagheaders.PUT /api/refs/<id>/contentbody{content, actor?, thread_id?}+If-Matchheader.{current_etag, current_content}for 3-way UI decision.thread_idis supplied, post✏️ <actor> 于 HH:MM 编辑了 <label>（+N / -M 行）as__editor__. Router is bypassed (no @mention fan-out from inside the file body).workspace-<agent>segment.Out of scope (PRD non-goals, design checklist): CodeMirror, rich text, collab cursors, version history, diff view, autosave. v1 is textarea + Save.
Tests: 510/510 green. Replaced
requires_writablewith md-always-editable + non-md-still-readonly + etag-conflict (unit + HTTP).