450 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
xarmian	e6fd25322e	feat(cmdhelp): dynamic enum resolution + workspace context (TASK-936) (#329) ... * feat(cmdhelp): dynamic enum resolution + workspace context (TASK-936) The killer differentiator from the cmdhelp v0.1 spec — splice live workspace facts into help output so an LLM asking "what collections exist?" gets the real answer rather than a generic "any string". Files: - internal/cmdhelp/dynamic.go (new) — Resolver type with Apply method. ArgEnumSources / FlagEnumSources map names to enum_source identifiers; Sources maps enum_source to a fetcher func. Apply walks the Document, stamps enum_source on matching args/flags, populates Enum from the fetcher, and sets doc.Context.Workspace. Per-Apply caching keeps each source func to ≤1 invocation regardless of how many commands need it. - internal/cmdhelp/json.go — added Options.Resolver; Build calls Resolver.Apply after the static walk, so callers that inspect Build output as either pre- or post-resolution still work. - cmd/pad/help_cmdhelp.go — newDynamicResolver constructs a Resolver bound to the runtime: workspace from DetectWorkspace, server URL from config, three sources (collections, roles, members). Returns nil when no workspace is detected so help still works outside any workspace. cmdhelpOptions takes target so Binary derives from root.Name() instead of hardcoded "pad" (preserves test-tree bindings for synthetic roots). Pad-side bindings (matches `pad item create --help`'s existing context): arg collection → dynamic:pad collection list flag role → dynamic:pad role list flag assign → dynamic:pad workspace members End-to-end on the real binary (inside docapp workspace): pad help item create --format json → args[0].collection: type=enum, enum=[ideas,conventions,...,roadmap], enum_source="dynamic:pad collection list" → flags.role: enum=[planner,implementer,reviewer] → flags.assign: enum=[dave] → context.workspace="docapp" pad help --format md → "## Workspace context\n- workspace: `docapp`" Outside any workspace (cd /tmp; pad help item create --format json): → collection arg: type=string, no enum, no enum_source (graceful fallback) → context: null → output still validates against schema/cmdhelp.schema.json Fail-safe behavior: - newDynamicResolver returns nil on any config/detection error → static doc. - Per-source fetcher errors are caught inside Apply → enum_source still announced on the binding arg/flag, but Enum is left empty. The help command MUST NOT fail because dynamic facts can't be fetched. - Existing Enum values from alternation/ValidArgs are preserved (resolver only fills the gap, never overwrites authoritative spec). Tests: - 10 dynamic-resolver tests in internal/cmdhelp/dynamic_test.go covering: arg + flag enum population, context population, per-source caching across multiple commands, graceful error handling, nil resolver as no-op, existing-Enum preservation, global flag resolution, unaffected commands left unchanged, end-to-end via Build(). - All 24 prior cmdhelp tests + 12 routing tests still green. - make check clean (lint + go test + web build). Out of scope (deferred): - --capabilities discovery flag — TASK-937. - Schema-validate live output in CI — TASK-938. - Audit pad's existing commands' Examples — TASK-939. Parent: PLAN-930. * fix(cmdhelp): scope --role / --assign bindings per-command per Codex review (round 1) Codex round 1 on PR #329 caught a real semantic bug: globally binding --role to dynamic:pad role list was wrong because pad has two unrelated --role flags: pad workspace invite --role workspace role: owner|editor|viewer pad item create --role <slug> agent role slug pad item update --role <slug> agent role slug Globally announcing agent-role slugs as the values for `pad workspace invite --role` would mislead consumers (LLMs would suggest "planner" when "owner" is expected; tab-completion would offer the wrong set). Fix: - Resolver gains CommandArgBindings and CommandFlagBindings (map[path]map[name]source) — scoped to a specific command path. Per-command bindings win over wildcard ArgEnumSources/FlagEnumSources when both match. - Helper methods argSource(path,name) / flagSource(path,name) own the precedence rule so both args and flags use it consistently. - newDynamicResolver in cmd/pad keeps `<collection>` as a wildcard ArgEnumSources (universal — every <collection> in pad means a pad collection), but moves --role and --assign into CommandFlagBindings scoped to "item create", "item update", and "item list". `pad workspace invite --role` is intentionally left without a binding. - A header comment in newDynamicResolver enumerates every --role / --assign site in the CLI and which one each binding targets, so a future reviewer adding a new flag can see the rule at a glance. End-to-end on the real binary: pad help item create --format json → flags.role: type=enum, enum=[planner,implementer,reviewer], enum_source=... pad help workspace invite --format json → flags.role: type=string (untouched). ✓ New tests: - TestResolver_Apply_PerCommandBindingScoped — explicitly mirrors the Codex finding: two commands both have a `role` flag, only the bound command resolves. workspace-invite-style isolation regression test. - TestResolver_Apply_PerCommandWinsOverWildcard — precedence: when both wildcard and per-command match, per-command wins. - TestResolver_Apply_PerCommandArgBindings — same precedence rule for positional args. All 33 cmdhelp tests + 12 routing tests still green; make check clean. * fix(cmdhelp): bind item list --role to agent roles per Codex review (round 2) Codex round 2 caught that item list --role was still unbound — I missed it in round 1's grep because the variable name is `&roleFilter` rather than `&roleFlag`. Pad has 4 --role flags total: pad workspace invite --role workspace role (NOT bound) pad item create --role agent role slug (bound) pad item update --role agent role slug (bound) pad item list --role agent role filter (now bound) Fix: extend CommandFlagBindings["item list"] to include the same itemRoleAssign map as item create/update, so all three item subcommands that reference an agent role get the dynamic binding. Added a `grep` recipe in the comment so a future maintainer adding a new --role / --assign site can find every existing one in one shot (both `&roleFlag` and `&roleFilter` style declarations). End-to-end on real binary (inside docapp workspace): pad help item list --format json → flags.role: enum=[planner,implementer,reviewer], enum_source set ✓ → flags.assign: enum=[dave], enum_source set ✓ make check clean.	2026-05-01 01:27:06 -04:00
xarmian	5e93abe552	feat(cmdhelp): implement --format md emitter (TASK-935) (#328) ... Adds internal/cmdhelp/md.go that renders the Document built in TASK-934 as markdown with the predictable section order from cmdhelp v0.1 §6. Replaces the markdown stub in cmd/pad/help_cmdhelp.go so `pad help --format md` (and the `--llm` alias) produce real output. Section order per command (spec §6): ## `binary path` summary / description (when distinct from summary) ### Synopsis — fenced usage line, reconstructed from args + flags ### Arguments — table with name | type | required | description ### Flags — table with flag | type | default | description ### Stdin — when Stdin.Accepted is true ### Examples — fenced bash blocks, drawn from same canonical example set as JSON (spec §6 same-source rule) ### Output — text_template + json_schema_ref when populated ### Exit codes — table when ExitCodes is populated ### See also — bullet list of related command paths Top-level YAML frontmatter: cmdhelp_version, binary, version, generated (RFC3339, UTC). Now is overridable via Options.Now for snapshot-test stability. Top-level structure: `# binary` heading, summary, optional homepage, optional `## Workspace context` (populated in TASK-936), `## Global flags` table, then per-command sections sorted by path for determinism. Synopsis reconstruction uses the structured Args from Build() (rather than cobra.UseLine) so JSON and MD stay driven by the same parsed data — the variadic `<ref>...` and alternation enums from TASK-934 carry through naturally. Pipes in flag/arg descriptions are escaped (`\|`) so they don't break markdown table grids. cmd/pad/help_cmdhelp.go: emitCmdhelpMarkdown stub replaced with a call into cmdhelp.EmitMarkdown. --depth/--all threaded through MaxDepth identically to the JSON path. Tests: - 15 markdown emitter tests in internal/cmdhelp/md_test.go covering frontmatter (presence + timestamp injectability), per-command section order, synopsis reconstruction (incl. variadic + alternation), global-flag dedup, fenced-bash examples, hidden-thing exclusion, deterministic ordering, Stdin/Output/ExitCodes/SeeAlso sections, Workspace context, table-pipe escaping. - TestHelpCmd_FormatMarkdownStubError replaced by TestHelpCmd_FormatMarkdownEmits (asserts frontmatter + structural markers for both md and llm). - TestHelpCmd_FormatLLMAliasRoutesToMarkdown replaced by TestHelpCmd_FormatLLMIsAliasForMD (asserts md and llm produce byte-identical output modulo the timestamp). End-to-end on the real binary: - pad help --format md emits valid markdown with all sections. - pad help --format llm produces byte-identical output (after timestamp normalization). - pad help item create --format md scopes correctly. - make check clean. Out of scope (deferred): - Dynamic Workspace context population — TASK-936. - --capabilities discovery flag — TASK-937. - Schema-validation + golden-file tests in CI — TASK-938. - Examples populated for all pad commands (still in cobra Long for now) — TASK-939. Parent: PLAN-930.	2026-05-01 01:05:30 -04:00
xarmian	eecc683ab0	feat(cmdhelp): implement --format json emitter (TASK-934) (#327) ... * feat(cmdhelp): implement --format json emitter (TASK-934) Adds internal/cmdhelp package that walks the cobra command tree and emits a cmdhelp v0.1 Document conforming to schema/cmdhelp.schema.json. Wires it into cmd/pad/help_cmdhelp.go so `pad help --format json` is no longer a stub. Files: - internal/cmdhelp/types.go — Document/Command/Arg/Flag/Stdin/Stdout/ ExitCode/Example structs mirroring the schema. ExitCode implements custom MarshalJSON for the string-or-object union (spec §5.2). - internal/cmdhelp/json.go — Build() walks target's subtree; EmitJSON() serializes to indented JSON. Type mapping covers pflag's full type space, including slice/array→repeatable. Hidden commands and flags filtered. Cobra's auto-installed --help flag suppressed. Zero-default values suppressed to keep output compact. argRE parses positional arg placeholders from cobra Use strings, filtering [flags]/[options]/ [command] cobra conventions. parseExamples splits cmd.Example by newline, drops blanks and # comments. MaxDepth maps to spec §4 semantics: 0 = subcommand list, 1 = + grandchildren, -1 = unlimited. - cmd/pad/help_cmdhelp.go — replaces emitCmdhelpJSON stub with a call into the package; threads --depth and --all through MaxDepth (--all overrides --depth). Verification: - 17 emitter tests in internal/cmdhelp/json_test.go covering envelope, global-flag emission, hidden-thing exclusion, positional arg parsing, pflag type mapping, zero-default suppression, example parsing, description-vs-summary, command-path key shape, MaxDepth semantics, target-subtree scoping, JSON validity, version pattern, ExitCode union marshaling, parseExamples filtering. - 11 cmd/pad routing tests still pass; TestHelpCmd_FormatJSONStubError replaced by TestHelpCmd_FormatJSONEmits which validates the structure. - End-to-end: `pad help --format json` on the real binary emits 100 commands across the full tree; output validates against schema/cmdhelp.schema.json (verified with python jsonschema). - `pad help item --format json` correctly limits output to 29 commands in the item subtree (homepage and other top-level metadata still populated from root). - `pad help --format json --depth 0` correctly emits 15 immediate children of root, no grandchildren. - make check clean (lint + go test + web build). Out of scope (deferred): - Examples: pad's existing commands embed examples in Long rather than using cobra's Example field. The emitter correctly reads Example; TASK-939 will normalize the pad-side commands to populate it. - Dynamic enum injection (workspace-aware enums): TASK-936. - --capabilities discovery flag: TASK-937. - Schema-validation of live output in CI: TASK-938. Parent: PLAN-930. * fix(cmdhelp): handle alternation, variadic, and ValidArgs in Use parser per Codex review (round 1) Codex round 1 on PR #327 flagged that parseArgs missed two real cobra Use-string idioms in pad's command tree: 1. `completion [bash|zsh|fish|powershell]` — alternation in brackets. The old regex only allowed `[a-zA-Z0-9_./-]+` inside brackets, so the `|` made the whole token unmatched and the shell arg disappeared from the emitted JSON. Consumers asking "what does completion take?" got nothing. 2. `item bulk-update [--status X] <ref>...` — variadic ellipsis. Old regex didn't capture trailing `...`, so the arg was emitted but without `repeatable: true`. Consumers couldn't tell that <ref> may be passed multiple times. Fixes: - argRE now allows full-bracket content (`[^<>]+` / `[^\[\]]+`) and captures an optional trailing `...` group. - parseArgs takes *cobra.Command (not just Use string) so it can read cmd.ValidArgs and attach those values as the first arg's enum when set. This covers `Use: "completion [shell]"` + `ValidArgs: [...]` where the allowed values only live on the cobra struct. - Alternation `<a|b|c>` / `[a|b|c]` produces an enum-typed arg with the values as `Enum`. When Use carries no semantic name (only the alternation), the arg name is synthesized as "value". - New validArgName check rejects embedded flag-like fragments such as `[--status X]` and prose with whitespace/punctuation that the broader regex would otherwise capture from idiosyncratic Use strings. - ValidArgs entries strip cobra's tab-separated completion descriptions before becoming enum values. New tests: - TestBuild_VariadicArgsRepeatable — `<ref>...` → repeatable=true. - TestBuild_AlternationProducesEnum — `[bash|zsh|fish|powershell]` → enum with values in source order. - TestBuild_ValidArgsFillsEnumOnNamedArg — Use says `[shell]`, ValidArgs carries the values → enum-typed arg named `shell`. - TestBuild_EmbeddedFlagFragmentsFiltered — `[--status X]` does not leak as a positional arg. Verified on the real binary: - `pad help completion --format json` now emits the shell enum. - `pad help item bulk-update --format json` now marks <ref> repeatable. - `pad help --format json` still validates against the schema (100 cmds). - `make check` clean.	2026-05-01 00:56:50 -04:00
xarmian	e2b19d393f	feat(cli): wire pad help subcommand + scope levels (TASK-933) (#326) ... Replaces cobra's built-in `help` with a custom subcommand implementing the cmdhelp v0.1 mandatory surface (https://getpad.dev/cmdhelp; IDEA-927): pad help [subcommand…] [--format <fmt>] [--depth <n>] [--all] Routing: - --format text (default): delegates to cobra's text renderer so the existing --help UX is byte-for-byte unchanged. - --format json|md|llm: routes to cmdhelp emitters. JSON and markdown emitters are stubs that return clear "not yet implemented" errors pointing to TASK-934 / TASK-935; this lets the routing layer ship and be tested independently of the emitter work. - --depth <n> and --all are accepted at this layer; their effect lives in the JSON/MD emitters. - llm is a renderer-level alias for md per spec §3. Scope levels (spec §4): - pad help → root tree summary (text mode delegates to cobra) - pad help <group> → subtree (e.g. `pad help item`) - pad help <group> <leaf> → single command, deep Tests (cmd/pad/help_cmdhelp_test.go): - 11 cases covering all routing branches: default text, group scope, leaf scope, --format text == default, --format json stub references TASK-934, --format md/llm stubs reference TASK-935, llm aliases md, unknown --format rejected, unknown topic rejected, --depth/--all accepted, CmdhelpVersion is MAJOR.MINOR. Verification: - All 11 new tests pass. - `make check` clean (lint + go test + web build). - Manual smoke-test on built binary: 13 paths exercised — group/leaf routing, format aliases, error paths, existing `pad <cmd> --help` unchanged. The JSON/MD emitters land in TASK-934/935. --capabilities discovery is a separate concern (TASK-937). Parent: PLAN-930.	2026-05-01 00:37:11 -04:00
xarmian	1e3991c865	feat(schema): publish cmdhelp.schema.json v0.1 (TASK-932) (#325) ... * feat(schema): publish cmdhelp.schema.json v0.1 (TASK-932) Adds the formal JSON Schema (draft 2020-12) describing cmdhelp v0.1's `--format json` wire format, plus a schema/README.md documenting intended use for CLI authors and consumer wrappers. Per IDEA-927 the schema enforces the v0.1 contracts: - Required top-level: cmdhelp_version, binary, commands. - cmdhelp_version pattern is MAJOR.MINOR (no PATCH); '0.1.0' invalid. - Argument types: closed set {string,int,float,bool,enum,path,url, duration,date,datetime,json,ref} plus x-* extension namespace. - Boolean flag arity (§5.3): negate_flag only valid when type=bool. - exit_codes union (§5.2): each entry is string OR object{when,recovery, message_template}; object form requires `when`. Codes must be numeric. - Dynamic enums (§7): enum_source pattern is `^dynamic:.+$`. - additionalProperties: true at extension points for forward-compat (§9). Validated locally with python jsonschema: 8 tests (1 valid sample, 5 negative cases, 2 x-*/dynamic positive cases) all pass. Unblocks TASK-934 (JSON emitter), TASK-938 (test suite validates against this schema), and TASK-940 (publish on getpad.dev). Parent: PLAN-930. * fix(schema): enforce flag-name pattern + clarify test-suite wording per Codex review (round 1) Two findings from Codex round 1 on PR #325: 1. flagMap accepted invalid keys ('--verbose', empty string, digit-leading) despite README documenting "without leading --". Added propertyNames pattern `^[a-zA-Z][a-zA-Z0-9_-]*$` so producers can't ship malformed flag names that consumers would parse incorrectly. 2. schema/README.md described `internal/cmdhelp/` as if it existed and already validated in `go test ./...`. The package ships in TASK-934 (and the validation test in TASK-938). Reworded as future/planned. Verification: - Schema syntax: still valid Draft 2020-12. - Original 8 tests still pass. - 3 new negative tests for the propertyNames pattern: '--verbose', empty string, and digit-leading keys all correctly rejected. - Single-letter short-flag keys like `h` still accepted. - `make check` clean. Note for next review round: Codex's first run reported a `httptest` panic in `cmd/pad` tests; that's a sandbox networking constraint (read-only mode can't bind sockets), not a regression — verified locally with `go test ./cmd/pad/ -run TestEnsureWorkspaceSlugAttachExisting -count=1` (passes).	2026-05-01 00:30:53 -04:00
xarmian	c4f7d243e6	fix(billing): gate Pro upgrade CTAs while Stripe is unwired (#324) ... Stripe isn't configured on the cloud sidecar yet, so the "Upgrade to Pro" buttons on /console/billing dead-end at a 404 from /billing/checkout. Hide the Current-Plan CTA and replace the Compare-Plans CTA with a "Pro — coming soon" block plus a mailto:info@getpad.dev link to capture interest while we get the integration ready. The post-checkout polling/banners are left wired — they only fire on ?checkout=success, which can't happen until the gate flips back on. Flip the STRIPE_AVAILABLE constant (or thread it through a server flag like billing_enabled) once Stripe is live to restore the buttons.	2026-04-30 19:57:19 -04:00
xarmian	b783d06144	feat(web): last-used auth method banner on /login (TASK-923) (#323) ... Returning users who are logged out land on /login with no context about how they signed in before. This adds a soft "last time, you used X to sign in" hint and visually elevates the matching CTA so the right next step reads at a glance — without overwhelming first-time visitors who still see all methods equally. Implementation -------------- - New helper `web/src/lib/auth/lastMethod.ts` reads/writes a `pad_last_auth_method` value (`'password' | 'github' | 'google'`) and a `pad_last_auth_at` timestamp in localStorage. Wrapped in try/catch so SSR, private mode, and disabled storage never break auth pages. - Login page records `password` on successful credential or 2FA login, and records the OAuth provider speculatively on button click. The OAuth handshake completes outside the SPA (provider → pad-cloud → pad backend session → redirect), so there's no JS callback to hang the write on. If the user bails at the consent screen the value still reflects "what the user tried last", which is the right answer for the next-visit banner. - Register page records `password` on successful registration so newly registered users see the same hint when they next return logged out. - Banner above the form names the method; matching OAuth button gets a border lift + "Last used" pill. Banner is suppressed when an OAuth error banner for the same provider is already showing — surfacing both at once muddles the message. Privacy ------- - Only the method *name* is stored — never an email, user ID, or token. - localStorage is per-origin and never sent over the wire. - No cookie, no URL param, no server log entry, no new endpoints. Parent: PLAN-776 (Post-launch Backlog). Promotes IDEA-922.	2026-04-30 17:14:56 -04:00
xarmian	21e8ca6a20	chore(make): add make check mirroring CI lint + test + web build (IDEA-921) (#322) ... * chore(make): add `make check` mirroring CI lint + test + web build (IDEA-921) Closes the local-vs-CI gap that let PR #321 ship a trivial gofmt violation past every step of CONVE-190's pre-flight (`go build && go test && cd web && npm run build`). - `make lint` now runs the same golangci-lint v2.11.4 suite CI runs (govet, ineffassign, staticcheck SA*, unused, plus the gofmt formatter with simplify: true). The bootstrap rule auto-installs the pinned binary into $(go env GOPATH)/bin on first run, so contributors don't need a separate setup step. - `make check` is a new umbrella target that runs lint, the Go test suite, and the web build — the exact set of jobs CI's "Go" and "Web" jobs run. Run it before pushing. - `make install` is unchanged (build + restart) so the inner dev loop stays fast. `check` is the opt-in pre-push gate. CONVE-190 updated separately via the Pad CLI to point contributors at `make check` instead of the old three-command list. Verified locally: `make check` passes on a clean tree (after a one- time `golangci-lint cache clean` to clear stale entries from a prior run — that's a known golangci-lint quirk, not a workflow bug). * fix(make): enforce lint version pin + cover full CI surface (round 1) Codex review on PR #322 round 1 surfaced two real gaps in the make check / make lint plumbing. 1. Makefile:83 — `lint` did not actually enforce GOLANGCI_LINT_VERSION. The previous file-target dependency only fired the install rule when the binary was missing, so an older or newer locally-installed golangci-lint was silently reused, defeating the pin. The recipe now compares the installed version against the pin and reinstalls on mismatch. 2. Makefile:97 — `check` claimed to mirror CI's Go and Web jobs but omitted Web's `npm run check` (svelte-check type checking) and the Go job's `govulncheck` step. CI could fail on either while local `make check` passed. Added new `vuln` (pinned to GOVULNCHECK_VERSION = v1.2.0, matches CI) and `web-check` targets, both wired into `make check`. `make check` now runs: lint + go test + govulncheck + npm ci + npm audit + npm run build + svelte-check — exactly mirroring the gates the CI Go and Web jobs use to fail a PR. The race-detector and PostgreSQL jobs only run on push to main and are intentionally not part of `make check` (run `make test-pg` separately if needed). Verified locally: `make check` exits 0; `make lint` correctly no-ops when the pinned version is already installed.	2026-04-30 16:32:40 -04:00
xarmian	5e27989ab8	feat(attachment): add pad attachment view|show|list CLI surfaces (IDEA-898) (#321) ... * feat(attachment): add `pad attachment view|show|list` CLI surfaces (IDEA-898) Agents and CLI users had no first-class way to fetch attachment bytes through the API: the only path to read an `` reference was to read the raw blob out of `~/.pad/attachments/<storage_key>`, which bypasses workspace ACLs, doesn't work on Pad Cloud / remote / Postgres deployments, skips the variant pipeline (TASK-872 / TASK-879 / TASK-880), and breaks when storage moves to S3. Three new subcommands wrap the existing REST endpoints: - `pad attachment view <id> [-o path]` — agent-friendly: with no `-o`, fetches to a fresh OS temp directory using the stored filename and prints just the absolute path on stdout (so `$(pad attachment view <id>)` composes cleanly into shell pipelines). Reuses `download`'s atomic temp-then-rename pattern via a shared helper. - `pad attachment show <id>` — HEAD-based metadata only; surfaces MIME, size, filename, ETag, Last-Modified. - `pad attachment list [--item REF] [--category X] [--attached|--unattached] [--collection ID] [--sort ...] [--limit N] [--offset N]` — workspace list. The `--item REF` flag resolves a TASK-5-style ref to a UUID client-side and passes it to a new `item_id` query param on the list endpoint (server side: AttachmentListFilters.ItemID, ~6 lines in the store + 1 in the handler). Skill update: `skills/pad/SKILL.md` gains a "Working with attachments" subsection plus a CLI Reference entry, both ending in the hard rule that agents must NEVER read directly from `~/.pad/attachments/`. * style(cli): gofmt AttachmentListParams field alignment CI's golangci-lint v2 flagged this with the gofmt formatter (configured with simplify: true in .golangci.yml). The contiguous Sort/Limit/Offset block at the end of the struct needs uniform column alignment — gofmt considers the doc comment above Sort attached to that field rather than a block separator, so the three int/string fields get aligned together. Verified locally with `golangci-lint run --timeout=5m ./...` (v2.11.4 to match CI) — 0 issues. Local make lint only runs `go vet ./...` and `golangci-lint` wasn't installed, which is why this slipped through; filing a separate follow-up to mirror the CI checks in the local workflow.	2026-04-30 16:06:08 -04:00
xarmian	f8ed3e10a7	fix(search): explicit selection on Enter + numeric go-to (BUG-864, BUG-910) (#320) ... * fix(search): require explicit selection on Enter; add bare-number go-to (BUG-864, BUG-910) The command palette had two related issues: - BUG-864: Pressing Enter armed the first search result automatically — the user could close the modal and navigate without ever pressing an arrow key. selectedIdx now starts at -1 and only advances on ArrowDown/ArrowUp. - BUG-910: Typing a bare number (e.g. "843") returned no results because parseItemRef requires PREFIX-NUMBER and FTS doesn't index item_number. Backend (internal/store): - Add parseItemNumber() helper alongside parseItemRef. - In Search(), add a bare-numeric direct-lookup path that mirrors the existing ref-lookup block but without a collection prefix filter. item_number is unique per workspace (idx_items_workspace_number) so this resolves to at most one direct hit, prepended with rank=-1000. Frontend (CommandPalette.svelte): - selectedIdx defaults to -1; reset to -1 (not 0) on modal open and after every search. - Enter on a non-numeric query is a no-op unless the user has arrow-selected. - Numeric queries are a deliberate exception: Enter on a bare-number query flushes the debounce, navigates directly to the matching item, and lets the search palette double as a quick "go to item N" jump. Tests: - TestSearch_BareNumericQueryFindsItemByNumber covers the new path. - TestParseItemNumber covers helper edge cases. * fix(search): exclude direct hits from FTS WHERE to keep pagination correct Codex review (round 1) on PR #320: > Numeric direct hits are appended before the FTS query, but the later > pagination only removes duplicates after SQL LIMIT/OFFSET. If item #2 > also matches FTS for query "2" through its title/content, that > duplicate consumes an FTS slot, so page 1 can return fewer than `limit` > results and later pages can repeat/skip rows. Hoist the direct-hit (ref + numeric) snapshot to before the FTS query is built, then append `AND i.id NOT IN (...)` to both the SELECT and COUNT FTS queries. After a successful count, add refCount back so SearchResponse.Total still reflects the full result set (since FTS itself no longer counts those rows). The flaw also applied to the pre-existing parseItemRef path; this fix covers both. The post-LIMIT dedup loop is now defense-in-depth. New test TestSearch_BareNumericQueryDedupsAgainstFTS guards the case: an item whose title/content literally contains its own item_number (so it matches both the direct lookup and FTS) appears exactly once in Results and Total counts it exactly once. * fix(search): paginate direct hits properly across workspaces Codex review (round 2) on PR #320: > P1: Bare numeric direct hits break pagination in global search. > item_number is only unique per workspace, so q=1 with WorkspaceIDs > spanning N workspaces returns N direct hits — all appended without > being sliced to Limit. limit=1 with three workspaces each having #1 > returns three results on page 0, and offset=1 drops all direct hits > then returns FTS rows instead of the second direct hit. The same flaw applied to the pre-existing parseItemRef path: the global search "TASK-5" can match TASK-5 in multiple workspaces. Fix: - Add deterministic ORDER BY i.workspace_id, i.id to both ref and bare- numeric direct-hit lookups so pagination is stable across pages. - Replace the offset==0/offset>0 branching pagination with a uniform slice: directStart = min(Offset, refCount); directEnd = min(Offset+Limit, refCount); results = results[directStart:directEnd]; ftsLimit = Limit - directConsumed; ftsOffset = max(Offset - refCount, 0). This honours (offset, limit) whether direct hits, FTS, or both fill the page. Total stays correct because the FTS count was already excluding direct hits (round-1 fix) and we add refCount back unconditionally. New test TestSearch_BareNumericQueryPaginatesAcrossWorkspaces creates three workspaces each with item #1 and verifies that limit=1 with offsets 0/1/2 returns three different direct hits in stable order, and limit=10 returns all three. * chore: gofmt — column alignment in struct field declarations CI Go (SQLite) lint failed on two files: - internal/store/store_test.go (TestParseItemNumber, this PR's new test) — unaligned column widths and inconsistent comment spacing. - internal/config/config.go (drive-by) — pre-existing alignment regression in the Config struct that snuck in via an earlier landed PR; included here because it blocks merge. No semantic changes — `gofmt -w` only.	2026-04-30 14:23:23 -04:00
xarmian	94ebe5a83d	test(screenshots): capture in dark mode (Pad's default theme) (#319) ... The README screenshot capture script ran in light mode because Playwright's headless Chromium reports prefers-color-scheme: light by default. The Pad layout's onMount logic explicitly forces data-theme="light" when matchMedia matches 'light' — so the captures came out light-themed even though Pad defaults to dark when no user preference exists. Two effects made this misleading: 1. README screenshots showed a theme most Pad users never see by default. The first impression in the README didn't match the first impression of the running app. 2. The screenshots could not be reused in the getpad.dev marketing site (dark themed) without visible whiplash. TASK-918 (PLAN-911) needs them on the homepage; light-mode captures would have looked like screenshots of some other product. Fix: pass colorScheme: 'dark' via test.use(). Chromium then reports prefers-color-scheme: dark to the page; the layout's matchMedia check no longer matches 'light', so it leaves the document on the default theme — which is dark. Also fixed a typo in the re-run instruction in the docstring (the PAD_SCREENSHOTS=1 env var was attached to the wrong command). Re-captured all three screenshots (dashboard, board, list) under the new config. Docstring updated to call out the theme rationale so future maintainers don't accidentally flip it back.	2026-04-30 13:10:51 -04:00
xarmian	10309fc599	fix(config): read PUBLIC_URL for emailed link generation (BUG-899) (#318) ... * fix(config): read PUBLIC_URL for emailed link generation (BUG-899) The Pad Cloud deployment binds pad to 0.0.0.0 (Dockerfile, k8s configmap, pad-cloud's docker-compose) and never set PAD_URL on the pad service, so cfg.BaseURL() fell through to "http://0.0.0.0:7777" — that string ended up in password-reset (and invite + share-link + admin-invitation) emails and was unreachable to recipients. Adds a PUBLIC_URL env var read by the server only (does not flip CLI to remote mode the way PAD_URL does — PUBLIC_URL is a generic env var name commonly set in unrelated deployment contexts). Stored in a separate Config.PublicURL field consulted by BaseURL() as a fallback after URL. Resolution order in BaseURL(): PAD_URL > PUBLIC_URL > host:port. Also logs a WARN at server startup if the resolved base URL has an unspecified bind-all host (0.0.0.0, ::, [::]) — a backstop that would have caught BUG-899 the first time email went out. Tests cover the precedence ladder, mode-not-flipping, PAD_URL-beats- PUBLIC_URL, and the BUG-899 repro shape (Host=0.0.0.0 with no URL set yields the broken http://0.0.0.0 URL). Companion change in pad-cloud/docker-compose.yml passes PUBLIC_URL through to the pad service so the Cloud deployment stops shipping broken email links. Parent: BUG-899 (TASK-908). * fix(config): keep PUBLIC_URL out of IsConfigured() per Codex review (round 2) PUBLIC_URL was setting LoadedFromEnv = true, which IsConfigured() consults to decide whether the CLI has explicit configuration. A generic PUBLIC_URL in the environment (very common name) would have made any host appear "configured" to the CLI and skipped the not-configured / setup branch — the exact footgun the separate-field design was supposed to avoid. PUBLIC_URL is purely a server-side fact; LoadedFromEnv is purely a CLI affordance. Stop conflating them. Adds a focused regression test pinning the IsConfigured() invariant. * fix(config): split PublicLinkBaseURL from BaseURL per Codex review (round 3) Round 2's BaseURL fall-through to PublicURL leaked PUBLIC_URL into ~20 CLI-client call sites (cli.NewClientFromURL(cfg.BaseURL()) patterns across cmd/pad/main.go, init.go, server_info.go, configure.go) — same footgun the separate-field design was meant to avoid: a developer with a host-level PUBLIC_URL set for unrelated reasons would have their CLI silently route requests to that URL instead of the local server. Restore BaseURL() to its original CLI-only contract (URL > host:port). Add PublicLinkBaseURL() with the URL > PublicURL > host:port ladder that's used at exactly the two server-side call sites that build emailed-link targets: - cmd/pad/main.go:279 srv.SetBaseURL(cfg.PublicLinkBaseURL()) - cmd/pad/main.go:464 email.NewSender(..., cfg.PublicLinkBaseURL()) Tests pin both contracts: BaseURL() ignores PublicURL even when set; PublicLinkBaseURL() honors the precedence ladder. PAD_URL still wins in both, preserving back-compat. * fix(config): drop public_url toml tag to prevent CLI persistence per Codex review (round 4) Round 3 left PublicURL serializable to ~/.pad/config.toml via toml: "public_url". A CLI user who runs `pad init` or `pad configure` on a host where PUBLIC_URL is set for unrelated reasons would end up with that URL persisted into their config file, surviving any later unset of the env var and contaminating server-side emailed link generation indefinitely (server reads ~/.pad/config.toml on the next boot). Switch the field to toml:"-". PUBLIC_URL is a deployment-time fact (env var / docker-compose / k8s); operators who want a config-file equivalent already have `url` (the PAD_URL path), which serializes properly. Adds a regression test pinning that Save() never writes PublicURL to the file.	2026-04-30 08:40:33 -04:00
xarmian	cec056cefe	feat(email): cloud-mode marketing footer in transactional emails (TASK-907) (#317) ... * feat(email): cloud-mode marketing footer in transactional emails (TASK-907) Extracts a shared HTML/plain shell helper for the five existing transactional-email templates (SendInvitation, SendWelcome, SendPasswordReset, SendPaymentFailed, SendTest) and adds a Cloud-only marketing footer that mirrors the auth-page AuthFooter component: GitHub / Docs / Changelog / Privacy / Terms link list plus a "© <year> Pad · Perpetual Software" copyright line. Self-hosted output (the default for any pad instance NOT in PAD_CLOUD/PAD_MODE=cloud) is byte-equivalent to the prior inline templates: same wordmark header, same body, same footer-note disclosure, no marketing links. Operators ship Pad under their own brand and getpad.dev's link list would be wrong on their notifications. Plumbing: - email.Sender gains a cloudMode bool + SetCloudMode/CloudMode accessors. Configure() does not touch cloudMode (it's set independently from API-key/from-addr config). - Server.SetCloudMode now propagates to s.email.SetCloudMode(true) so existing senders pick up the flag. - Server.SetEmailSender propagates s.cloudMode → e.cloudMode when email is wired AFTER cloud mode (handles the cmd/pad/main.go ordering where SetEmailSender is called from main). - Server.reconfigureEmail() (admin-settings reload path) does the same so an admin reconfiguring email mid-flight doesn't end up with a sender stuck in self-hosted mode. The email accent color (#2563eb) is preserved from the prior templates — it has known contrast properties on white email backgrounds. Email is light-themed for cross-client readability; the dark-theme tokens from docs/brand.md §3 are for in-app/auth surfaces, not transactional mail. Pinned with three regression tests: - self-hosted shell renders no Cloud-only markers - Cloud shell renders the link list in canonical order (GitHub → Docs → Changelog → Privacy → Terms) - plain-text shell branches identically Visual contract: docs/brand.md §7 (link order) and §6 (Pad wordmark). Companion to AuthHeader, AuthFooter, +error.svelte, and UserMenuResources already shipped on PLAN-900. Test plan: - go build ./... — clean - go vet ./... — clean - go test ./... — all pass (including new shell_test.go cases) - web/npm run check — 0 errors - web/npm run build — clean * fix(email): full canonical link list per Codex (round 2) Codex caught that the Cloud-mode email footer carried only 5 of the 9 canonical links from docs/brand.md §7 (GitHub / Docs / Changelog / Privacy / Terms — omitted Contribute / FAQ / Security / Sub-processors). The brand spec §1 says transactional emails get "Full parity" with the auth-page AuthFooter; my trim violated that contract. Add the four missing links to both the HTML and plain-text shells in the canonical order: GitHub → Docs → Changelog → Contribute → FAQ → Security → Privacy → Terms → Sub-processors. Update the regression tests to pin all 9 markers + their pairwise ordering. The "keep emails small" instinct that motivated the trim was a real design concern but not strong enough to defy the brand spec. If we later decide email needs a reduced subset, the right move is to update §7 in docs/brand.md FIRST (acknowledging email as a surface with a smaller link list) and trim the implementation to match.	2026-04-30 00:09:52 -04:00
xarmian	f122bec84a	feat(layout): in-app Resources menu in user dropdown (TASK-905) (#316) ... * feat(layout): in-app Resources menu in user dropdown (TASK-905) New UserMenuResources component adds a Resources block to the user-menu dropdown in TopBar, closing the product → marketing handoff seam. Logged-in users now have a clear path back out to Docs / Changelog / GitHub / Status / Support without having to remember getpad.dev URLs or visit the marketing site separately. Cloud-mode (cloudMode=true) shows: Docs / Changelog / GitHub / Status / Support. Replaces the prior inline Support/Status pair — that block became a special case of this unified Resources component. Self-hosted (cloudMode=false) shows the trimmed Docs / GitHub set. Changelog / Status are Cloud-specific surfaces; getpad.dev's support@getpad.dev mailbox isn't the operator's to direct people to. The Docs link still points at getpad.dev because that's the canonical project documentation regardless of deployment shape. Component is wired into BOTH the desktop and mobile branches of TopBar (the existing dropdown duplication). All links open in a new tab so a user mid-task doesn't lose state. Each entry has a small external-link icon so the off-property nature is visible without the user having to hover-and-read the title. The `:global(.user-dropdown)` selectors keep the new styles scoped to the existing dropdown surface in TopBar without forcing a CSS refactor of that component. Visual contract: docs/brand.md §6/§7. Companion to AuthHeader, AuthFooter, and +error.svelte from PLAN-900. Test plan: - web/npm run check — 0 errors (694 files, +1 new component) - web/npm run build — clean - Svelte autofixer — clean * fix(layout): UserMenuResources mirrors dropdown-item styles per Codex (round 2) Codex caught that .dropdown-item and .dropdown-divider rules in TopBar.svelte's <style> are scoped to that component — Svelte's scoped CSS attaches a per-component hash so the rules don't apply to DOM rendered by UserMenuResources.svelte (a separate component). The new resource links lost the dropdown padding/color/text-decoration/ hover styling, and the divider rendered as an unstyled empty 1px row. Mirror the base .dropdown-item / .dropdown-divider / .dropdown-item:hover rules inside UserMenuResources using :global(.user-dropdown) qualifiers so the dropdown surface remains the styling boundary — the rules apply to anything dropped into the menu but never leak outside it. Same scoping pattern that already worked for .resources-label and .external-icon in this component, just extended to the base classes. * fix(layout): respect canonical link order from brand spec per Codex (round 3) Codex caught that UserMenuResources rendered links in the order Docs / Changelog / GitHub / Status / Support, but docs/brand.md §7 defines a canonical relative order with GitHub before Docs and Changelog. The whole point of the brand spec is one canonical order across surfaces; violating it in the user menu undermines that. Reorder Cloud to GitHub / Docs / Changelog / Status / Support, and self-hosted to GitHub / Docs. Status and Support are user-menu-specific additions that don't appear in the marketing footer; they land at the end so the brand-spec subset stays in canonical position at the front.	2026-04-29 23:57:16 -04:00
xarmian	8f2be1b391	feat(error): branded 404 + 500 error pages with cloud-mode chrome (TASK-906) (#315) ... * feat(error): branded 404 + 500 error pages with cloud-mode chrome (TASK-906) New web/src/routes/+error.svelte renders for any unhandled error or unmatched route in the SvelteKit tree. Friendly status-specific titles + hints (404, 401/403/500 covered explicitly; falls through to a generic "An error occurred" + framework message for anything else). Cloud mode wraps the error in marketing chrome — AuthHeader at the top, AuthFooter at the bottom — and adds two extra escape CTAs ("Back to getpad.dev" + "Open docs") in addition to the always-present "Go to home" button. So a 404 doesn't drop the user out of the brand and they always have somewhere to go. Self-hosted (cloudMode=false) renders a minimal centered card with just the status code, friendly title/hint, the inline Pad wordmark (matching the auth-card pattern), and a single "Go to home" CTA. No getpad.dev branding imposed on operators' deployments — same gating philosophy as TASK-902/903. The page hydrates authStore in onMount so cloudMode resolves on first paint, fire-and-forget; if the session fetch fails we render the self-hosted variant — safe fallback. Reuses AuthHeader and AuthFooter from TASK-902/903; no need for hand-rolled chrome since those components landed first. Parent: PLAN-900. Test plan: - web/npm run check — 0 errors (693 files; +1 from new page) - web/npm run build — clean - Svelte autofixer — clean * fix(error): context-aware chrome for in-app vs marketing routes per Codex (round 2) Codex caught that +error.svelte unconditionally rendered the Cloud marketing AuthHeader/AuthFooter, but the root +layout.svelte already wraps workspace pages in the Sidebar/TopBar/main-content app shell. On a workspace 404 the result would be both chromes stacked: app shell underneath plus a fixed-position marketing header floating over the top. Fix: branch on the same paths the root layout uses to decide whether to render bare children. "Marketing context" (auth/share/console paths) keeps the full Cloud-mode AuthHeader + AuthFooter treatment; "app-shell context" (everything else, i.e. workspace pages) renders a minimal centered block inside the existing main-content area with no fixed-position chrome of its own. This means the user-facing experience in each context is correct: - /this-does-not-exist (no auth): Cloud → branded marketing 404; self-hosted → minimal centered card with Pad wordmark - /login → same (auth-page family) - /[user]/[ws]/some/missing/route: workspace shell stays intact with a centered "Page not found" inside the main-content area The marketing-context list mirrors the bare-render condition in web/src/routes/+layout.svelte (isAuthPage || isSharePage || isConsolePage) plus the share-page prefix. Verification: npm run check 0 errors; web build clean. * test(e2e): wait for workspace heading before probing topbar trigger The bundle-roundtrip test fired a synchronous isVisible() check on the desktop topbar trigger immediately after `domcontentloaded`. The workspace shell is fully client-rendered (adapter-static has no SSR for app routes), so isVisible() raced hydration: on slower CI runners the topbar wasn't in the DOM yet, the check returned false, the test fell through to the mobile branch, and it then timed out waiting for an element that doesn't exist on the desktop-chromium project. Surfaced by TASK-906 (this PR), which adds ~8 KB of root-level JS (error page + AuthHeader/AuthFooter chunks). That extra chunk-loading shifted the hydration race past the test's check on GitHub Actions runners; it had been winning consistently before. Locally the test passes in ~4s either way — the race is real but tight. Anchor the wait on the workspace heading ("E2E Workspace") which the dashboard route renders the moment hydration completes. Keeps the existing desktop/mobile branching intact and adds one toBeVisible() gate so the rest of the flow runs against a fully-hydrated UI on any runner speed. Verified locally: 4.0s pass after the change.	2026-04-29 23:46:07 -04:00
xarmian	2900a66861	feat(auth): footer parity with marketing site on auth-page family (TASK-903) (#314) ... * feat(auth): footer parity with marketing site on auth-page family (TASK-903) New <AuthFooter cloudMode={...} /> replaces the prior LegalFooter + SupportFooter pair. Single component matches the brand spec (docs/brand.md §7) which describes ONE footer pattern, not two separate strips. Cloud mode (cloudMode=true) carries the full getpad.dev marketing footer: copyright line ("© <year> Pad · Perpetual Software") + the nine-link list in canonical order — GitHub, Docs, Changelog, Contribute, FAQ, Security, Privacy, Terms, Sub-processors. Visual contract anchored on pad-web/src/routes/+layout.svelte (border-top, max-w-6xl, flex-wrap, sm: breakpoint at 640px). Self-hosted (cloudMode=false) renders the legal-essentials only — Terms / Privacy / Sub-processors — preserving the visual treatment of the prior LegalFooter exactly so existing self-hosted deployments see no change after this PR. The Status / Support / GitHub / Changelog / Contribute / FAQ / Security links were Cloud-only in the prior shape too; that stays the case. Wired the new AuthFooter into all five auth-family pages: - /login (replaces LegalFooter + SupportFooter) - /register (replaces LegalFooter + SupportFooter) - /forgot-password (replaces LegalFooter + SupportFooter) - /reset-password/[token] (NEW — was footer-less) - /join/[code] (NEW — was footer-less) LegalFooter.svelte and SupportFooter.svelte are deleted; they were internal to the auth-pages feature and never used elsewhere (grep-verified). AuthHeader's comment that referenced them is updated to point at AuthFooter instead. Year is computed once per page render via new Date().getFullYear() — no auto-refresh needed since auth pages don't sit open across a year boundary in any realistic flow. Parent: PLAN-900. Test plan: - web/npm run check — 0 errors (692 files now, was 693; net -1 reflects 2 deletions + 1 addition) - web/npm run build — clean - go build ./... && go vet ./... && go test ./... — all pass - Svelte autofixer — clean * fix(auth): self-hosted AuthFooter renders nothing per Codex review (round 2) Codex P1: the prior LegalFooter + SupportFooter both gated their entire body on `{#if cloudMode}` — i.e. self-hosted rendered nothing at all. The first draft of AuthFooter incorrectly assumed self-hosted should get the legal-essentials subset (Terms / Privacy / Sub-processors links), which would have rendered getpad.dev's hosted-service legal links on someone else's deployment, misrepresenting the operator's own legal terms. Revert the self-hosted branch to render nothing. The brand spec (docs/brand.md §7) already flags an operator-owned legal/footer mechanism as deferred to the operator-branding follow-up plan, so this restores the prior behavior exactly. Removed the now-dead self-hosted link list, the .auth-footer-legal CSS rules, and the $derived links computation. * fix(auth): flex-direction on reset-password + join wrappers per Codex (round 3) Codex P2: AuthFooter on /reset-password/[token] and /join/[code] sat horizontally next to the auth card instead of below it because those two page wrappers were `display: flex` without `flex-direction: column`. The other three auth pages (login, register, forgot-password) already had column layout so they were unaffected — only the two pages that gained a footer in this PR were broken. Add `flex-direction: column` to .page (reset-password) and .join-page so the footer renders below the card on those routes too.	2026-04-29 23:23:39 -04:00
xarmian	973301847c	feat(auth): shared marketing header on auth-page family in Cloud mode (TASK-902) (#313) ... New <AuthHeader cloudMode={...} /> component renders a top header that visually continues getpad.dev's marketing nav, so users clicking "Login"/"Sign Up" from the marketing site land on auth pages without the sense of jumping properties. Wired into all five pre-login pages: - /login - /register - /forgot-password - /reset-password/[token] - /join/[code] When cloudMode === false (self-hosted) the component renders nothing, so operators ship Pad under their own brand without our chrome imposed on them — matches the existing pattern in LegalFooter.svelte / SupportFooter.svelte. The inline <h1 class="logo">Pad</h1> wordmark on each auth card is now hidden when cloudMode === true (the fixed header carries the wordmark) and kept on self-hosted (where the header is absent). Each page wrapper gets a .cloud-mode class with padding-top: 4rem so the card does not collide with the fixed header. Two pages (reset-password, join) did not previously hydrate authStore; both now call authStore.ensureLoaded() in onMount, matching the pattern already established in /forgot-password. Visual contract anchored on docs/brand.md sections 5–6 — colors and spacing pulled from the app's existing CSS variables; structure matches pad-web/src/routes/+layout.svelte byte-for-byte for the SVG hamburger, flex layout, max-w-6xl container, and md: breakpoint at 768px. Verified via the Svelte autofixer (caught a misplaced <svelte:window> on first draft and was corrected). Parent: PLAN-900. Test plan: - web/npm run check — 0 errors - web/npm run build — clean - go build ./... && go vet ./... && go test ./... — all pass - Manual: covered in PR body	2026-04-29 23:06:15 -04:00
xarmian	de873d8a01	docs(brand): add brand spec defining cohesion contract (TASK-904) (#312) ... Foundation doc for PLAN-900 (Cohesive UX between getpad.dev and Pad Cloud). Defines the visual contract for surfaces that border between marketing and product so the two codebases (this repo's web/ and ../pad-web) can converge intentionally rather than drift accidentally. Central thesis (Section 1): cohesion applies at the SEAMS — auth pages in Cloud mode, error pages, transactional emails — not in the deep app. Self-hosted installs stay neutral throughout. Every parity decision is gated on the existing cloud_mode flag (no new env var). Concrete decisions baked in: - Canonical color tokens anchored on pad-web/src/app.css; the app side moves toward those values for bordering surfaces. Accent palette (blue/green/amber/purple) is already aligned and stays. - Type families: Inter + JetBrains Mono on bordering surfaces only; workspace shell keeps system-ui (intentional — system feel inside a tool). - Header pattern (fixed top, blur backdrop, max-w-6xl, hamburger spec) and footer pattern (link order, copyright format) specified byte-level so a developer can rebuild either from this doc alone. - Header link list deliberately differs between marketing and auth pages (marketing carries Login CTA; auth pages don't); footer link list and order are identical. Includes a known-drift note flagging --text-muted: #666666 in web/src/app.css as failing WCAG AA — pad-web's #8a8a93 passes. Out of scope for this doc; tracked as a fast-follow. No code changes — pure documentation. docs/ is not embedded in the Go binary so this doesn't affect builds.	2026-04-29 22:49:25 -04:00
xarmian	9c5f4d5165	fix(cli): construct auth login URL on CLI side to avoid 0.0.0.0 leak (TASK-839) (#311) ... * fix(cli): construct auth login URL on CLI side to avoid 0.0.0.0 leak (TASK-839) The server builds the CLI auth-approval URL from r.Host, which echoes back whatever Host header the CLI sent. When the local pad server is bound to a bind-all address (e.g. --host 0.0.0.0), the CLI's own config points at that address, so the URL printed by `pad auth login` ends up as http://0.0.0.0:7777/auth/cli/{code} — a bind address, not a usable browser destination. Construct the URL on the CLI instead, using cfg.BrowserURL() (which already rewrites 0.0.0.0 / :: / empty to 127.0.0.1, and returns the explicit URL verbatim for Remote/Cloud). The server-issued auth_url field is now ignored; session_code is what we actually need and is already returned separately. Extracts a small cliAuthBrowserURL helper so the wiring is unit-testable and adds regression coverage for IPv4 bind-all, IPv6 bind-all, empty host, explicit loopback, explicit Remote URL, and trailing-slash trim. * chore(lint): remove unused readBundleAsBytes test helper golangci-lint v2.11.4 (CI) flags this as unused — it was added in the import-bundle test scaffolding (TASK-885 / TASK-891 era) but no caller ever picked it up. Removing it unblocks the lint gate on main. Reviewable in isolation; pure deletion, no behavior change.	2026-04-29 22:03:27 -04:00
xarmian	272868291c	test(e2e): web export → import round-trip with attachment (TASK-894) (#310) ... Closes PLAN-890's regression-safety net: a Playwright spec that seeds a source workspace with an item embedding a real PNG attachment, drives the bundle export through the settings page, imports it through the Create Workspace modal, and verifies via the API that the imported workspace carries the item, the rewritten attachment reference, and the rehydrated blob with bytes matching the original upload. Failure modes the spec catches: - Export link reverts to JSON (toHaveAttribute on href + download) - Import dispatch silently routes to the legacy JSON path (server would fail with gzip decode) - Attachment id rewrite regresses (asserts the new content does NOT contain the OLD UUID and DOES contain a fresh UUID) - Storage/rehydrate path corrupts bytes (assert downloaded blob bytes equal the original PNG) Implementation notes: - Bundle bytes are fetched via the auth'd `request` fixture, not via the browser's `<a download>` click. Playwright's download fork doesn't carry extraHTTPHeaders (Bearer token), so the click would 401. The link's href + download attribute are still asserted via toHaveAttribute — that pins the export-side UI contract. - Modal is opened via a dual-path selector: TopBar's "+ New workspace" button when visible, falling back to the WorkspaceSwitcher dropdown's "+ New Workspace" entry. Same uiStore.createWorkspaceOpen flag, same modal — different chrome on different viewports. - Spec is pinned to desktop-chromium. Running both projects in parallel trips the server's general-API rate limit (~10 req/sec) because seed + export + import + verify makes ~30 calls per worker. The flow has no viewport-specific code worth covering twice; one project is sufficient for round-trip integrity. Documented in the inline `test.skip`. - 1x1 PNG byte sequence is the same as Go's realPNG() — same bytes the server-side attachment tests use, so the e2e exercises the same MIME-validation path. Parent: PLAN-890.	2026-04-29 20:49:37 -04:00
xarmian	89ae5369ae	feat(web): settings page exports .tar.gz bundle (TASK-892) (#309) ... * feat(web): settings page exports .tar.gz bundle (TASK-892) Replace the legacy "Download JSON" button on the workspace settings page with a single "Download .tar.gz" link that hits the existing ?format=tar dispatch on handleExportWorkspace. The bundle ships items + comments + version history + attachment blobs + manifest in a single archive — same shape the CLI's 'pad workspace export' command produces. Behavior: - Field label changed from "Export" to "Export bundle" - Button text changed from "Download JSON" to "Download .tar.gz" - href appended ?format=tar - download attribute changed from {slug}-export.json to {slug}-export.tar.gz - Added a title= tooltip explaining the bundle contents and that it's re-importable via the Create Workspace dialog No JSON-export UI surface remains in the settings page. The legacy JSON path on the server side stays for back-compat (any operator still hitting /export with no query keeps getting JSON). Parent: PLAN-890. Sibling task TASK-893 will flip the import modal to consume .tar.gz so the round-trip closes. * feat(web): import workspace bundle (.tar.gz) in CreateWorkspaceModal (TASK-893) Folded into the same PR as TASK-892 because Codex (correctly) flagged that exporting .tar.gz while still importing JSON ships a half-baked state — the settings page tooltip even tells users the bundle is re-importable via this modal. Now it actually is. Changes in CreateWorkspaceModal.svelte: - importWorkspace() now calls api.workspaces.importBundle(file, name) instead of reading + JSON.parse-ing the file and POSTing through api.raw.post. The new method sets Content-Type: application/gzip and posts the raw File body, which the server's existing dispatch in handleImportWorkspace routes to the bundle path (handlers_workspaces.go:361). - File picker accept attribute changed from ".json" to ".tar.gz,.tgz,application/gzip,application/x-gzip" — UI advertises only the new format. - Drag-drop guard accepts .tar.gz, .tgz, AND .json (legacy back-compat — server still supports JSON imports for any operator with an old archive lying around, even though we don't advertise it). - Drop-zone hint and import explanatory text updated to mention the bundle format and what's preserved (items, comments, attachments, version history). - Auto-fill regex strips -export.tar.gz, .tar.gz, .tgz, AND .json suffixes when seeding the workspace name from the filename. New api.workspaces.importBundle method in web/src/lib/api/client.ts: - Bypasses the JSON-only `request` helper — sets Content-Type: application/gzip and posts the File body raw. - Handles CSRF token, 401 redirect, and shaped error responses the same way `request` does. - Mirrors the CLI's `pad workspace import <bundle.tar.gz>` flow. Server-side: no changes — handleImportWorkspace dispatches on Content-Type and the bundle path was already audited + hardened in PR #308. Parent: PLAN-890. Closes the import/export round-trip alongside TASK-892. TASK-894 (Playwright e2e) covers the round-trip. * fix(web): drop .json from import accept list per Codex review (round 2) Codex P2 on PR #309: I left .json in the drag-drop guard isAcceptedBundleFile, intending to be lenient for users with legacy JSON exports. But api.workspaces.importBundle always POSTs as Content-Type: application/gzip — so a dropped .json file would route to the server's bundle path and fail with a gzip decode error. Confusing UX. Make the modal strictly tar.gz-only: - isAcceptedBundleFile regex narrowed to /(\.tar\.gz|\.tgz)$/i - name auto-fill regex narrowed to strip only -export.tar.gz, .tar.gz, .tgz suffixes - Comment documents that operators with legacy JSON exports can still curl them against POST /workspaces/import directly — the server keeps the JSON dispatch for back-compat. The file picker accept attribute was already strict (.tar.gz, .tgz, application/gzip, application/x-gzip) — this commit makes the drag-drop path consistent with it. Parent: PLAN-890.	2026-04-29 20:34:42 -04:00
xarmian	47a4448afc	chore(import-bundle): audit + harden bundle import validation (TASK-891) (#308) ... * chore(import-bundle): audit + harden bundle import validation (TASK-891) Re-reviewed handlers_import_bundle.go before exposing the bundle import flow through the web UI under PLAN-890. The audit doc lives at DOC-895; this commit lands the small inline fixes. Findings + actions: - Duplicate pad-export.json now rejected (was: silently ran ImportWorkspace twice, stranding the first workspace as an orphan with no attachments). - Duplicate attachments/manifest.json now rejected (was: silently overwrote manifestByPath, dropping prior entries). - Defense-in-depth path-traversal guard added via isSafeBundleEntryName — rejects entries with `..` segments, absolute paths, or NUL bytes BEFORE the switch. Storage was already hash-keyed and safe, but the silent-skip behavior on malicious tar names was a poor audit story. - Auth/permissions now documented on the handler — RequireAuth middleware gates the endpoint; no per-workspace role check applies because the request creates a new workspace (mirrors handleCreateWorkspace). Tests added: TestImportBundle_RejectsDuplicateExport, TestImportBundle_RejectsDuplicateManifest, TestImportBundle_RejectsPathTraversal, TestIsSafeBundleEntryName. Two larger gaps deferred as their own tasks: - TASK-896 (partial-import orphan workspace on mid-stream failure — needs design discussion). - TASK-897 (per-user storage quota enforcement on import — gated on Phase 2 quota work; matches upload handler's warn-only Phase 1 policy today). Parent: PLAN-890. * fix(import-bundle): roll back partial workspace on validation reject per Codex review (round 1) Codex P1 on PR #308: when the duplicate-pad-export.json or duplicate-manifest.json guards fire, the workspace from the first occurrence has already been inserted by ImportWorkspace. The handler returned 400 but the orphan workspace stayed in the destination DB. A malformed/malicious bundle could repeatedly POST and pile up half-imported workspaces. Fix: when importBundle returns an importStatusError after creating a workspace, the handler now soft-deletes that workspace via DeleteWorkspace before returning the 400. Mid-stream errors that are NOT importStatusError (e.g. manifest decode after items inserted) intentionally keep the partial workspace — that's the existing design tracked under TASK-896 (partial-import design discussion). Tests extended: TestImportBundle_RejectsDuplicateExport and TestImportBundle_RejectsDuplicateManifest now also list the destination workspaces after the rejected import and assert the partial workspace does NOT appear. Parent: PLAN-890. * fix(import-bundle): cascade attachment tombstone on rollback per Codex review (round 2) Codex P1 round 2 on PR #308: when the duplicate-manifest guard fires AFTER blobs have already been rehydrated (e.g. bundle layout [pad-export, manifest, blob1, blob2, duplicate-manifest]), the previous fix soft-deleted the workspace but left the attachment rows live. Live rows pin blobs from orphan-GC and continue counting toward per-user storage usage even though the workspace is gone. Fix: added Store.SoftDeleteWorkspaceAttachments(workspaceID), a single bulk UPDATE that tombstones every live attachment row (originals AND thumbnails — both carry the same workspace_id) under a workspace. The handler's rollback path now calls this BEFORE DeleteWorkspace so orphan-GC reclaims the blobs after the grace window. Best-effort: any error in either op is logged with workspace context but the original 400 still flows. New test: TestImportBundle_RollbackTombstonesAttachments builds a real export bundle from a source workspace with one attachment, surgically appends a duplicate manifest.json AFTER the real entries, posts it, and asserts (a) 400, (b) workspace gone from listings, (c) zero live attachment rows on the destination. The pre-fix code left rows=1 live; the new path tombstones them. Parent: PLAN-890. * fix(import-bundle): return ws on path-traversal reject so rollback fires (Codex round 3) Codex P1 round 3 on PR #308: the path-traversal early-return at the top of the import loop returned (nil, importStatusError) instead of (ws, importStatusError). When a malicious path-traversal entry follows a valid pad-export.json, the workspace was already created — but because the handler saw ws == nil, it skipped the rollback cascade, leaving the workspace and any rehydrated attachments behind. Fix: return ws (which is nil before pad-export.json is processed, so the no-workspace cleanup path still works for first-entry-bad bundles, and non-nil after, so the cascade runs). One-line change keyed off the existing rollback flow. New test: TestImportBundle_PathTraversalAfterExportRollsBack hand-builds a tar with a valid pad-export.json followed by a "attachments/../../etc/passwd" entry, posts it, asserts 400, and asserts the partial workspace is GONE from listings. Pre-fix this test would have shown the workspace leaking through. Parent: PLAN-890.	2026-04-29 20:24:25 -04:00
xarmian	2bb7ac35e4	feat(attachments): orphan GC sweep with periodic scheduler (TASK-886) (#307) ... * feat(attachments): orphan GC sweep with periodic scheduler (TASK-886) Background job that reclaims attachments past the grace period. Two qualification criteria, both with a 30-day default grace: - item_id IS NULL AND deleted_at IS NULL AND created_at < cutoff (never-attached uploads — editor uploaded then tab-closed before attaching to an item) - deleted_at IS NOT NULL AND deleted_at < cutoff (soft-deleted via the Settings → Storage delete button or the DELETE /attachments/{id} endpoint) Reclamation is dedupe-aware: content-addressed storage means the same hash can be referenced by multiple rows, so the on-disk blob is only removed when the GC'd row is the LAST live reference to its content_hash. Otherwise the row drops and the blob stays for the remaining references. CountLiveAttachmentsForHash is the predicate. Per-row failures (resolve backend, blob delete, hard-delete) are logged and skipped; the sweep keeps making progress. Catastrophic errors (DB failure) return up to the loop, which logs and waits for the next tick rather than crashing the server. Lifecycle: - SetOrphanGCConfig overrides the default 24h interval / 30-day grace. cmd/pad reads PAD_ORPHAN_GC_INTERVAL / PAD_ORPHAN_GC_GRACE (Go duration syntax — 1m, 24h, 720h) so operators can tune without recompiling and tests can crank the interval down to 1ms to see sweeps land in CI. - StartOrphanGC kicks the loop. Idempotent — second call is a no-op so a misconfigured caller can't double-spawn. - Server.Stop() now signals the loop via stopOrphanGC() before s.bg.Wait(), so process shutdown drains the goroutine cleanly (BUG-842 invariant). - Each tick wraps the sweep in a 30m context timeout so a slow scan can't pin the goroutine across multiple intervals. Tests: - TestOrphanGC_ReclaimsSoftDeleted: upload → soft-delete → sweep with future cutoff → DB row gone + blob gone from FSStore. - TestOrphanGC_ReclaimsLongOrphans: upload → backdate created_at 31d → sweep with 30d grace cutoff → row reclaimed. - TestOrphanGC_KeepsRecentRows: upload → soft-delete → sweep with past cutoff → row stays. Catches a typo in the WHERE clause that would silently destroy live attachments. - TestOrphanGC_PreservesSharedBlob: two uploads with identical bytes (same hash, same blob), soft-delete only one → sweep → one row reclaimed BUT BlobsReclaimed=0 because the other row still references the blob. Pin for content-addressed dedupe. - TestOrphanGC_StartStop: loop spins up at 1ms interval, second StartOrphanGC is a no-op, Stop drains via testServer's cleanup. Parent: PLAN-866. Closes the phase 1 plan with full export → import → orphan-cleanup round-trip. * fix(attachments): protect referenced/in-flight blobs from orphan GC per Codex (round 1) Two real correctness issues Codex caught on PR #307: P1. The editor's normal upload flow leaves attachments.item_id NULL. The canonical association lives in markdown content (the editor PATCHes "pad-attachment:UUID" into the item) — but the GC's "never-attached past 30d" predicate only checked item_id. So a legitimate inline image could be hard-deleted 30 days after upload even though item content still references it. Added store.AttachmentReferencedInItems(workspaceID, attachmentID) that scans items.content + items.fields for "pad-attachment:UUID". The GC sweep now runs this check before reclaiming any never-attached row; if any live item references the attachment, the row is left alone (and re-checked next sweep). P2. Race between concurrent upload and GC. Upload calls AttachmentStore.Put (blob lands on disk) → THEN inserts the DB row. Between those two steps an orphan-GC sweep could count zero live refs for the hash, delete the blob, and the upload's row insert would then point at a missing blob. Added Server.inFlightUploadHashes (sync.Map of *atomic.Int64 counters) with markUploadInFlight / uploadInFlight helpers. Every Put + CreateAttachment site fences itself via markUploadInFlight: the upload handler, the transform handler, the thumbnail derivation pipeline, and the bundle-import rehydrate path. The GC sweep treats an in-flight hash as "another live ref" so it leaves the blob alone. Tests: - TestOrphanGC_KeepsReferencedNeverAttachedRows: upload (item_id NULL) → create item with pad-attachment: ref → backdate 31d → sweep with 30d cutoff → row stays. - TestOrphanGC_RespectsInFlightUploads: upload → soft-delete → register an in-flight upload at the same hash → sweep → DB row goes (it's tombstoned past grace) but blob stays so the in-flight upload can complete cleanly. The DB row still gets reclaimed in the in-flight case because the soft-deleted row is independently past grace; only the blob delete is fenced. That's correct: the blob remains usable for the incoming upload and the new upload will register its own attachments row. * fix(attachments): mutex-protect in-flight tracker + portable JSONB scan per Codex (round 2) Two fixes for the round-2 findings on PR #307: P1. Same-hash race in the in-flight upload tracker. The sync.Map + *atomic.Int64 design split increment from LoadOrStore-then-add and release-decrement from delete, so a release could see "0" and start deleting while another upload concurrently reloaded the same map entry and incremented to "1" — the second upload's signal then lived in a doomed map slot, invisible to subsequent uploadInFlight calls. Replaced with a plain map[string]int64 + sync.Mutex. Inc, dec, delete-when-zero all run under one critical section, so any inspection sees a consistent snapshot. Net cost is one mutex per mark/release; uncontended this is ~10ns and the upload path is already doing far more expensive work (Put + DB insert). Stress test: 20 goroutines × 500 iterations of mark→check→release on a shared hash. Every check must observe in-flight=true while the calling goroutine holds the mark. Final state must be empty. Runs cleanly under -race -count=3. P2. Postgres JSONB compatibility. items.fields is TEXT on SQLite but JSONB on PostgreSQL (per pgmigrations/001_initial.sql). LIKE on JSONB fails with a type error, so the orphan GC's reference scan would error on Postgres and skip every never-attached row — breaking orphan reclamation for those rows entirely. Cast fields::text in the Postgres dialect path: fieldsExpr := "fields" if s.dialect.Driver() == DriverPostgres { fieldsExpr = "fields::text" } Same approach used elsewhere in the store for dialect-sensitive text searches. * fix(attachments): close GC/upload TOCTOU + protect in-grace peers per Codex (round 3) P1 round 3: TOCTOU race between uploadInFlight check and store.Delete. The mutex protected the in-flight counter but not the GC's check-and-delete sequence. A new upload could call markUploadInFlight between our check and our blob delete, then run Put after the blob was gone — its CreateAttachment would insert a live row pointing at the missing hash. Fixed by holding inFlightHashesMu across the check + FS Delete: s.inFlightHashesMu.Lock() inFlight := s.inFlightHashes[hash] > 0 if !inFlight && others == 0 { store.Delete(ctx, key) } s.inFlightHashesMu.Unlock() A concurrent markUploadInFlight blocks until either we skip (because we observed in-flight) or finish deleting. Lock window is ms-class on FSStore; a per-hash lock can replace this server-wide mutex when S3 lands in Phase 2. P2 round 3: CountLiveAttachmentsForHash counted only live rows, so GC could reclaim the blob from row A (soft-deleted 31d ago) even when row B was also soft-deleted but only 1 day old — within grace, so its blob must stay reachable until its own grace lapses. Replaced with CountProtectingAttachmentsForHash which counts rows where deleted_at IS NULL OR deleted_at >= graceCutoff. The blob is preserved until every soft-deleted peer has aged past its own grace window. Tests: - TestOrphanGC_RespectsSoftDeletedInGracePeer: two rows sharing a hash, soft-delete both, backdate only one past 30d → sweep with 30d cutoff → older row reclaimed but blob stays for the still-in- grace peer. - existing TestOrphanGC_RespectsInFlightUploads still passes (still uses the in-flight signal correctly). * fix(attachments): dedupe blob-reclaim metric across same-hash peers per Codex (round 4) Codex round 4 noted that when multiple soft-deleted peers share a content_hash and all are past grace, the GC sweep would inflate BlobsReclaimed and BytesReclaimed: AttachmentStore.Delete treats a missing key as success, so the second peer's idempotent no-op delete still bumped the counter. Functional cleanup was correct (the blob really was gone after the first peer); only the metric / log line was wrong, which makes operator dashboards report fictitious bytes-reclaimed values. Track per-sweep reclaimed hashes in a map and skip the Delete call + counter increment for repeats. The DB row still gets hard-deleted on each peer. Test: TestOrphanGC_DedupesBlobReclaimMetric uploads twice with identical bytes (single shared blob), soft-deletes both, backdates deleted_at past grace → sweep deletes 2 rows and reports BlobsReclaimed=1 / BytesReclaimed=blobLen rather than 2 / 2*blobLen.	2026-04-29 19:25:55 -04:00
xarmian	134f55045d	feat(attachments): import workspace bundle with rehydrate + UUID remap (TASK-885) (#306) ... * feat(attachments): import workspace bundle with attachment rehydrate + UUID remap (TASK-885) POST /workspaces/import now accepts a tar.gz bundle (Content-Type: application/gzip) and rebuilds the workspace + attachments + items in one round trip. JSON imports still work — content-type dispatch in handleImportWorkspace routes the request. Three-phase flow: 1. Walk the tar, capture pad-export.json + manifest.json + every attachment blob into memory. 2. Run the existing ImportWorkspace path to create the workspace + collections + items + comments + links + versions. New IDs are generated; item.slug is preserved (the existing remap path doesn't re-slugify). 3. For each manifest entry, rehydrate the blob through the storage backend (re-validate MIME + re-hash defensively, don't trust the manifest), insert a fresh attachments row. Build an oldID→newID map keyed on attachment uuid. 4. Walk every imported item's content + fields, replace "pad-attachment:OLD" with "pad-attachment:NEW" in one transactional pass. Refresh FTS afterward (direct UPDATE bypasses triggers). Phase 2 errors per-attachment are logged and skipped — the workspace keeps importing rather than rolling back. The import handler returns the new workspace and the operator can inspect logs for any attachment that didn't make it. CLI: - pad workspace export now defaults to --bundle (.tar.gz) since pad import handles bundles. --json reverts to legacy items-only. - pad import auto-detects format by file extension (.tar.gz / .tgz → application/gzip). Other extensions go through the legacy JSON path. - New Client.PostRawWithContentType for explicit-content-type POSTs. Tests: - TestImportBundle_RoundTrip: upload → embed in markdown → export source → import to FRESH server → verify attachment list has 1 row with new UUID → item content rewritten to new UUID and old UUID is gone → download new blob matches original bytes. - TestImportBundle_LegacyJSONStillWorks: JSON content-type still hits the legacy path. - TestImportBundle_RejectsBadGzip: garbage gzip body returns 400. Parent: PLAN-866. With TASK-884 + TASK-885 merged, the round-trip acceptance criterion (export → import → images intact) is met. * fix(attachments): stream import end-to-end per Codex (round 1) Two memory regressions Codex caught on PR #306: P1 (server). importBundle was buffering every blob into a map[string][]byte during a first pass, then iterating the manifest on a second pass. A 2 GiB bundle full of 25 MiB attachments would pin ~2 GiB of heap. Reworked to single-pass streaming: pad-export.json → import workspace + build slug→id map attachments/manifest.json → index entries by tar path attachments/<uuid>.<ext> → look up entry, rehydrate now The export bundler always writes pad-export.json + manifest.json BEFORE any blob (deterministic order from handlers_export_bundle.go), so this works without buffering. Bundles that violate the ordering — a third-party tool that writes blobs first — return 400 with a clear error. Memory footprint now bounded by the largest single blob (≤25 MiB) regardless of bundle size. Stale blobs without a manifest entry are skipped (their bytes io.Copy'd to io.Discard so the tar reader stays in sync). Unknown top-level entries (forward-compat for future bundle additions) are also consumed and ignored rather than left dangling. P2 (CLI). pad import used os.ReadFile, buffering the entire bundle client-side before posting. Switched to os.Open + a new Client.PostStreamWithContentType helper that streams the body directly into the request — together with the server-side fix, import is end-to-end streaming. Tests: - TestImportBundle_RejectsOutOfOrderTar: hand-crafted bundle with a blob before pad-export.json returns 400 with "ordering" in the message. - existing TestImportBundle_RoundTrip / LegacyJSONStillWorks / RejectsBadGzip continue to pass under the new streaming flow. * fix(cli): give streaming endpoints a 1h timeout per Codex (round 2) Codex P1 round 2: PostStreamWithContentType + RawStream were both using the shared 10s-timeout httpClient. The default works fine for normal API calls but kills a multi-GiB bundle import or export over anything slower than a local network — Client.Timeout fires mid-stream with "Client.Timeout exceeded". Added a dedicated streamClient on Client with a 1h timeout, used by both RawStream (export bundle download) and PostStreamWithContentType (import bundle upload). 1h is generous enough for ~100 MB/s uplinks shipping a 350 GiB bundle and still caps a hung connection eventually. The 10s default stays in place for every other call — short timeouts are the right SLA for normal API requests and protect the CLI from hanging on a wedged server. * fix(attachments): make import bundle cap configurable per Codex (round 3) Codex P1: the 2 GiB import cap was hard-coded with a comment promising operator override "later" — but no setter existed, so workspaces over 2 GiB stream out fine on export and fail on re-import. Added Server.SetImportBundleMaxBytes wired from the PAD_IMPORT_BUNDLE_MAX_BYTES env var in cmd/pad/main.go. Mirrors the existing PAD_ATTACHMENT_MAX_BYTES pattern. Default stays at 2 GiB so the typical workspace works without configuration; operators with larger exports can raise it without recompiling. The per-blob cap (importBlobMaxBytes = 25 MiB) is intentionally kept constant — it bounds in-flight memory regardless of total bundle size, and a 25 MiB-per-blob ceiling matches the upload handler's default, so a bundle can never smuggle larger blobs than the upload endpoint accepts. * fix(attachments): scale per-blob import cap with PAD_ATTACHMENT_MAX_BYTES per Codex (round 4) Codex P1 round 4: importBlobMaxBytes was hard-coded at 25 MiB but the upload handler's per-file cap is configurable via PAD_ATTACHMENT_MAX_BYTES. An operator who raised the upload cap to allow 50 MiB attachments could export a workspace successfully (WorkspaceAttachmentsForExport doesn't gate on size) but the re-import would reject every blob over 25 MiB. Replaced the const with effectiveBlobMaxBytes() which reads s.attachmentMaxBytes (or falls back to defaultAttachmentMaxBytes). The pad-export.json cap also scales with this value (4×) so a content-heavy workspace doesn't trip its own JSON ceiling on a server with raised attachment limits. Error message on a too-large blob now points the operator at PAD_ATTACHMENT_MAX_BYTES so they know which knob to turn rather than digging through code to find the cap. * fix(attachments): independent metadata cap for bundle import per Codex (round 5) Codex P2 round 5: tying pad-export.json + manifest.json caps to PAD_ATTACHMENT_MAX_BYTES regressed deployments that LOWER the attachment cap. A 1 MiB attachment cap would force metadata to fit in 4 MiB / 1 MiB respectively — but metadata size scales with workspace item count, not attachment blob sizes, so a tight upload limit shouldn't gate it. Added importMetadataMaxBytes = 100 MiB constant for both metadata files. effectiveBlobMaxBytes() still drives the per-blob cap which genuinely tracks attachment-upload policy.	2026-04-29 18:56:43 -04:00
xarmian	a0336e0248	feat(attachments): bundle attachments + manifest in workspace export (TASK-884) (#305) ... * feat(attachments): bundle attachments + manifest in workspace export (TASK-884) GET /workspaces/{ws}/export?format=tar streams a gzip'd tar bundle: pad-export.json # the existing WorkspaceExport JSON attachments/manifest.json # uuid → {filename, mime, size, hash, ...} attachments/<uuid>.<ext> # original blobs only — no thumbnails Default (no ?format) keeps returning JSON so existing automation hitting the endpoint without a query param continues to work unchanged. The CLI's pad workspace export now opts into the bundle by default; pass --json for the legacy items-only output. Implementation: - store.WorkspaceAttachmentsForExport returns originals only (parent_id IS NULL); thumbnails are re-derived on import via the existing pipeline so shipping them would double the bundle size. - handleExportWorkspaceBundle streams chunks straight into the response writer rather than buffering — a workspace with multi- GB of attachments would otherwise pin that much memory. - AttachmentManifest is versioned (separate from WorkspaceExport version) so the bundle layout can evolve independently. - bundleAttachmentPath is exported (lowercase package fn) so the import path in TASK-885 can resolve manifest entries to tar entries without duplicating the filename logic. - CLI gates against writing binary tar.gz to a TTY and appends the conventional extension when -o is passed without one. Tests: - TestExportBundle_RoundTrip: two uploads → bundle contains pad-export.json + manifest + 2 blobs whose bytes match the uploads + manifest decodes cleanly + WorkspaceExport decodes. - TestExportBundle_HidesThumbnails: synthetic thumbnail row, the manifest excludes it. - TestExportBundle_LegacyJSONStillWorks: no ?format param returns application/json with a decodable WorkspaceExport (backward compat regression guard). Parent: PLAN-866. TASK-885 (import path + UUID remap) consumes the manifest produced here. * fix(attachments): stream export bundle + revert default to JSON per Codex (round 1) Two findings from Codex on PR #305: 1. CLI buffered the entire response in memory via RawGet → io.ReadAll, defeating the server-side streaming design and risking OOM on a multi-GB bundle. Added Client.RawStream which copies the response body straight into an io.Writer; export now opens the target file and streams directly into it. 2. Default tar.gz output broke `pad export → pad import` round trip because the import handler still only accepts JSON. Reverted the CLI default to JSON; bundle is now opt-in via --bundle. The flag docstring notes that TASK-885 will flip the default once import handles bundles. * fix(attachments): surface tar/gzip close errors and truncation per Codex (round 2) Codex round 2 finding: deferred tw.Close() / gzw.Close() ignored errors. If a backend returned fewer bytes than size_bytes claimed, io.Copy returned nil, the tar writer's "missed N bytes" trip fired at Close, and the handler still completed a 200 OK with a corrupt bundle that gunzip would later refuse to decompress — silently from the operator's perspective. Two changes: 1. The deferred close now logs both tw.Close() and gzw.Close() errors with structured context, so a corruption-on-finalize trip shows up in the operator log. 2. streamAttachmentToTar checks the bytes-copied count against a.SizeBytes after io.Copy and returns a per-attachment error when they disagree. The error is logged with attachment_id + storage_key so an operator can correlate the corruption with the row to investigate. Regression test: TestExportBundle_TruncatedBlobLogsError forces a size_bytes/blob desync via direct UPDATE and asserts the resulting bundle bytes don't decode cleanly. (HTTP status stays 200 because headers are already on the wire by the time we detect the desync; that's an inherent limitation of mid-stream errors, but the new logs + close-error surfacing make the failure observable.) * fix(attachments): X-Bundle-Status trailer for export-stream success per Codex (round 3) Codex P1 round 3: even with the per-blob truncation log + tar/gzip close-error logs, mid-stream failures looked successful to clients. The CLI's RawStream finished without a transport error, the file landed on disk, and "Exported workspace" printed regardless of whether the bundle was actually complete. Two complementary signals now mark a clean stream: 1. HTTP trailer X-Bundle-Status. The handler declares the trailer in the initial Trailer header and sets it to "ok" only after tw.Close() and gzw.Close() both return without error. CLI checks the trailer after streaming and discards the file + returns error if it's absent or non-"ok". 2. The handler skips the deferred clean close on the error path, leaving the gzip footer unwritten. A client that ignores the trailer (curl, third-party tooling) still sees a corrupt gzip stream that gunzip refuses to decompress. CLI: pad workspace export --bundle now removes any partial output file on failure rather than leaving a corrupt one behind. Client.RawStream signature changed to return (bytes, *http.Response, error) so callers can inspect resp.Trailer; the only caller is the export command. Tests: TestExportBundle_TruncatedBlobAbortsStream now asserts both signals (trailer absent + gzip/tar can't fully decode), and TestExportBundle_SuccessTrailer pins the happy-path trailer.	2026-04-29 18:30:43 -04:00
xarmian	d3a543db6f	feat(attachments): admin per-user storage quota override UI (TASK-883) (#304) ... * feat(attachments): admin per-user storage quota override UI (TASK-883) Surfaces the storage_bytes plan_overrides key in the admin user-detail page so operators can lift or tighten an individual user's quota without poking at JSON via the API directly. Frontend (console/admin/+page.svelte): - Dedicated "Storage quota override" input below the existing overrides grid. Storage is byte-counted, not row-counted, so a number input forcing the admin to type 536870912 for 512MB would be hostile. Accepts: • "10 GB" / "500MB" / "1.5 GB" (IEC shorthand) • "1024" (raw bytes) • "-1" (unlimited) • "" (clear → falls back to plan default) - Live parse preview ("= 10.0 GB (10,737,418,240 bytes)") so the admin can verify the unit was understood. - "Reset to plan default" button clears the field; save commits the absence as a removed override key. - Pre-fills with the current effective override formatted in the largest exact unit so a previously-set "10 GB" doesn't reload as "10737418240". Backend: - ActionPlanOverridesChanged audit constant. - handleAdminUpdateUser now logs an audit event with old/new override JSONs whenever plan_overrides is patched. Lets operators correlate a mysteriously-allowed upload with the override that enabled it. Tests: - TestAdminUpdateUser_StorageOverrideRoundTrip: PATCH with storage_bytes:1073741824 → GET shows the new override → audit feed contains plan_overrides_changed event → clearing the override removes it. - TestAdminUpdateUser_NonAdminForbidden: member-role user cannot PATCH another user's plan_overrides (regression guard for the audit-log path). Parent: PLAN-866. The Settings → Storage page (TASK-882) reflects the new effective limit immediately after save because both call the same WorkspaceStorageInfo helper. * fix(admin): parse plan_overrides JSON on read, clear via empty string per Codex (round 1) Two related bugs in the admin user-detail page that Codex caught in PR #304 round 1: 1. The save path sent JSON null when every override field was blank, but the Go handler uses a *string and JSON null decodes to a nil pointer — the handler's existing nil-vs-non-nil branch then skips the update, meaning "Reset to plan default" reported success without actually clearing the override. Fixed by sending "" (empty string) which routes through SetUserPlanOverrides("") and clears the column. 2. The form-populate path treated u.plan_overrides as an object while the API actually returns the raw column value as a JSON string. So `'storage_bytes' in ov` was checking string indices on a literal '{"storage_bytes":1073741824}' string, returning false, and any user with stored overrides loaded a blank form. This was a pre-existing bug in the workspaces / api_tokens / etc. fields too — fixed for all of them by parsing the JSON in parsePlanOverrides() before reading keys, with a defensive "future-proof" branch in case the API ever switches to a decoded object. TS type for AdminUser.plan_overrides updated to `string | null` to match the actual API contract. Backend regression test added (TestAdminUpdateUser_OmittedOverrides Preserved) that pins the other half of the contract: PATCH with plan_overrides absent must NOT clear the column. The test was straightforward to add because the existing test infrastructure (bootstrapFirstUser, doRequestWithCookie) already covers the admin auth path.	2026-04-29 17:56:19 -04:00
xarmian	504d348917	feat(attachments): Settings → Storage tab with attachment list (TASK-882) (#303) ... * feat(attachments): Settings → Storage tab with attachment list (TASK-882) Adds the Settings → Storage tab and the underlying list/delete API endpoints so workspace owners can audit and reclaim attachment bytes. Backend (TASK-882 needs this — there was no list/delete API yet): - store.WorkspaceAttachments: paginated list with filter (category, attached/unattached, collection_id) + sort allowlist (size, filename, created_at — each with desc variant). LEFT JOIN to items + collections enriches each row with item_title/slug + collection_slug for the "in [[Item]]" link. Hides derived (thumbnail) rows by default — they count toward quota but are managed automatically and would clutter the user-facing list. - store.SoftDeleteAttachment: tombstones the row + every variant. Blob on disk stays put; orphan GC reclaims past the grace period (TASK-886). - GET /workspaces/{ws}/attachments — viewer+, returns {attachments, total, limit, offset}. - DELETE /workspaces/{ws}/attachments/{id} — editor+. Refuses to delete derived rows directly (returns 400 with derived_attachment code) and invalidates the storage-usage cache. Frontend: - StorageTab.svelte component (lib/components/settings) with usage bar (color thresholds at 80%/100%, override badge), 5-select filter row (category, item, collection, sort, page size), attachment list with thumbnails (image variants via thumb-sm, emoji icon otherwise), item link, MIME, size, date, and per-row delete with confirm() dialog. Pagination footer with Prev/Next + "showing X–Y of Z". - TS api.attachments.list() / delete() + types. - Wired as a new "Storage" tab on the workspace settings page. Tests: - TestListAttachments_Pagination: 3 uploads, default + size-asc sort, limit/offset paging. - TestListAttachments_HidesDerived: synthetic thumbnail row, asserts the list excludes parent_id != NULL rows. - TestDeleteAttachment_HappyPath: upload → delete → list empty → storage usage drops to 0 (cache invalidation hook fires) → second delete returns 404. - TestDeleteAttachment_DerivedRefused: thumbnail rows can't be deleted directly via the API. Parent: PLAN-866. * fix(attachments): collection visibility + category gaps + item ref shape per Codex (round 1) Three findings from Codex on PR #303 round 1: P1 — Collection visibility leak. The storage list returned all workspace attachments without applying per-user collection access, so a member with collection_access=specific would receive hidden collections' attachment IDs/filenames/item titles and could then pull the bytes via the existing download endpoint. Fixed by threading visibleCollectionIDs(r, workspaceID) through to the store filter. nil = admin/no restriction; empty slice = zero visible collections (zero rows by design); explicit set = restrict i.collection_id IN (...). Orphans (item_id IS NULL) are excluded for restricted users since their filenames would still leak. P2 — item_ref shape didn't match the route. The store synthesized "<collection_slug>/<item_number>" and the UI inserted it verbatim into the URL, producing /user/ws/tasks/tasks/5. Dropped item_ref entirely; UI now builds URLs from item_slug + collection_slug which is the actual route shape. P2 — Category filter coverage. mimePrefixForCategory only handled image/video/audio. Selecting Documents/Text/Archive/Other in the UI silently passed through with no MIME predicate so the list showed everything. Replaced with mimePredicateForCategory which emits the right SQL fragment per bucket: prefix LIKE for the type/ buckets, explicit IN list for document/text/archive (mirroring the allowlist in internal/attachments/mime.go), and a NOT-IN composite for "other". Tests: - TestWorkspaceAttachments_VisibilityFilter: admin sees all 3 rows; restricted to one collection sees only that collection's row + orphan suppressed; empty visibility yields zero rows. - TestWorkspaceAttachments_CategoryFilters: image/document/text/ archive/other each return exactly the matching MIME types. * fix(attachments): item-level visibility on list + delete per Codex (round 2) Two more findings from Codex on PR #303 round 2: 1. The list filter used VisibleCollectionIDs alone — but that set includes collections containing any item-level grant for the user. A guest with one item granted in collection B would still receive attachment metadata for every item in collection B. Replaced with the (fullCollIDs, grantedItemIDs) tuple from guestResourceFilter so the SQL ORs collection-level full access against per-item grants, matching how handlers_search / handlers_activity narrow lists. 2. The delete endpoint validated workspace membership but never checked the attachment's parent item is visible to the caller. An editor with restricted collection access could delete attachments in hidden collections by guessing/obtaining the attachment ID. Added requireItemVisible after fetching the parent item, plus a fallback gate for orphan attachments (item_id IS NULL) so restricted users get 404 there as well. Store-level filter renamed: VisibleCollectionIDs → Restricted + FullCollectionIDs + GrantedItemIDs. Tests cover the collection-only, item-grant-only, and zero-visibility paths. * fix(attachments): allow deleting attachments when parent item is soft-deleted (round 3) Codex P2 from PR #303 round 3: the storage list intentionally surfaces attachments whose parent item has been soft-deleted (so the user sees what's still consuming quota), but the delete handler used GetItem, which filters soft-deleted out and returned 404 before SoftDeleteAttachment could run — turning every Delete button on those rows into a no-op. Fixed by adding store.GetItemIncludeDeleted (mirroring the existing GetItemBySlugIncludeDeleted) and switching the delete path to use it. The visibility check still keys off the (still-set) collection_id, so soft-deleting an item doesn't escalate access — restricted users still hit requireItemVisible's 404 if they couldn't see the parent. Regression test: create item → attach → soft-delete item → list still returns the row → delete returns 204. * fix(attachments): list surfaces attachments under soft-deleted parents (round 4) Codex round-4 finding: WorkspaceAttachments still LEFT JOIN'd items with AND i.deleted_at IS NULL, so attachments whose parent item was soft-deleted disappeared from the list — even though the previous round wired GetItemIncludeDeleted on the delete path. Net effect: restricted editors with access to that collection couldn't discover the row in the UI; only full-access users saw it as an orphan-looking entry. Fix: drop the deleted_at filter from the JOIN. The collection ACL predicate (i.collection_id IN ...) now sees the (still-set) collection_id from the soft-deleted item, so visibility behaves consistently for live and tombstoned parents. Soft-deleted items don't escalate access — the collection_id stays put. UX: response now carries item_deleted=true when the parent is soft-deleted; the StorageTab renders the title with strike-through + a small "deleted" badge instead of a clickable link (which would 404). Tests: - store-level: admin/full-access sees the row + ItemDeleted flag, restricted-to-correct-collection sees it, restricted-to-other- collection does not. - (existing TestDeleteAttachment_AfterParentSoftDeleted continues to pass on the handler side.)	2026-04-29 17:44:12 -04:00
xarmian	335762c2bf	feat(attachments): storage usage API + effective-limit computation (TASK-881) (#302) ... * feat(attachments): storage usage API + effective-limit computation (TASK-881) Adds GET /api/v1/workspaces/{ws}/storage/usage returning {used_bytes, limit_bytes, plan, override_active}. Resolves the effective limit through the existing three-tier chain (per-user override → platform setting → hardcoded plan default) and surfaces the override flag for the upcoming Settings → Storage and admin user-detail UIs. Implementation: - store.WorkspaceStorageInfo consolidates SUM(size_bytes) + owner-plan resolution in one call; WorkspaceStorageLimit is now a thin wrapper so the upload-time quota check and the API path stay consistent. - Server.storageInfoCache is a 30s TTL memoizer to absorb repeated Settings → Storage page loads. Invalidation hooks fire on upload, thumbnail derivation, and transform — the ~30s eventual-consistency window is bounded by TTL only when invalidation isn't reachable. - Defensive copy on cache read so a caller mutating the returned struct can't poison subsequent reads. - New CLI command `pad workspace storage` prints "X used of Y (Z%)" with IEC units (humanBytes helper) and surfaces the override flag. - TS api.attachments.storageUsage() + WorkspaceStorageInfo type ready for TASK-882's Settings → Storage page consumer. Tests: - Store-level: no-owner fallback, free-plan resolution chain, override flip, pro-plan override-active visibility, soft-delete exclusion. - Server-level: empty-workspace happy path, two uploads with cache invalidation between, dedicated cache TTL/invalidate/copy-safety test. Parent: PLAN-866. * fix(attachments): gate storage usage on viewer+ per Codex review (round 1) Codex correctly flagged that the storage/usage handler relied solely on RequireWorkspaceAccess, which admits item-grant guests with workspaceRole=="guest". Workspace-wide quota numbers (used_bytes, plan, override status) shouldn't surface to guests — every other workspace- level read handler uses requireMinRole("viewer") for exactly this case. Adds the explicit gate + a regression test that calls the handler with a guest-role context and asserts 403.	2026-04-29 17:05:19 -04:00
xarmian	9e8fec93ff	fix(ci): bump golang.org/x/image to v0.39.0 to clear 5 govulncheck CVEs (#300) ... govulncheck flagged 5 vulnerabilities reachable through the new attachments image processor (TASK-878), all in the golang.org/x/image module that disintegration/imaging pulls in transitively. We were stuck on the ancient v0.0.0-20191009234506-e7c1f5e7dbb8 because nothing else explicitly required a newer version. - GO-2026-4815: OOM from malicious IFD offset in tiff (fix v0.38.0) - GO-2024-2937: Panic on invalid palette-color images (fix v0.18.0) - GO-2023-1990: Excessive CPU on 0-height tiff images (fix v0.10.0) - GO-2023-1989: Excessive resource consumption in tiff (fix v0.10.0) - GO-2023-1572: DoS via crafted tiff image (fix v0.5.0) go get golang.org/x/image@latest landed v0.39.0, which fixes all five. golang.org/x/text bumped 0.35.0 → 0.36.0 as a transitive ride-along. Verification: go build ./... — clean go test ./... — pass govulncheck ./... — "No vulnerabilities found" This closes the last CI gap: PR #299 (gofmt + race-timeout) cleared the lint and PostgreSQL race-step failures; this clears the third red light. Race step on PR #299's merge run finished in 19m36s ✓ under the new 30m cap.	2026-04-29 16:06:25 -04:00
xarmian	756d91acad	fix(ci): gofmt + bump race-detector timeout to 30m (#299) ... CI on main has been failing since the PLAN-866 attachment work landed. Two independent issues: 1. gofmt failures (golangci-lint) — seven files in the attachments path had trailing-comment alignment that gofmt wanted nudged a column. Pure whitespace; ran `gofmt -w` across the affected files. golangci-lint's gofmt linter caught it on every PR / push since TASK-870 but we hadn't been watching those signals. Files cleaned: internal/attachments/{fs_store_test,mime, mime_test,processor_test}.go, internal/server/{ handlers_attachments_download_test,handlers_attachments_transform, render/attachments_test}.go. Local guard: `gofmt -l ./...` now exits clean. 2. Race-detector tests timed out at 20m on the GitHub-hosted runner. Two contributors: - PostgreSQL adds latency on every CREATE/DROP plus on the bcrypt hash inside auth/bootstrap (~3s per call under -race on the runner). Tests that bootstrap a fresh user (e.g. TestSessionIPChange_*) pay the full cost each time. - The PLAN-866 image-processing tests (thumbnail derivation, rotate / crop transform) added ~2-3 minutes of decode/encode work on top of the existing suite. The previous "20m gives margin without papering over a hang" comment was right at the time it was written; we now genuinely need more headroom. Bumped to 30m on both the SQLite and PostgreSQL race steps. Genuine deadlocks would still trip this and produce the goroutine-dump panic — we just stop confusing "slow but progressing" with "permanently hung". Reference points before / after: - TASK-875 main run #294: Go (PostgreSQL) finished in 17m48s ✓ - TASK-880 main run #298: Go (PostgreSQL) hit 20m timeout ✗ - Local: my new tests under -race add ~63s on a developer laptop (TestThumbnails + TestTransform + TestProcessor combined). Verification: go test ./... — pass go vet ./... — clean gofmt -l (recursively) — clean	2026-04-29 15:41:12 -04:00
xarmian	3bf1b60365	feat(attachments): editor image crop with aspect presets (TASK-880) (#298) ... Adds a drag-to-crop modal on top of the AttachmentImage toolbar introduced in TASK-879. The /transform endpoint already accepted the "crop" operation shape from TASK-879 — this PR wires the editor UI plus the supporting tests. Editor: - attachment-crop-modal.ts (new): pure-DOM crop modal in the same style as the existing image lightbox. Returns a Promise that resolves to the crop rect in ORIGINAL-IMAGE pixel coordinates when the user clicks Apply, or null on cancel / dismiss / image-load failure. - Image fits to a centered <dialog> via flex layout; backdrop click and Esc both cancel cleanly. - Crop rectangle starts at 80% of the image, centered. Body is a "move" handle; four corner handles resize. - Aspect presets: Free, 1:1, 4:3, 16:9. Preset clicks snap the current rect to the new ratio while preserving its center; subsequent corner drags clamp to the locked ratio. - Pointer events (touch + mouse for free) with setPointerCapture so drag continues even if the cursor leaves the handle. - Coordinate translation: rect in preview-pixel space → naturalWidth / offsetWidth scale → original-image pixel space. Result is clamped to natural bounds so a fractional- rounding overrun doesn't push the rect off-image. - attachment-image.ts: extracts swapNodeUuid() helper from runRotate so runCrop can share the setNodeMarkup + invalidate-old-metadata flow. The toolbar gains a fourth button (⌶ Crop…) that opens the modal pointed at the original variant. Per-format gating (refreshToolbarState) treats the crop button identically to the rotate trio — both go through /transform, so a libvips-only format (e.g. WebP on the pure-Go build) disables the whole toolbar with the same explanatory tooltip. - app.css: full styling for the crop modal — header with aspect toolbar, image stage with shadow-cutout overlay around the crop rect, four corner handles, footer with Cancel + Apply. Uses the existing CSS-variable palette so light/dark mode track automatically. Server tests (3 new): - TestTransform_CropProducesNewBlobAtRectDimensions: end-to-end PNG crop, verify the response dimensions AND that the served bytes decode at the same dimensions (guards against an encode-pipeline off-by-one). - TestTransform_CropClipsToImageBounds: rect that extends past the image boundary clips rather than 400ing — the editor's rounding can produce rect+1px past natural width/height in rare fractional-scale cases, and the processor's Crop intersects with image bounds for exactly this reason. - TestTransform_CropRejectsBadRect: missing rect, zero width, negative xy, rect entirely outside → 400. Parent: PLAN-866. Closes the editor-side image-tools track on top of TASK-878 (Processor) and TASK-879 (rotate / transform endpoint).	2026-04-29 15:02:07 -04:00
xarmian	f93b0ee4ce	feat(attachments): server-side rotate transform + editor toolbar (TASK-879) (#297) ... * feat(attachments): server-side rotate transform + editor toolbar (TASK-879) Adds the POST /transform endpoint and the editor's rotate toolbar on top of TASK-878's Processor abstraction. Rotation produces a NEW content-addressed attachment row; the editor swaps the AttachmentImage node's UUID via setNodeMarkup and the original ages into orphan GC. Server (internal/server/handlers_attachments_transform.go): POST /api/v1/workspaces/{slug}/attachments/{id}/transform with body {operation, ...params}. Phase 1 wires the "rotate" branch (degrees: 90 | 180 | 270 only — pixel-exact reorderings, no resampling, matches what the editor emits). The "crop" branch is parsed and validated but the transform path is wired in TASK-880; defining the wire format here keeps both PRs aligned. Auth: editor+ on the workspace. Cross-workspace and deleted-parent probes return 404 (not 403) so the new endpoint can't become a side-channel for ID enumeration. Unsupported MIME → 415; oversized image → 413; bad params → 400; missing processor → 503. Output format follows the same PNG-stays-PNG / else-JPEG policy as the thumbnail pipeline so derived blobs deduplicate cleanly. Tests (10): rotate 90 swaps WxH, rotate 180 keeps WxH, bad degrees → 400, unknown op → 400, non-existent attachment → 404, cross- workspace → 404, no processor → 503, derived row has fresh hash + inherits workspace/uploader/item, served bytes decode at the new dimensions, deleted-parent → 404. Web client (web/src/lib/api/client.ts + types): api.attachments.transform(slug, id, payload) hits the new endpoint with a discriminated AttachmentTransformRequest type. New api.server.capabilities() reads the public capability profile added in TASK-878. Both surface PadApiError on failure so the editor can show actionable messages. Editor: - attachment-metadata.ts (new): shared HEAD-probe cache extracted from attachment-chip.ts so AttachmentImage's toolbar can probe the image's MIME with the same zero-extra-network-cost deduplication. Adds mimeToFormat() — maps MIME to the canonical short format name the server's Capabilities reports. - attachment-chip.ts: swapped to use the shared cache. Behavior unchanged. - attachment-image.ts: NodeView now wraps the <img> in a positioned <span> and lazy-builds a 3-button rotate toolbar (rotate left 90°, rotate 180°, rotate right 90°). selectNode shows it; deselectNode hides it. On click → calls options.transform → setNodeMarkup with the returned UUID at getPos(); cached metadata for the OLD UUID is invalidated. Per-button gating via refreshToolbarState: empty supportedFormats list (degraded build) → all disabled with a "this build doesn't have image processing" tooltip. MIME probed and not in supportedFormats → disabled with a format- specific tooltip ("Image editing for image/webp requires libvips"). Otherwise → enabled with the action tooltip. - Editor.svelte: configures AttachmentImage with the workspace slug, the supportedFormats list (initially empty, populated asynchronously after capabilities resolve), and the transform callback wired to api.attachments.transform. Errors surface via console.error + window.alert — same fallback as the upload plugin until a centralized toast system lands. - app.css: wrapper + toolbar styles. Toolbar pinned top-right with absolute positioning; selected-state ring on the image; disabled button state at 40% opacity. Parent: PLAN-866. Unblocks TASK-880 (crop) — the /transform endpoint already accepts the crop op shape, the editor's toolbar pattern is the same, and the supportedFormats gating composes cleanly. * fix(attachments): rotate attribution + toolbar refresh per Codex review (round 1) Two findings from the round-1 Codex review: 1. The transform handler set UploadedBy = currentUserOrSystem(r), contradicting the comment that said "inherit attribution from the parent" and creating an audit-attribution drift whenever a user rotated/cropped someone else's upload. Inherit parent.UploadedBy instead — same policy as the thumbnail pipeline. Added TestTransform_DerivedRowInheritsUploadedByFromParent to lock in the contract. Removed the now-unused currentUserOrSystem helper. 2. The rotate toolbar's per-format gating could permanently stick in "all-disabled" state if the user selected an image before the async capabilities fetch resolved. supportedFormats started as [] (matching "no processor"), refreshToolbarState ran once in that state, and the later mutation of ext.options. supportedFormats had no observer to push the change down to already-open toolbar DOM. Fix: module-level toolbarRefreshers set, populated by each NodeView at ensureToolbar() and torn down in destroy(); a new notifyAttachmentImageCapabilitiesChanged() export iterates the set and re-runs each toolbar's refresh hook. Editor.svelte calls it after the capabilities fetch updates ext.options.supportedFormats, so any toolbar opened during the in-flight request snaps to its correct state the moment caps arrive. Verification: go test ./internal/server -run TestTransform passes (11 cases now); npm run check passes with the existing 6 warnings.	2026-04-29 14:54:48 -04:00
xarmian	02be33902f	feat(attachments): ImageProcessor interface + pure-Go impl + thumbnail pipeline (TASK-878) (#295) ... * feat(attachments): ImageProcessor interface + pure-Go impl + thumbnail pipeline (TASK-878) Adds the abstraction Phase 1 needs to derive thumbnail variants on upload, with a pure-Go default implementation that keeps Pad's single-binary distribution intact (no cgo). The libvips-tagged build (Phase 2 / Pad Cloud Docker) will replace processor_purego.go with a vips-backed implementation behind the same Processor interface — see DOC-865. internal/attachments/processor.go: Processor interface — Decode(io.Reader)→(image.Image, format), Resize(img, maxLong), Rotate(img, deg), Crop(img, rect), Encode(img, format, w), Capabilities(). Capabilities struct (image_formats, can_transcode, max_pixels) surfaces what the editor needs to gate per-format rotate/crop UI on (TASK-879/880). ErrUnsupportedFormat + ErrImageTooLarge are separate sentinels so callers can distinguish "format not supported" from "image dimensions too big". internal/attachments/processor_purego.go (//go:build !libvips): Uses github.com/disintegration/imaging plus the stdlib decoders. Supports PNG/JPEG/GIF/BMP/TIFF for all ops. WebP/AVIF/HEIC reach Decode and bounce out via ErrUnsupportedFormat — uploads still succeed (the MIME allowlist is the upload gate), but thumbnails skip and the editor disables rotate/crop UI per Capabilities. Memory ceiling: Decode peeks via image.DecodeConfig (header only) before allocating any pixel buffer and rejects images whose width*height exceeds MaxPixelsDefault (8000² = 64MP). At 4 bytes per pixel that caps the decode buffer at ~256 MiB and prevents an attacker uploading a forged 100kx100k claim from OOMing the server. The forged-CRC test exercises this gate. internal/server/handlers_attachments_thumbnails.go: deriveThumbnails(parentID) runs in goAsync after every image upload. Generates thumb-sm (256px long edge) + thumb-md (1024px), each as its own attachments row with parent_id pointing at the original. Server.Stop() drains the goroutine before SQLite closes, so tests can assert post-conditions deterministically. Skip cases: parent deleted (race), source format not supported (logged at debug), source already smaller than the variant's bound, variant already exists (idempotent reruns). Variants count toward workspace storage usage — DOC-865 is explicit about this and TestThumbnails_CountsTowardWorkspaceUsage proves it. Output format policy: PNG inputs stay PNG to preserve transparency; everything else encodes as JPEG q=85. internal/server/handlers_capabilities.go: GET /api/v1/server/capabilities returns the Processor's static capability profile under {image: {...}}. Public route — the editor needs it before login (e.g. shared-item preview surfaces). Reports an empty image-formats list when no processor is wired, signalling the editor to disable rotate/crop UI rather than 500-ing the editor mount. cmd/pad/main.go: wires SetImageProcessor(NewProcessor()) alongside SetAttachments at startup; logs the supported formats so operators know whether they're on the pure-Go or libvips build. Tests: - processor_test.go: 12 unit tests covering capability profile, decode round-trip for PNG/JPEG/GIF, rejection of unsupported formats and oversized images (forged-CRC PNG), resize aspect preservation + pass-through for already-small inputs, rotate multiples-of-90 + negative + 360-modulo handling, crop with bounds clipping + empty-intersection rejection, encode round- trip for PNG/JPEG, ThumbnailFormat/Mime/Ext policy. - handlers_attachments_thumbnails_test.go: 5 integration tests covering thumb-sm + thumb-md generation on PNG/JPEG uploads, skip-when-source-already-small, ?variant=thumb-md serving via the existing GET handler, workspace usage accounting. - handlers_capabilities tests cover the happy path + the no-processor degraded path. Parent: PLAN-866. Closes the thumbnail-fallback gap that TASK-874 / TASK-876 left open (thumb-md URLs were falling back to original because no thumbnails existed). Unblocks TASK-879 (rotation tool) and TASK-880 (crop tool) — both will reuse Processor.Rotate / Processor.Crop with the same Capabilities-driven UI gating. * fix(attachments): make /server/capabilities public per Codex review (round 1) Codex flagged that GET /api/v1/server/capabilities was registered inside the auth-gated API group but missing from isPublicAPIPath, so once any user existed the editor's pre-login fetch would 401 — contradicting the route's "public" register-time intent and breaking the share-preview surface. Fix: add the path to isPublicAPIPath. The handler is read-only, returns a static profile, and has no per-user state, so making it public has no security implication. Added TestServerCapabilities_PublicAfterBootstrap as a regression guard: it bootstraps an admin (so RequireAuth is active) and then fetches the endpoint with no auth cookie, asserting 200. * fix(attachments): make -tags libvips compile per Codex review (round 2) Codex flagged that build tag !libvips on processor_purego.go meant NewProcessor + the Thumbnail* helpers were absent under \`go build -tags libvips\`, so cmd/pad/main.go and the thumbnail handler — which call them unconditionally — broke that build. Two minimal fixes preserving the documented Phase 2 split: 1. Move ThumbnailFormat / ThumbnailMime / ThumbnailExt out of the tagged file and into processor.go (untagged). They're pure format-name policy, not implementation specifics, so both backends share the same definitions. 2. Add processor_libvips.go (//go:build libvips) with a stub NewProcessor that panics at runtime with a clear "Phase 2 hasn't shipped libvips yet" message. The libvips build now compiles; anyone actually instantiating the processor under that tag gets a loud failure rather than a silent degradation. Phase 2 will replace the body with the real govips-v2-backed implementation. Verified: \`go build ./...\` and \`go build -tags libvips ./...\` both clean. Existing tests still pass on the default tag. * fix(attachments): make tests compile under -tags libvips per Codex review (round 3) Codex flagged that running \`go test -tags libvips ./internal/attachments\` or \`./internal/server\` panicked through the libvips NewProcessor stub: processor_test.go and the thumbnail/capability server tests all called NewProcessor() unconditionally, even though the libvips build's stub is intentionally panicking until Phase 2 ships the real implementation. Three minimal fixes: 1. Tag processor_test.go !libvips. It tests the pure-Go implementation specifically — there's no value in running it under libvips, and the stub processor would explode the moment NewProcessor() ran. 2. Tag handlers_attachments_thumbnails_test.go !libvips. Same reasoning — these integration tests assert thumbnail derivation against a working processor. 3. Split testServerWithAttachments's processor wiring into two build-tagged helper files: * testimageprocessor_purego_test.go (//go:build !libvips) wires the real pure-Go processor. * testimageprocessor_libvips_test.go (//go:build libvips) is a no-op so the rest of the server test surface (uploads, downloads, auth, etc.) compiles + runs cleanly under -tags libvips. Verification: go build ./... — OK go build -tags libvips ./... — OK go test ./internal/attachments ./internal/server (default) — pass go test -tags libvips ./internal/server -run "TestUpload|TestDownload" — pass Phase 2 will introduce a real libvips test backend and drop the !libvips tags on the thumbnail tests. * fix(attachments): libvips binary boots cleanly per Codex review (round 4) Codex flagged that the libvips build still crashed at \`pad serve\` startup: cmd/pad/main.go calls attachments.NewProcessor() unconditionally, and the libvips stub was panicking — so any operator who built with -tags libvips today (Phase 2 isn't shipped yet) lost the entire server, not just image processing. Two minimal changes: 1. processor_libvips.go: stop panicking. Return nil + slog.Warn instead. Every call site already nil-checks the processor (the upload handler skips thumbnail derivation, the capabilities endpoint reports a degraded empty formats list), so the libvips-tagged binary now has the same runtime profile as a self-host build that opted out of image processing entirely — uploads succeed, originals display, only derived transformations are unavailable. The slog.Warn keeps the "this build doesn't have it yet" signal loud. 2. cmd/pad/main.go: skip srv.SetImageProcessor when NewProcessor returns nil, and log a "not wired" message in that branch. Distinguishes the wired vs. unwired states cleanly in the boot log. Phase 2 will replace processor_libvips.go's body with the real govips-v2-backed implementation; main.go's wiring is already shape- correct for that transition. Verification: go build ./... — OK go build -tags libvips ./... — OK go test ./... — pass (74s server tests included) go test -tags libvips ./internal/server -run "TestUpload|TestDownload|TestServerCapabilities_Public" — pass	2026-04-29 14:35:49 -04:00
xarmian	7e5b15722f	feat(attachments): editor paste + drag-drop upload plugin (TASK-875) (#294) ... Tiptap extension that intercepts paste/drop events with files, uploads each through the attachment API, and replaces the placeholder with the right node (attachmentImage for image MIMEs, attachmentChip for everything else) at the position the user dropped. The plugin's flow: 1. Detect file payloads in clipboardData.items / dataTransfer.files (skip the event when there are none, so plain text paste / cursor drag still go through tiptap-markdown's transformPastedText path). 2. Insert a position-tracked placeholder at the paste/drop position via a setMeta transaction. Placeholders are widget decorations (zero document width) so they never enter serialized markdown even if the user navigates away mid-upload. 3. Race the network. The plugin's apply() handler maps every placeholder position through every intervening transaction, so continued editing doesn't strand the spinner. 4. On success: schema-aware replacement — attachmentImage for category=image, attachmentChip otherwise. The placeholder is removed in the same transaction. 5. On error: remove the placeholder and surface the failure via the injected onError callback. The upload bytes that did land become orphans; orphan-GC reclaims them after the grace period. 6. If the placeholder has been deleted before the upload completes (user cancelled, navigated away, etc.) the upload is dropped silently — same orphan-GC outcome. Multiple files in a single drop fan out as concurrent uploads; each gets its own placeholder and replaces independently as the network completes. Editor.svelte wires: - upload -> api.attachments.upload(workspaceSlug, file) (rejects with a clear message when no workspace context) - onError -> console.error + window.alert as a minimal fallback until a centralized toast system lands. Styles (app.css) cover the placeholder bubble (dashed border, faded colour) and a CSS spinner — kept inline via decoration widget DOM so ProseMirror's selection ignores it (ignoreSelection: true). Parent: PLAN-866. Closes the editor-input flow on top of TASK-874 (markdown resolver), TASK-876 (image node), TASK-877 (chip node).	2026-04-29 13:41:05 -04:00
xarmian	934794b606	feat(attachments): editor file chip node (TASK-877) (#293) ... * feat(attachments): editor file chip node (TASK-877) Custom Tiptap node `attachmentChip` for non-image `pad-attachment:UUID` references — Notion-style chip rendering with type icon, filename, and human-readable size. Node shape: - uuid: string — the attachments-row UUID - filename: string — display name; preserved across save/reload Markdown round-trip: - Serialize: `[filename](pad-attachment:UUID)` — same standard link syntax the markdown resolver in TASK-874 understands. `]` and `\` in the filename are escaped to keep the link label balanced. - Parse: markdown-it's link token produces `<a href="pad-attachment:UUID">filename</a>`. Our parseHTML rule `a[href^="pad-attachment:"]` runs at priority 1000 to beat SafeLink's default mark rule (priority 50), so attachment refs become a chip Node instead of a Link Mark on plain text. Editor display (NodeView): - <a class="file-chip"> with icon + name + optional size span - Icon: filename-extension heuristic on first paint, upgraded to a MIME-based icon once a single HEAD request resolves the canonical Content-Type. The HEAD goes against the existing GET handler — no new API endpoint required, and Go's net/http strips the body automatically for HEAD. - Size: rendered from Content-Length once HEAD resolves; hidden until then (CSS `:empty { display: none }`). - Module-level Promise cache keyed by `${ws}:${uuid}` deduplicates repeated chips for the same attachment and survives undo/redo without re-fetching. - target=_blank + download attribute so a click opens / saves the file with its canonical filename. - atom: true → Backspace/Delete remove the chip as a single unit. Styles in app.css (not Editor.svelte) so they reach the read-only markdown render path too — TASK-874's Go and TS resolvers emit the same `.file-chip` / `.file-chip-icon` / `.file-chip-name` / `.file-chip-size` markup. Parent: PLAN-866. Unblocks TASK-875 — the upload plugin needs a chip node to insert on successful non-image uploads. * fix(attachments): chip HEAD route + chip click handler per Codex review (round 1) Two findings from the round-1 Codex review: 1. chi router does not auto-route HEAD to GET handlers, so the chip's metadata HEAD probe was returning 405 and chip size + MIME-refined icons never loaded. Fix: register HEAD on the same path/handler; http.ServeContent already strips the body for HEAD on the seekable path, and the streaming fallback short-circuits before io.Copy so future S3-style backends don't burn GetObject bandwidth on HEAD. Tests added: HEAD returns 200 with Content-Type + Content-Length and an empty body; HEAD cross-workspace returns 404 (not 403) so the new endpoint can't become a side-channel for ID enumeration. 2. Editor.svelte installs a global anchor-click suppressor that preventDefaults every <a> inside the editor, so the chip looked clickable but did nothing in edit mode. Fix: the chip's NodeView now attaches an explicit click handler that calls window.open with the download URL and stops propagation before the global handler runs. Mirrors the AttachmentImage lightbox click pattern.	2026-04-29 13:35:15 -04:00
xarmian	f1ce9ca24a	feat(attachments): editor inline image node (TASK-876) (#292) ... Custom Tiptap node for inline `pad-attachment:UUID` image references. Stores the attachment UUID (not a backend URL) so item content survives a storage-backend migration untouched. See DOC-865. Node shape: - uuid: string — the attachments-row UUID (required) - alt: string — preserved across save/reload for accessibility Markdown round-trip: - Serialize:  via tiptap-markdown's addStorage.markdown.serialize, with [/] in the alt text escaped so brackets stay balanced. - Parse: markdown-it's default image token already produces <img src="pad-attachment:UUID" alt="…">, captured by parseHTML rule img[src^="pad-attachment:"]. The alternate parseHTML rule img[data-attachment-id] catches editor-rendered HTML on copy/paste. Editor display: - addNodeView renders <img class="attachment-image" loading="lazy"> pointing at /api/v1/workspaces/{ws}/attachments/{id}?variant=thumb-md via an injected getDownloadUrl callback (Editor.svelte resolves the workspace slug from page.params at mount time, falling back to the workspace store). - Single-click opens a native <dialog> lightbox with the original- resolution variant; multi-click events fall through so users can drag-select around the image. - atom: true means Backspace/Delete remove the image as a single unit and the cursor never lands inside the node. Lightbox styles live in app.css because the <dialog> is appended to document.body, outside Editor.svelte's scoped style block. The configure() default returns the literal `pad-attachment:UUID` href — sufficient for markdown round-trip in headless / SSR contexts and a clearly-broken render in any environment that hasn't wired the URL builder, which is the right signal to fix. Parent: PLAN-866. Unblocks TASK-875 (the upload plugin needs a node to insert on success).	2026-04-29 13:22:13 -04:00
xarmian	5af54ddc05	feat(attachments): markdown reference resolver for pad-attachment:UUID (TASK-874) (#291) ... * feat(attachments): markdown reference resolver for pad-attachment:UUID (TASK-874) Add the shared step that translates `pad-attachment:UUID` markdown references into rendered HTML for image embeds, file chips, and missing placeholders. Wired into the editor preview path; Go-side helpers seed the future server-side rendering pipeline (export / shared item view). TS side (`web/src/lib/markdown/attachments.ts`): - Pure helpers: parseAttachmentHref, attachmentDownloadUrl, isImageMime, formatAttachmentSize, renderAttachmentImage/Chip/Missing - resolveAttachmentImage / resolveAttachmentLink for the marked hooks - Image MIME → <img src=...?variant=thumb-md data-attachment-id=...> - Non-image MIME (or link syntax) → file chip with download attribute - Missing/deleted → "Missing attachment" placeholder span `web/src/lib/utils/markdown.ts`: - renderer.image override (defaulting to marked's standard image when href is not pad-attachment:) - renderer.link checks for pad-attachment: prefix before the existing external/internal-link logic - renderMarkdown gains an optional attachmentResolver parameter; the resolver is threaded via a per-call module slot (synchronous render) - DOMPurify allowlist extended with data-attachment-id, download, width, height — ALLOW_DATA_ATTR stays false so only this single data-* attribute slips through Go side (`internal/server/render/attachments.go`): - Mirror of the TS API so server-rendered output matches client output byte-for-byte for the same input - ResolveAttachmentReferences scans markdown source via regex, skipping fenced code blocks (backtick + tilde), substitutes both image and link forms - Comprehensive table-driven tests (24 cases) covering: href parsing, URL building, MIME detection, size formatting, image/chip/missing rendering, escape safety against script-tag injection in alt / filename / display text, fenced-code skip, tilde fences, title suffix on link destinations, nil resolver pass-through, no false positives on non-attachment URLs, deterministic round-trip References are stored as opaque `pad-attachment:UUID` so a backend migration (FS → S3) can rewrite storage_keys without touching item content. See DOC-865 for the architecture. Parent: PLAN-866 (Attachments Phase 1). * fix(attachments): chip label double-escape + escaped-bracket lockstep per Codex review (round 1) Two findings from the round-1 Codex review: 1. TS chip labels were double-escaped. renderer.link was passing the parseInline(tokens) HTML output to resolveAttachmentLink, which feeds it into renderAttachmentChip → escapeHtml. A label like `[**Report**](pad-attachment:id)` rendered literal `&lt;strong&gt;Report&lt;/strong&gt;` instead of plain text. Switched to the link token's raw `text` field; markdown emphasis inside chip labels now degrades to literal markers (acceptable for filename-style labels) and matches what the Go regex extracts. 2. Go regex didn't accept CommonMark `\]` / `\\` escapes inside link/image labels, so `[Q1 \] report](pad-attachment:id)` resolved on the TS side (marked handles escapes) but stayed literal on the Go side — breaking the documented lock-step contract. Updated the regex to accept escaped characters inside the alt/text capture, and added unescapeMarkdownText to mirror marked's behavior of dropping the backslash before the label reaches the render helpers. Tests added: TestResolveAttachmentReferences_EscapedBrackets covers image alt, link text, and combined backslash/bracket escapes; TestUnescapeMarkdownText is the unit-level table for the unescape helper (including dangling-backslash and non-punctuation pass-through).	2026-04-29 13:09:42 -04:00
xarmian	fc1c47f124	feat(attachments): CLI + TypeScript clients + types (TASK-873) (#290) ... * feat(attachments): CLI + TypeScript clients + types (TASK-873) Rounds out the API surface with Go and TS client methods + a \`pad attachment\` Cobra subcommand for ops debugging. internal/cli/client.go AttachmentUploadResult struct mirrors POST /attachments JSON. UploadAttachment streams a multipart file part via io.Pipe — never buffers the upload in memory. itemRef is optional. Uses a fresh http.Client with a 5-minute timeout per request so a 25 MiB upload over a constrained link doesn't trip the package-shared 10s default. DownloadAttachment streams the bytes into the caller's writer, returning Content-Type + total bytes copied. Optional ?variant= parameter for thumbnails (server falls back to original silently per TASK-872). cmd/pad/main.go pad attachment upload <item-ref|-> <path> [--filename NAME] pad attachment download <id> <out|-> [--variant thumb-sm|thumb-md] Item arg accepts an issue ref (TASK-5) or slug; "-" means no parent. Out arg "-" streams to stdout (with status messages on stderr) so callers can pipe into image viewers etc. Resolves the item via GetItem first so a typo'd ref fails fast with a useful error. List + delete subcommands intentionally omitted — those endpoints ship with TASK-881 (storage usage) and the future GC task. Adding client methods that hit 404s would mislead callers; same logic kept the upload response's "url" out of TASK-871 until TASK-872 wired GET. web/src/lib/types/index.ts Attachment interface mirroring the Go model (pointer types → optional). AttachmentUploadResult interface for the upload response shape. web/src/lib/api/client.ts api.attachments.upload(workspaceSlug, file, itemId?) — multipart POST via direct fetch (skips shared request() because that helper hard-codes Content-Type: application/json). Carries CSRF, cookies, and the same 401 → /login redirect. api.attachments.downloadUrl(workspaceSlug, attachmentId, variant?) is a pure URL builder so callers can wire <img src> directly without going through fetch. End-to-end smoke verified: pad attachment upload TASK-869 /tmp/tiny.png # uploads PNG pad attachment download <id> /tmp/dl.png # bytes are identical cmp /tmp/tiny.png /tmp/dl.png # PASS Verification go build ./... — clean go vet ./... — clean go test ./... — all packages pass cd web && npm run build — clean make install — server restarts on the new binary Parent: PLAN-866. * fix(cli): atomic download — write to temp + rename so a failed download doesn't truncate the destination per Codex review (round 1) P2: pad attachment download <bad-id> /existing/file used to wipe the existing file on auth/network/404 errors because os.Create truncated before the request was even attempted. The bytes were never written because DownloadAttachment errored out, but the destination was already 0 bytes — a footgun for anyone running the CLI in scripts. Fix: for the file-path case, write to a sibling .tmp via os.CreateTemp in the destination directory, fsync, close, then os.Rename only on success. Same atomic-write pattern as FSStore.Put. The defer cleans up the .tmp on any error path. The stdout case (outPath == "-") is unchanged — bytes already streamed to stdout can't be rolled back, so any partial write is just visible to the caller as a short payload. Verified end-to-end: echo X > /tmp/existing.png pad attachment download not-a-real-id /tmp/existing.png # errors cat /tmp/existing.png # still "X" — file untouched * docs(cli): clarify os.Rename atomic-replace behavior on Windows (Codex round 2 disagreement) Round 2 flagged this as P2: "os.Rename does not replace an existing destination on Windows." That is technically incorrect for modern Go. Verified directly against the Go stdlib source: src/internal/syscall/windows/syscall_windows.go: func Rename(oldpath, newpath string) error { ... return MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING) } MoveFileEx with MOVEFILE_REPLACE_EXISTING atomically replaces an existing destination on Windows. This has been the behavior since Go 1.5 (2015), so every version of Go this codebase supports already gets the desired replace-on-rename semantics on every platform. Added an inline code comment so future readers don't worry about the same false alarm. No code-path change.	2026-04-29 12:48:57 -04:00
xarmian	00baf75576	feat(attachments): download/serve API with auth + Range support (TASK-872) (#289) ... Adds the GET endpoint that pairs with TASK-871's upload. Streams the blob from the resolved storage backend with proper headers, Range support, and cross-workspace defense. GET /api/v1/workspaces/{slug}/attachments/{attachmentID} Optional ?variant=thumb-sm|thumb-md - 200 inline render for images / video / audio / PDF / etc. - 200 attachment download for HTML / JS / forced-download MIMEs - 206 Partial Content on Range requests (video/audio seek) - 304 Not Modified on conditional GETs (If-Modified-Since etc.) - 400 unknown variant - 404 missing attachment OR cross-workspace probe (not 403, to avoid leaking existence of attachments in other workspaces) - 404 blob_missing if DB row exists but on-disk blob is gone (logs a warning since this is a "shouldn't happen" state) - 503 if attachments registry not configured internal/server/handlers_attachments.go handleGetAttachment looks up the row, gates cross-workspace via 404, optionally swaps to a derived variant via GetAttachmentVariant (silent fallback to original when the variant row doesn't exist yet — TASK-878 will populate them; this handler shipping today doesn't have to wait), resolves the storage backend via Registry, and hands off to http.ServeContent when the body satisfies io.ReadSeeker. FSStore returns *os.File so that's the common path and gets us Range / 206 / conditional GETs for free. Backends without Seek (a future S3 streaming reader) fall through to a plain io.Copy with no Range support — the contract is "Range works when the backend supports it, never breaks correctness". Headers: Content-Type from att.MimeType (already canonical post-allowlist) Content-Disposition: inline | attachment, filename sanitized to strip quotes/backslashes/control bytes (header-injection defense on top of the upload-time basenaming) Cache-Control: private, max-age=3600 (Phase 3 revisits for CDN) X-Content-Type-Options: nosniff (browser should never re-sniff; we already validated MIME at upload) Upload response now includes "url" again — TASK-871 had dropped it because the GET handler didn't exist yet. Slug-form path matches every other API endpoint. internal/store/attachments.go GetAttachmentVariant(parentID, variant) for the ?variant lookup. internal/server/server.go GET /workspaces/{slug}/attachments/{attachmentID} wired alongside the existing POST. Tests Happy-path PNG, HTML force-download, 404 missing, cross-workspace 404 (NOT 403), Range 206 with bytes 10-29 of an MP4 payload, variant fallback to original, unknown variant rejected, derived thumb-sm row honored when present, blob-missing 404, and the filename sanitizer table. Verification go build ./... — clean go vet ./... — clean go test ./... — all packages pass make install — server restarts on the new binary Parent: PLAN-866.	2026-04-29 12:34:36 -04:00
xarmian	48b9e18d34	feat(attachments): upload API with MIME sniff + dedupe + quota tracking (TASK-871) (#288) ... * feat(attachments): upload API with MIME sniff + dedupe + quota tracking (TASK-871) Wires the upload endpoint that turns a multipart POST into an attachments row plus a stored blob. Auth-gated (editor+), per-file size cap, hash-streaming, MIME allowlist with extension blocklist, fire-and-forget quota warning. POST /api/v1/workspaces/{slug}/attachments Multipart "file" field. Optional ?item_id=… or form item_id to associate at upload time. Returns {id, url, mime, size, width?, height?, filename, category, render_mode}. Errors: 400 bad multipart, 400 empty file, 401 unauthorized, 403 insufficient role, 413 over per-file cap, 415 MIME or extension rejection, 503 attachments not configured. internal/attachments/mime.go MIMEEntry + RenderMode + Category typed allowlist mirroring DOC-865. Default-deny. SniffMIME wraps http.DetectContentType. ValidateUpload cross-checks the sniff result against the filename extension and: (a) rejects when the extension maps to a *blocked* MIME — covers .svg (sniffs as text/xml; .svg ext makes the browser run embedded <script>) and .exe family (sniffs vary; extension is unambiguous); (b) rejects when the extension maps to an allowed MIME but the sniff's category disagrees — the "exe pretending to be png" case. Tests cover normalize/lookup/sniff plus happy path, exe-as-png, extension mismatch, SVG, .exe-by-extension-alone, text/plain accept, HTML force-download. internal/store/attachments.go CreateAttachment / GetAttachment / WorkspaceStorageUsage. Pointer scan for nullables; SUM(size_bytes) excludes soft-deleted rows but includes derived blobs (thumbnails are real bytes on disk). internal/server/handlers_attachments.go Body capped via http.MaxBytesReader BEFORE ParseMultipartForm spools any of it. Streams "file" part into an os.CreateTemp file, sha256ing in one io.MultiWriter pass — multi-GB POST never reaches RAM. Sniff on first 512 bytes; image dimension probe via stdlib image.DecodeConfig (PNG/JPEG/GIF). WebP/AVIF/HEIC accepted but width/height nil — matches the "pure-Go gracefully degrades" decision in DOC-865. Calls AttachmentStore.Put (which hash-verifies via the dedup fast path) and inserts the row. Quota check (CheckLimit + WorkspaceStorageUsage) runs in a goroutine — Phase 1 logs only; Phase 2 will enforce. Anonymous uploads on a fresh install (RequireWorkspaceAccess grants implicit owner without a current user) get uploaded_by="system". internal/server/server.go Server.attachments + attachmentMaxBytes fields and SetAttachments setter. Route POST /workspaces/{slug}/attachments wired inside the authenticated workspace block. cmd/pad/main.go Boot wiring: NewFSStore(<DataDir>/attachments) → Registry registered under "fs" → SetAttachments. PAD_ATTACHMENT_MAX_BYTES env override for the per-file cap. Tests internal/server/handlers_attachments_test.go covers: happy path PNG (1x1, dimensions resolve to 1×1) exe bytes with .png filename → 415 PNG bytes with .pdf filename → 415 (extension mismatch) empty body → 400 missing file part → 400 over the size cap → 413 same content uploaded twice → two rows, same content_hash + storage_key, WorkspaceStorageUsage = 2 × bytes (dedupe is at the blob layer, not the row layer) 8 concurrent uploads of identical bytes → all 201, no corruption no registry wired → 503 Verification go build ./... — clean go vet ./... — clean go test ./... — all packages pass make install — server restarts on the new binary Parent: PLAN-866. * fix(attachments): three Codex round-1 findings — drop premature url, accept Office docs, real quota probe 1. Upload response no longer returns "url". TASK-872 wires GET so any URL we return today is a 404 — pulling it out keeps clients from baking in the broken endpoint. 2. Office Open XML docs (.docx/.xlsx/.pptx) and OpenDocument formats (.odt/.ods/.odp) are zipped XML — http.DetectContentType correctly sniffs them as application/zip. Previously the validator's extension-vs-sniff category check rejected them as "mime_extension_mismatch" (archive vs document). Now: when the sniffed type is exactly application/zip and the extension maps to a document MIME, trust the extension and route to the document entry. Plain .zip with the same bytes still routes to archive. Test covers all six office/odf extensions plus the plain-zip case. 3. CheckLimit("storage_bytes") returned "unknown workspace feature" because featureCount only knows row-counted features (items, members, webhooks). The warning path silently dropped every probe. Added Store.WorkspaceStorageLimit which does the same three-tier resolution (user override → platform setting → hardcoded fallback) but returns the limit only — usage is computed separately via the existing WorkspaceStorageUsage. Self-hosted/pro plans return -1 (unlimited). Workspaces without an owner_id (fresh installs and legacy rows) also return -1, so a fresh-install upload no longer logs "owner not found". Switched maybeWarnStorageQuota to use WorkspaceStorageLimit + WorkspaceStorageUsage directly. Now also spawned via Server.goAsync so Stop() drains it (BUG-842 hygiene). Tests - TestValidateUpload_AcceptsOfficeOpenXMLAsZipBytes covers all six extensions + plain .zip - TestUpload_QuotaCheckResolves regression-tests finding 3: both storage helpers return non-error after a real upload - TestUpload_HappyPathPNG asserts the response no longer carries url Verification go build ./... — clean go vet ./... — clean go test ./... — all packages pass * fix(attachments): trim trailing blank line at EOF in mime.go per Codex review (round 2) Round 2 LOW: git diff --check flagged a "new blank line at EOF" on internal/attachments/mime.go. Cosmetic but addressed because the ship-tasks workflow requires zero findings (HIGH/MEDIUM/LOW alike) — leaving LOWs unfixed compounds across PRs and prevents the loop from ever converging clean on later work. * fix(attachments): alias stdlib MIME-sniff quirks per Codex review (round 3) http.DetectContentType returns names that don't match modern IANA conventions for two formats on the allowlist: audio/wave → audio/wav (.wav uploads) application/x-gzip → application/gzip (.gz uploads) Without aliasing, valid uploads of either format hit "mime_not_allowed" because the allowlist uses canonical names. Added a sniffAliases map applied inside SniffMIME so allowlist lookups always see the canonical form. Allowlist stays single-sourced; the fix is one map entry per quirk we discover. Tests: - TestSniffMIME_AliasesStdlibQuirks pins both aliases at the sniff layer - TestValidateUpload_AcceptsWAV / TestValidateUpload_AcceptsGzip verify the end-to-end accept path with real WAV (RIFF/WAVE) and gzip headers	2026-04-29 12:19:06 -04:00
xarmian	de4d28d576	feat(attachments): AttachmentStore interface + FSStore (TASK-870) (#287) ... * feat(attachments): AttachmentStore interface + FSStore (TASK-870) Introduces the storage backend abstraction described in DOC-865 and ships its first concrete implementation. No call sites yet — TASK-871 (upload API) wires it in. internal/attachments/store.go AttachmentStore interface (Put/Get/Stat/Delete) and ErrNotFound sentinel. Put is documented as idempotent — concurrent Puts of the same hash converge — and required to verify that the streamed bytes actually hash to the supplied value. internal/attachments/registry.go Registry routes "<prefix>:<rest>" keys to the store registered for that prefix (Phase 1 = "fs"; Phase 2 will register "s3" alongside). Convenience Get/Stat/Delete helpers resolve + forward in one call so callers don't have to spell out the two-step pattern everywhere. Register panics if the prefix contains ':' since that would make the store unreachable. internal/attachments/fs_store.go FSStore writes to <baseDir>/<aa>/<bb>/<full-hash> with the first 4 hex chars sharding the directory tree two levels deep. Atomic writes: stream + hash to a randomized .tmp in the destination dir, fsync, then intra-directory rename. The streaming sha256 is verified against the supplied hash before the rename, so a mismatch never leaves a visible file. Idempotent fast path: if the canonical file already exists Put short-circuits (and drains the reader so callers don't get a stuck stream). Get returns wrapped ErrNotFound on missing keys; Delete on a missing key is a no-op (matches what the orphan GC needs). Tests cover put/get/stat/delete, hash mismatch, invalid hash format, idempotency, 16-goroutine concurrent Put of the same hash converging to one on-disk file with no orphan tmp files, registry routing, forward-error semantics, and the prefix-with-colon panic. Parent: PLAN-866. * fix(attachments): validate hash on every FSStore key + verify on fast path per Codex review (round 1) Round 1 raised two issues — both real, both fixed. 1. Path traversal in Get/Stat/Delete. extractHash only checked that the key began with "fs:" and the suffix was non-empty before passing it to pathFor(), which used the suffix as a path component. A key like "fs:../../etc/passwd" would escape baseDir for reads/stats/deletes. Fix: extractHash now requires the canonical 64-char lowercase-hex sha256 form via validHash. Same gate that Put already used; now it covers every public method. 2. Idempotent Put fast path skipped hash verification. If the canonical target file already existed, Put returned the key without checking that the supplied reader's bytes hashed to the supplied hash — violating the AttachmentStore.Put contract that implementations MUST verify on every call. A buggy upload path could associate the wrong bytes with an existing hash and silently succeed. Fix: stream r through a hasher when the target exists (no disk I/O), compare against the supplied hash, and reject on mismatch. Also dropped the dead "_short" branch in pathFor — every caller now goes through validHash. Tests added: - TestFSStore_GetStatDeleteRejectBadKeys covers empty/wrong-prefix/empty- hash/non-hex/wrong-length/path-traversal/path-separator/uppercase keys across all three read methods. - TestFSStore_PutFastPathStillVerifiesHash confirms the contract holds on the fast path: a second Put that lies about the hash is rejected with no corruption of the existing file.	2026-04-29 11:44:58 -04:00
xarmian	6461aafd16	feat(store): attachments table + Attachment model (TASK-869) (#286) ... Adds the schema groundwork for inline images and file uploads — see DOC-865 (Attachments — architecture & migration design). - migrations/047_attachments.sql — SQLite migration. Table + 4 indexes (workspace, item, hash, parent). Partial indexes on workspace/item/parent match the items table convention. The hash index is full (not partial) so dedupe can resurrect a soft-deleted blob if the same bytes are re-uploaded without writing a duplicate. - pgmigrations/026_attachments.sql — Postgres mirror with BIGINT for size_bytes; same partial-index pattern. - internal/models/attachment.go — Go model with all columns. Uses pointer types for nullable columns (item_id, width, height, parent_id, variant, deleted_at) so JSON omitempty works correctly. No call sites yet — purely schema groundwork. Verified the migration runs cleanly on a fresh install and on the live dev DB. Parent: PLAN-866.	2026-04-29 11:34:35 -04:00
xarmian	2f58193f22	chore(web/connect-modal): point footer + install links at getpad.dev/docs (#285) ... Last piece of PLAN-859. The ConnectWorkspaceModal's three footer/install links were placeholders pointing at GitHub README anchors while TASK-863's docs page didn't exist yet. That page is now live at getpad.dev/docs/connect-workspace (pad-web#30 / e472586), so swap the three URLs to the real docs: - "Other install options →" → https://getpad.dev/docs#installation (broader install matrix: Homebrew + Binary + Docker + Source) - "Documentation" → https://getpad.dev/docs/connect-workspace - "Troubleshooting" → https://getpad.dev/docs/connect-workspace#troubleshooting Updated the in-source comment to reflect that the URLs are now the canonical ones, not placeholders. This closes out PLAN-859 (web-first onboarding on-ramp): a user who creates a workspace in the web UI now has a complete in-app + docs path to connecting that workspace to their local project.	2026-04-29 10:26:15 -04:00
xarmian	e5eae5e94e	feat(web): persistent dismissible CLI-connect banner on workspace pages (TASK-862) (#284) ... * feat(web): persistent dismissible CLI-connect banner on workspace pages (TASK-862) Final web piece of the web-first onboarding on-ramp from PLAN-859 / IDEA-750. A slim banner now nudges users to connect their workspace to the CLI on every workspace page, until they either dismiss it or actually do it. Server: - New store method WorkspaceHasCLISource(workspaceID) — backed by EXISTS(... WHERE source='cli' AND deleted_at IS NULL), so it's a cheap O(1) check that short-circuits on the first match. - Dashboard payload (GET /workspaces/{ws}/dashboard) gains HasCLISource bool (json: has_cli_source). - Unit tests cover empty workspace, web/skill items don't trip it, one cli item flips it on, soft-delete flips it back off, and cross-workspace isolation. Web: - New <ConnectBanner> Svelte 5 component (web/src/lib/components/ConnectBanner.svelte). Self-contained: reads dismissed state from localStorage, fetches has_cli_source itself, mounts <ConnectWorkspaceModal> internally. Two split $effect blocks per CONVE-606 — one for the localStorage sync, one for the dashboard fetch — so a workspace change doesn't entangle the two reactive lifecycles. - Banner is hidden while loading (hasCliSource === null) to avoid a flash-then-auto-hide on workspaces that already have CLI items. - Storage key pad-cli-banner-dismissed-${wsSlug} matches the existing onboarding-dismissed pattern. Per-browser only; TODO comment in source about backing it with a workspace_user_state row if cross- device persistence is wanted later. - Mounted in web/src/routes/[username]/[workspace]/+layout.svelte above {@render children()} so it appears on every workspace page (dashboard, collection lists, item detail, search, activity, etc.) and NOT on console/auth pages (the layout is workspace-scoped). - DashboardResponse type in web/src/lib/types/index.ts gains has_cli_source: boolean. Smoke-tested against the running server: the field is live in the dashboard payload and reflects reality (this workspace returns has_cli_source: true since it has many CLI-sourced items, so the banner is correctly auto-hidden here). Test plan: - go build ./... && go test ./... — all green (incl. new TestWorkspaceHasCLISource with 5 sub-cases). - cd web && npm run build — clean. - make install — clean, server restarted. - Svelte MCP autofixer ran on ConnectBanner.svelte — no issues. Parent: PLAN-859. Driving idea: IDEA-750. * fix(web/connect-banner): stale-response guard + refetch on modal close (Codex round 1) Two findings from Codex review on PR #284: 1. Stale-response race: rapid workspace switches could let a slow dashboard fetch from workspace A overwrite hasCliSource for workspace B after the user navigated. Capture the requested slug at fetch time, ignore the response if wsSlug has changed since. 2. Auto-hide didn't work in-session: if a user opened the banner modal, copied the command, ran it elsewhere, and closed the modal, the banner stayed visible because hasCliSource was stale. Refetch when the modal transitions from open → closed (the natural moment the user has just connected). Uses $effect.pre with a tracked previous value, matching the transition pattern in ShareDialog. The 'someone ran the CLI from another terminal without ever opening the modal' edge case is left for a follow-up — would require SSE item-created subscription, which is heavier than this PR's scope. * fix(server/items): persist source from auth context on create (Codex round 2) Codex caught an architectural bug while reviewing the TASK-862 banner work: items created via the CLI were persisting with source='web' (the column default) instead of 'cli', because handleCreateItem decoded ItemCreate from the body — which the CLI doesn't set Source on — and only consulted actorFromRequest AFTER persisting (for SSE / activity log emission). Result: TASK-862's has_cli_source dashboard signal would never flip on for normal CLI usage, so the connect-CLI banner would never auto-hide for users who actually wired up the CLI. Fix: in handleCreateItem, backfill input.Source from actorFromRequest before calling store.CreateItem, but only when the client didn't explicitly set it (so agents marking themselves as 'skill' still pass through unchanged). Test: TestCreateItemSourcePersistedFromAuth covers all three branches - bearer Authorization header → source=cli (uses bootstrap + a real session token in the header since the auth middleware validates token format and rejects fake values with 401 before the handler runs) - cookie session, no Authorization → source=web - explicit source in body wins over auth-derived (e.g. 'skill') * fix(web/connect-banner): seq counter for same-workspace race (Codex round 3) Round 3 caught a same-workspace race the slug guard didn't cover: an in-flight workspace-change fetch that resolves AFTER the modal-close refetch could overwrite the newer 'true' with the older 'false', making the banner reappear after the user actually wired up the CLI. Add a monotonic fetchSeq counter — captured at call time, rechecked before applying the response. Only the LATEST request's result wins, regardless of arrival order. The slug guard stays as a second-layer defense for cross-workspace races. * fix(web/connect-banner): guard banner keydown to currentTarget (Codex round 4) Round 4 caught a keyboard-event bubble: pressing Enter or Space on the dismiss X button also fired the banner-level keydown handler, so the user would dismiss AND open the modal in one stroke. Guard the parent handler with `e.target !== e.currentTarget` so it only reacts to keydown that originated on the banner itself. Tabbing to the dismiss button + Enter now ONLY dismisses. * fix(store): visibility-filter has_cli_source query (Codex round 5) Round 5 caught a P2 information leak: WorkspaceHasCLISource scanned the entire workspace regardless of caller visibility, so a guest with grants only on web-sourced items could still see has_cli_source return true (revealing that CLI items exist somewhere they can't see). That also produced wrong UX — the banner could auto-hide for guests who couldn't actually use the CLI. Extend the query to take optional collectionIDs/itemIDs filters matching the dashboard's existing visibility model: an item counts when its collection is in collectionIDs OR its id is in itemIDs (union — guest item-level grants can expose items in otherwise- hidden collections). Mirrors ListItems' filtering pattern incl. the "non-nil empty CollectionIDs = no visibility = short-circuit false" semantics. Handler now passes dashCollIDs and dashItemIDs to match the rest of the dashboard payload's filtering. New TestWorkspaceHasCLISourceVisibility covers the four cases: unfiltered sees all, visible-coll-only hides CLI items in hidden collections, item-level grant surfaces a hidden CLI item, and empty visibility short-circuits to false.	2026-04-29 10:05:34 -04:00
xarmian	a28767d323	feat(web): ConnectWorkspaceModal + empty-workspace and avatar surfaces (TASK-861) (#283) ... * feat(web): ConnectWorkspaceModal + empty-workspace and avatar surfaces (TASK-861) Web side of the web-first onboarding on-ramp from PLAN-859 / IDEA-750. Gives a user who created a workspace via the web UI a one-line copy-paste to connect that workspace to their local repo, exposed in the two zero-state surfaces where they'd look for it. Changes: - New `<ConnectWorkspaceModal>` Svelte 5 component (web/src/lib/components/ConnectWorkspaceModal.svelte). Reusable, no host-page coupling. Matches ShareDialog's modal pattern (overlay + centered modal, native, open = $bindable(), Escape closes). Props: serverUrl, workspaceSlug, workspaceName?. Renders Step 1 (OS-tabbed install — macOS/Linux/Windows/Docker, default tab from detected platform) and Step 2 (pad init --url ... --workspace ... snippet with a copy button on the full snippet). Footer links to docs + troubleshooting. - New web/src/lib/utils/platform.ts — tiny dependency-free OS detection helper. SSR-safe (defaults to "macos" with no navigator). - Mounted in the workspace landing page as a "Connect your local project" card directly under <OnboardingChecklist> in the empty- workspace .onboarding-wrapper. Modal itself is mounted unconditionally at the page root so it survives re-renders of the conditional empty state. - Mounted in TopBar.svelte's user menu (both desktop and mobile branches): "Connect a project..." entry between Theme/Cloud-support links and the Sign-out divider. Modal lives outside the dropdown so it doesn't unmount when the dropdown closes. Both gated on workspaceStore.current?.slug since the modal needs a workspace to interpolate. Docs URLs in the modal footer (getpad.dev/docs/install, getpad.dev/docs/connect-local-project) are placeholders; TASK-863 in PLAN-859 will publish those pages and we'll wire the final URLs then. Test plan: - go build ./... && go test ./... clean - cd web && npm run build clean - make install clean, server restarted - Svelte MCP autofixer ran on all four touched files — no findings Parent: PLAN-859. Driving idea: IDEA-750. * fix(web/connect-modal): correct brew tap + point placeholder docs links to README (Codex round 1) Two findings from Codex review on PR #283: 1. macOS install command was `brew install xarmian/pad/pad`, but the actual tap is `PerpetualSoftware/tap/pad` (per README.md and skills/INSTALL.md). Users would have hit a failing install. 2. Footer links pointed at `getpad.dev/docs/install` and `getpad.dev/docs/connect-local-project` — pages TASK-863 will publish but don't exist yet. Until they do, point at the GitHub README's #installation and #getting-started anchors so clicks at least land somewhere useful instead of 404. The TASK-863 follow-up will swap these back to the dedicated docs URLs once the pages ship. * fix(web/connect-modal): use real install commands from README (Codex round 2) Round 2 caught that Linux/Windows/Docker commands were fabricated: - Linux/Windows pointed at a getpad.dev/install.sh that doesn't exist - Docker used wrong volume mount (/root/.pad vs the image's /data) and didn't publish ports All four tabs now mirror the README's Installation section exactly: - macOS + Linux: brew install PerpetualSoftware/tap/pad - Windows: pointer to the GitHub releases page (no first-party one-liner) - Docker: docker run -p 127.0.0.1:7777:7777 -v pad-data:/data ghcr.io/perpetualsoftware/pad	2026-04-29 09:20:29 -04:00
xarmian	a03c96f9b0	feat(cli): pad init --url X --workspace <slug> as web-first cold-start (TASK-860) (#282) ... Make `pad init --url <server> --workspace <slug>` a reliable non-interactive cold-start so the web UI can hand users a single copy-paste command to connect a workspace they created on the web to their local project. Keystone CLI work for the web-first onboarding on-ramp under PLAN-859 (driven by IDEA-750). Changes: - `ensureWorkspace` gains a `wsSlug` parameter. When set, it ONLY attaches by slug — looks up the workspace via GetWorkspace, links the CWD if found, and surfaces a clear "not found on <server>" error otherwise. Critically, it never silently falls through to creating a new workspace named after the slug. - Refuses to clobber a CWD that's already linked to a different workspace; idempotent re-run when the existing link matches. - `pad init --url X` on a fresh machine (no config.toml on disk) now persists the config so subsequent commands don't need --url. - When both a positional name and --workspace are supplied, the slug wins and we print a Note: line so the override is visible. - Same wiring applied to `pad workspace init` for consistency. Tests: 5 new unit tests in cmd/pad/init_test.go cover slug-attach, not-found error, clobber refusal, idempotent re-run, and that the legacy name-driven path still works. Smoke-tested end-to-end against the local server: happy path links, missing slug errors cleanly with no `.pad.toml` written, clobber blocked, idempotent re-run silent.	2026-04-29 08:58:58 -04:00
xarmian	86a2f3c55b	fix(web/editor): copy from table puts plain text only on clipboard (TASK-858) (#281) ... * fix(web/editor): copy from table puts plain text only on clipboard (TASK-858) ProseMirror's default copy serialization for selections inside a table included the wrapping <table>...</table> in the text/html clipboard payload. Pasting into rich-text apps (or anywhere that prefers HTML over plain text) reproduced the table styling when the user just wanted the cell text. Add a tableCopyPlugin mirroring the existing codeBlockCopyPlugin pattern: when the selection lives entirely inside a table, write a plain-text representation to text/plain and clear text/html. Cut also deletes the range, same as the code-block plugin. Behavior: - Text selection inside a single cell: cell text on text/plain. - CellSelection (multi-cell drag): tab between cells, newline between rows. Pastes correctly into Excel/Sheets/Numbers. - Selection that spans into/out of the table: falls through to default. Trade-off (accepted): re-pasting a multi-cell copy into our own editor yields TSV text, not a reconstructed table. Matches Linear/Notion/Slack. Fixes BUG-855. * fix(web/editor): preserve parent Table plugins + selection-aware cut per Codex review (round 1) Two findings from Codex review of PR #281: 1. Table.extend's addProseMirrorPlugins was returning only [tableCopyPlugin], replacing the parent extension's plugins and silently dropping columnResizing (negating resizable: true) and tableEditing (cell selection / table editing). Now spreads ...(this.parent?.() ?? []) and appends tableCopyPlugin. 2. Cut path used tr.delete(from, to) which is unsafe for CellSelection — a contiguous document range can include unrelated cells (or row structure) between the rectangular cell-selection's endpoints. Switched to tr.deleteSelection(), which routes through prosemirror-tables' CellSelection.replace override and clears each selected cell's content. Still correct for the TextSelection-inside-one-cell case (deletes the text range as before). The codeBlockCopyPlugin's tr.delete(from, to) is intentionally left alone — that path validates the selection sits inside a single code_block, where from/to is a flat text range and no structural risk exists.	2026-04-29 00:57:14 -04:00
xarmian	cc4f1c16b6	feat(web): let users switch collection inside the Quick Add modal (TASK-857) (#280) ... The Quick Add modal previously locked users into the collection they launched it from. Replace the static `{icon} New {Singular}` header with a clickable pill that opens a small popover listing every regular collection in the workspace; selecting one swaps the target collection without losing the typed title. Behavior preserved: - Default collection still comes from the launch entry point (sidebar `+`, dashboard buttons, Cmd-N). - Picker excludes agent collections (conventions, playbooks) via the existing `regularCollections` filter. - If only one regular collection exists, the pill renders as a non- interactive label (no caret, no popover). - `submitQuickAdd` already re-derives default fields and content template from the current `quickAddCollection`, so swapping mid-flow Just Works. Keyboard: - Enter / Space / ArrowDown on the pill opens the picker. - ArrowUp/Down/Home/End navigate; Enter selects; Esc closes the picker only (textarea Esc still closes the modal). The outside-click handler is kept as its own `$effect` per CONVE-606 (don't combine reactive triggers in a single effect). Implements IDEA-749.	2026-04-29 00:06:49 -04:00
xarmian	eaae76f667	feat(auth): link to /console from CLI auth success state (TASK-856) (#279) ... After approving a CLI session at /auth/cli/{code}, the success state previously dead-ended with "you can close this tab" and no link out. Adds a primary "Go to your workspaces" CTA linking to /console — the same destination that / redirects to and that pad-cloud's OAuth flow lands users on post-login. Universal across self-hosted, Docker, Remote, and Pad Cloud (which proxies /auth/cli/ to the upstream pad backend via nginx, no pad-cloud change needed). The existing "you can close this tab" message stays — some users (CI runs, headless approvals, teammate's laptop) genuinely just want to close the tab. Source: IDEA-848. Parent: PLAN-833.	2026-04-28 23:33:01 -04:00
xarmian	2b752ba194	feat(release): sign + notarize macOS binaries (IDEA-830) (#278) ... * feat(release): sign + notarize macOS binaries (IDEA-830) Adds Developer ID code-signing and Apple notarization to the release pipeline so users installing via `brew install perpetualsoftware/tap/pad` or downloading binaries directly no longer hit Gatekeeper's "cannot verify the developer" warning. Uses GoReleaser v2's built-in `notarize:` block (Anchore/Quill backend), which signs and notarizes in-process from the existing ubuntu-latest runner — no rcodesign install, no macOS runner needed. Both the .p12 cert and the .p8 App Store Connect notary key are stored as base64-encoded repo secrets; Quill decodes them in-process. The notarize block is gated on `MACOS_CERT_P12` being set, so snapshot builds, fork PRs, and any context where the cert isn't available skip cleanly without failing. Verification plan: tag v0.0.1-rc.3, confirm Action goes green end-to-end, then on a clean Mac run `brew install perpetualsoftware/tap/pad` and verify `codesign -dv $(which pad)` shows the Developer ID signature and `spctl -a -t exec -vv $(which pad)` reports "accepted" with the notary ticket stapled. * harden(release): isolate npm build from macOS secrets + pin goreleaser Addresses Codex review findings on PR #278: MEDIUM — Apple signing secrets were exposed to the npm web build. The previous `before.hooks` block ran `npm ci && npm run build` inside the GoReleaser process, which means npm lifecycle scripts and the SvelteKit build inherited the Developer ID .p12 cert, cert password, and .p8 notary key from the goreleaser-action's env. Adding a 5-year signing cert to that environment meaningfully widened the blast radius of any npm supply-chain compromise. Fix: move the web build into a dedicated workflow step before the goreleaser-action invocation. The MACOS_* secrets are scoped only to the goreleaser env block, so the npm phase no longer sees them. LOW — GoReleaser binary version was floated as `~> v2`, while every third-party Action in this workflow is SHA-pinned per the policy at the top of the file. With Apple signing credentials now in the env, a compromised or regressed GoReleaser release would carry meaningful blast radius. Pinned to v2.15.4 (current latest) so bumps go through explicit review like the rest of the action pins. No functional change to the signing/notarization itself — same schema, same gating, same secrets. * docs(release): document web/build prereq for local snapshot builds Addresses second-pass Codex review finding on PR #278. Removing the goreleaser `before.hooks` web build (done in 6f4a52c to isolate npm from MACOS_* secrets) means `goreleaser release --snapshot` from a clean checkout now fails with "embed: no matching files found" because embed.go declares `//go:embed all:web/build` and there's no longer a hook that auto-populates that directory. CI is unaffected (the workflow has a dedicated `Build web UI` step), but contributors testing goreleaser config locally would hit a confusing error before reaching the part they wanted to validate. Doc-only fix: extend the existing comment block at the top of .goreleaser.yaml to point contributors at `make web` (or the explicit npm commands) as the prerequisite for local snapshot builds. Note that `make build` and `make install` already handle this automatically. * harden(release): add concurrency guard + bump goreleaser timeout Addresses third-pass Codex review findings on PR #278. Both findings were LOW (no ship blockers) and pre-existing concerns that the macOS notarize block makes slightly more visible. Folded into this PR rather than deferring because both relate directly to the goreleaser invocation we already touched. LOW #1 — GoReleaser overall timeout was the default 1h, while the notarize block now allows up to 20m of Apple notary wait time on top of build + cosign blob-sign + SBOM + multi-arch docker. On a slow notary day (or first-cert-use latency), that could come close to or hit the default ceiling. Bumped `release --clean` to `release --clean --timeout=2h` for comfortable headroom without burning Action minutes on the happy path (worker exits as soon as Apple replies). LOW #2 — No workflow-level concurrency guard. If two `v*` tags landed close together (rc.3 then rc.4 within a minute), runs would race on shared mutable outputs: GHCR `:latest`, the homebrew cask in the separate tap repo, the GitHub Releases page. Added a top-level concurrency block that serializes all release runs. Group is intentionally NOT keyed by `github.ref` — different tag names share the same mutable infrastructure, so we want all release tags to serialize, not just repeat pushes of the same tag. cancel-in-progress is false so a queued tag never aborts a release mid-publish, which could leave GHCR and the brew tap in inconsistent states. No functional change to signing/notarization itself. v0.0.1-rc.3	2026-04-28 20:24:38 -04:00