mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
b2c303c4bb
Go comments describe the web renderer constantly and cite it by LINE NUMBER. Nothing verifies those citations — they cross a language boundary, so no compiler, test or linter has ever checked one — and they had drifted onto unrelated code. This converts all 32 to `markdown.ts::symbolName` form and adds the check that makes the conversion worth something. Scope note, because this is wider than the rider it was dispatched as. The BUG-2834 commit added the pattern constant near the top of markdown.ts, shifting the file by +45 lines and invalidating EVERY line citation into it — including the three BUG-2832 had confirmed were still accurate. Leaving 13 knowingly-wrong citations because they sit outside the files this unit otherwise touched is not the neutral option when this branch is what broke them. Happy to split this commit back out if the lead would rather hold the rider to its stated bound. While converting, five of the filing's six "suspect, not established" citations were settled by reading the shifted positions: :307, :478-481, :485, :513 and :516 point at a @param doc line, unescapeDocLinks, REF_PATTERN, the tail of parseCrossWorkspaceBody, and findItemByRef respectively. All substantively stale, not merely off-by-lines. That answers the filing's open question. Two guard tests, per the filing's own proposed fix shape: TestMarkdownCitationsNameLiveSymbols verifies every cited symbol is really declared in markdown.ts. This is the check a line number could never have. TestMarkdownCitationsAreNotLineNumbers bans the line-number form, so the fix cannot erode the next time someone reads a number off their editor gutter. The first version of the symbol check FAILED its negative control and that is the part worth reading. It asked strings.Contains(ts, "function "+sym) — a PREFIX match. Renaming resolveWikiBody to resolveWikiBodyRENAMED leaves "function resolveWikiBody" a substring of the renamed declaration, so the guard stayed green through precisely the rename it exists to catch. It passed its first real run and would have shipped as coverage. Fixed by requiring the following character to be one that cannot continue a JS identifier; the control now fires and names the symbol. Both guards are non-vacuity-asserted: the sweep fails if it finds fewer than 50 Go files, and the symbol check fails if it finds no citations at all. Currently verifying 7 distinct symbols across 29 citation sites. The line-number guard earned its keep before being committed — it caught three citations silently reverted when a file was restored from a snapshot taken before the conversion.
212 lines
8.4 KiB
Go
212 lines
8.4 KiB
Go
package models
|
|
|
|
import (
|
|
"fmt"
|
|
"strings"
|
|
"time"
|
|
"unicode/utf8"
|
|
)
|
|
|
|
// Valid document types
|
|
var ValidDocTypes = []string{
|
|
"roadmap", "plan", "architecture", "ideation",
|
|
"feature-spec", "notes", "prompt-library", "reference",
|
|
}
|
|
|
|
// Valid document statuses
|
|
var ValidStatuses = []string{
|
|
"draft", "active", "completed", "archived",
|
|
}
|
|
|
|
// Valid actors
|
|
var ValidActors = []string{"user", "agent"}
|
|
|
|
// Valid sources
|
|
var ValidSources = []string{"cli", "web", "skill"}
|
|
|
|
type Document struct {
|
|
ID string `json:"id"`
|
|
WorkspaceID string `json:"workspace_id"`
|
|
Title string `json:"title"`
|
|
Slug string `json:"slug"`
|
|
Content string `json:"content"`
|
|
DocType string `json:"doc_type"`
|
|
Status string `json:"status"`
|
|
Tags string `json:"tags"` // JSON array
|
|
Pinned bool `json:"pinned"`
|
|
SortOrder int `json:"sort_order"`
|
|
CreatedBy string `json:"created_by"`
|
|
LastModifiedBy string `json:"last_modified_by"`
|
|
Source string `json:"source"`
|
|
CreatedAt time.Time `json:"created_at"`
|
|
UpdatedAt time.Time `json:"updated_at"`
|
|
DeletedAt *time.Time `json:"deleted_at,omitempty"`
|
|
}
|
|
|
|
type DocumentCreate struct {
|
|
Title string `json:"title"`
|
|
Content string `json:"content,omitempty"`
|
|
DocType string `json:"doc_type,omitempty"`
|
|
Status string `json:"status,omitempty"`
|
|
Tags string `json:"tags,omitempty"`
|
|
Pinned bool `json:"pinned,omitempty"`
|
|
CreatedBy string `json:"created_by,omitempty"`
|
|
Source string `json:"source,omitempty"`
|
|
}
|
|
|
|
type DocumentUpdate struct {
|
|
Title *string `json:"title,omitempty"`
|
|
Content *string `json:"content,omitempty"`
|
|
DocType *string `json:"doc_type,omitempty"`
|
|
Status *string `json:"status,omitempty"`
|
|
Tags *string `json:"tags,omitempty"`
|
|
Pinned *bool `json:"pinned,omitempty"`
|
|
SortOrder *int `json:"sort_order,omitempty"`
|
|
LastModifiedBy string `json:"last_modified_by,omitempty"`
|
|
Source string `json:"source,omitempty"`
|
|
ChangeSummary string `json:"change_summary,omitempty"`
|
|
}
|
|
|
|
type DocumentListParams struct {
|
|
Type string
|
|
Status string
|
|
Tag string
|
|
Pinned *bool
|
|
Query string
|
|
Sort string
|
|
Order string
|
|
}
|
|
|
|
// MaxDocumentTitleRunes bounds a document title at write time.
|
|
//
|
|
// Two reasons, and the second is the load-bearing one:
|
|
//
|
|
// - A title is emitted into every linking document by the rename cascade
|
|
// (`[[newTitle]]` per occurrence), so its length is an amplification
|
|
// factor on a body the renamer does not supply. Unbounded, one rename
|
|
// could project 10 GB from a 500 KB input — measured, 20,000x (BUG-2798).
|
|
// - 255 is the conventional identifier-ish bound and comfortably above any
|
|
// real title; the longest document title this codebase seeds is far short
|
|
// of it.
|
|
//
|
|
// RUNES, not bytes, because "255 characters" is what a user and a UI counter
|
|
// mean. That makes the byte-level residual up to 4x this number, which is
|
|
// precisely why the title bound is the cheap door and NOT the wall: the
|
|
// cascade's own guard (store.MaxRenameCascadeRetainedBytes) is byte-accurate
|
|
// and is what actually bounds the work.
|
|
//
|
|
// Enforced at WRITE time only. Titles already over the bound stay valid and
|
|
// keep working until their next rename — no retro-breakage of stored data
|
|
// (Dave's ruling, day-63).
|
|
const MaxDocumentTitleRunes = 255
|
|
|
|
// wikiTitleRoundTripFailure reports why emitting `[[title]]` the way
|
|
// links.ReplaceTitle does — plain concatenation, no escaping — would produce a
|
|
// bracket that does not read back as this title. Returns "" when the title
|
|
// survives the round trip.
|
|
//
|
|
// This is BUG-2796 stated as a property rather than as a character blacklist,
|
|
// and the distinction is not cosmetic: the first version of this function
|
|
// banned `]`, `\` and `|` on the reasoning that all three "look like
|
|
// wiki-link syntax", and the test below refuted two thirds of that. The rule
|
|
// is therefore derived from the two mechanisms that actually consume a stored
|
|
// bracket, both in web/src/lib/utils/markdown.ts:
|
|
//
|
|
// 1. THE GRAMMAR (markdown.ts::WIKI_LINK_PATTERN_SOURCE, shared by renderMarkdown and
|
|
// wikiLinksToMarkdown since BUG-1744): a bracket body is
|
|
// `(?:\\.|[^\]\\])+` — escape pairs, or anything that is neither `]` nor
|
|
// `\`. A raw `]` ends the bracket early, which IS BUG-2796's defect:
|
|
// renaming to `A]] [[A` emitted `[[A]] [[A]]`, two brackets, neither
|
|
// resolving to the renamed document, and the rename reported success. A
|
|
// title ending in `\` fails the same way — the trailing backslash pairs
|
|
// with the first `]` of the terminator and the bracket never closes.
|
|
//
|
|
// 2. THE UNESCAPER (markdown.ts::unescapeWikiBody, `\\(\\|\]|\|)` → `$1`): resolution
|
|
// unescapes the body before comparing it to a title, and the editor may
|
|
// therefore STORE a link in escaped form. That matters twice over: a
|
|
// title containing `\\` or `\|` emitted raw comes back as a different
|
|
// string, AND a link stored as `[[Alpha\|Beta]]` is invisible to the
|
|
// rename cascade, which searches for the raw `[[Alpha|Beta]]` only.
|
|
//
|
|
// The second half is why `|` and a lone `\` are refused (codex round 9),
|
|
// having been ALLOWED in the first version of this validator. The property
|
|
// tested there was "does the renderer read this back as the same title", and
|
|
// both characters pass it. That was the wrong property: a title also has to be
|
|
// one whose links the cascade can FIND, or a rename silently leaves them
|
|
// pointing at a name that no longer exists. The stricter property is the one
|
|
// that matters, and it is the same mistake as validating against the renderer
|
|
// while the cascade used an unescaped LIKE — checking the layer that displays
|
|
// a title instead of the layer that has to maintain it.
|
|
//
|
|
// Deliberately NOT rejected, because the code these titles pass through
|
|
// handles them and refusing them would be a validator inventing a defect:
|
|
//
|
|
// - `[` — the grammar excludes only `]` and `\`, so `[[A[B]]` carries the
|
|
// body `A[B` intact, and the cascade's search term matches it literally.
|
|
// BUG-2796's filing proposed rejecting "`[[` or `]]`"; measured against
|
|
// the grammar, the `[[` half of that is overreach.
|
|
//
|
|
// Boundary, stated rather than papered over: this is derived from the SHARED
|
|
// stored-syntax path in markdown.ts. The legacy documents surface has no
|
|
// renderer of its own that I could locate — every wiki-link consumer found
|
|
// routes through these two functions — so the rule is pinned to them.
|
|
func wikiTitleRoundTripFailure(title string) string {
|
|
if strings.Contains(title, "]") {
|
|
return `Title may not contain "]" — it would end the [[wiki-links]] that point at this document early, ` +
|
|
`turning them into broken links`
|
|
}
|
|
if strings.HasSuffix(title, `\`) {
|
|
return `Title may not end with "\" — it would escape the closing bracket of the [[wiki-links]] that point ` +
|
|
`at this document`
|
|
}
|
|
if strings.ContainsAny(title, `\|`) {
|
|
return `Title may not contain "\" or "|" — a link to a title containing them can be stored in ` +
|
|
`escaped form, which the rename cascade would not find, silently leaving those links pointing ` +
|
|
`at a title that no longer exists`
|
|
}
|
|
return ""
|
|
}
|
|
|
|
// ValidateDocumentTitle checks a document title at write time. Returns a
|
|
// message suitable for a 400 response, or "" when the title is acceptable.
|
|
//
|
|
// Covers BUG-2798 (length, which is an amplification factor on OTHER
|
|
// documents' bodies, not merely a field-size preference) and BUG-2796
|
|
// (wiki-link syntax the cascade emits raw). Both are write-time doors on the
|
|
// same field, which is why they share one validator and one insertion point.
|
|
func ValidateDocumentTitle(title string) string {
|
|
if title == "" {
|
|
return "Title is required"
|
|
}
|
|
if n := utf8.RuneCountInString(title); n > MaxDocumentTitleRunes {
|
|
return fmt.Sprintf("Title is too long: %d characters, maximum %d", n, MaxDocumentTitleRunes)
|
|
}
|
|
return wikiTitleRoundTripFailure(title)
|
|
}
|
|
|
|
func IsValidDocType(t string) bool {
|
|
for _, v := range ValidDocTypes {
|
|
if v == t {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
func IsValidStatus(s string) bool {
|
|
for _, v := range ValidStatuses {
|
|
if v == s {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
func IsValidActor(a string) bool {
|
|
return a == "user" || a == "agent"
|
|
}
|
|
|
|
func IsValidSource(s string) bool {
|
|
return s == "cli" || s == "web" || s == "skill"
|
|
}
|