750 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
xarmian	08f76f3486	fix(deps): bump go-jose/v3 to v3.0.5 to clear GO-2026-4945 (BUG-1619) (#634) ... * fix(deps): bump go-jose/v3 to v3.0.5 to clear GO-2026-4945 (BUG-1619) GO-2026-4945 — Go JOSE panics in JWE decryption. Reachable via github.com/ory/fosite v0.49.0 from internal/server/handlers_oauth.go's handleOAuthAuthorize (govulncheck call chain into jose.ParseSigned / JSONWebSignature.Verify / etc). Drop-in dependency bump, no API changes. Post-bump govulncheck reports 0 reachable vulnerabilities. Discovered during v0.6.0 release pre-flight (PLAYB-1160 step 1) — CI run 26518095093 failed govulncheck after the most recent main push, even though nothing about that change touched OAuth. Same shape as TASK-1583 (the golang.org/x/net bump earlier this cycle). * test(open-children-guard): filter readParentRef by SourceRef to fix race flake (BUG-1621) readParentRef was matching the first LinkType=="parent" link in GET /items/{ref}/links's response — but GetItemLinks returns links in BOTH directions (WHERE source_id = ? OR target_id = ?). After TestOpenChildrenGuard_PatchAtomicRejectionPreservesParentLink attaches a child task to its target plan, the response contains two parent links: target→oldParent AND child→target. ORDER BY created_at DESC ties at sub-microsecond resolution; SQLite tiebreaks on rowid; under race-scheduler CI load the child→target link sometimes lands first, the helper returns the target's OWN ref as the "parent", and the setup-sanity assertion fails with "parent should start as PLAN-1, got PLAN-3" (where PLAN-3 = target itself). Filter on SourceRef == itemRef so only the item's OUTGOING parent link matches. Test-file-only change. Latent since PR #571 (IDEA-1494, e59d390). Caught now during v0.6.0 release pre-flight on PR #634. v0.6.0	2026-05-27 13:21:57 -04:00
xarmian	83716a65fc	fix(backlinks): scope cross-workspace admin enumeration to membership for bearer auth (BUG-1617) (#633) ... Companion to BUG-1616. The admin platform role granted unrestricted cross-workspace visibility at the STORE layer too: `GetCrossWorkspaceBacklinks` ran `ListWorkspaces()` for any user with `Role=admin`, and `ResolveBacklinksVisibility` short-circuited to `(nil, nil)` for the same role check. Both fired BEFORE the BUG-1616 middleware gate could deny the request, so a bearer-borne admin (CLI / PAT / MCP) could enumerate cross-workspace backlinks from every workspace on the server. Policy: bearer-borne admin gets STRICT membership enumeration — no guest-grants fallback. Matches RequireWorkspaceAccess's membership-only stance from BUG-1616. Cookie-session admin keeps the global view (preserved web-UI affordance). Threads `authIsBearer bool` from the HTTP boundary (via the new isBearerAuth helper) into the store layer: - `Store.ResolveBacklinksVisibility` — admin bypass now gated on `!authIsBearer`; bearer-admin falls through to the regular member/grants pipeline. Also tightens the "no visibility" return shape from `(nil, nil)` to non-nil empty slices so callers can distinguish "unrestricted" from "explicit empty" — closes a latent ambiguity that doesn't fire in current callers but would if any future caller bypassed the upstream membership filter. - `Store.GetCrossWorkspaceBacklinks` — new switch: - cookie admin → ListWorkspaces (unchanged) - bearer admin → GetUserMemberWorkspaces (NEW, strict membership; no grants fallback) - non-admin → GetUserWorkspaces (unchanged; memberships ∪ guest-grant workspaces) - `Server.guestResourceFilterCore` — admin short-circuit now gated on `!isBearerAuth(r)`; bearer-admin delegates to the store-side helper with the bearer signal threaded through. - `handlers_backlinks.go` — pass `isBearerAuth(r)` to `GetCrossWorkspaceBacklinks`. New `Store.GetUserMemberWorkspaces` helper — the first half of `GetUserWorkspaces` without the UNION-with-grants block. Used by the bearer-admin path; existing callers continue to use `GetUserWorkspaces` unchanged. Tests: - `wiki_links_xws_test.go`: - Updated `TestWikiLinks_CrossWorkspaceAdminSeesAllWorkspaces` to cover both `authIsBearer=false` (cookie, sees all) and `authIsBearer=true` (bearer, sees none) cases. - NEW `TestWikiLinks_CrossWorkspaceBearerAdminGrantOnlyWorkspaceFiltered` — bearer-admin with a guest grant on workspace C still gets ZERO cross-ws rows from C (Codex round-2 finding). - NEW `TestWikiLinks_CrossWorkspaceBearerAdminSeesMemberWorkspaces` — positive control: bearer-admin who IS a member sees the row. - Extended `TestResolveBacklinksVisibility_RoleMatrix` with two bearer-admin subtests (non-member workspace → empty; member workspace → unrestricted). - NEW `handlers_backlinks_admin_bearer_test.go::TestCrossWorkspaceBacklinks_AdminBearer_OnlySeesMembershipWorkspaces` — full HTTP integration test, both cookie and PAT-bearer subtests. - All existing callers updated to pass `false` for `authIsBearer` (preserves current cookie-session / non-admin behavior). Verification: full `go test ./...` green; `make lint` clean; Codex round 2 review CLEAN. 🤖 BUG-1617	2026-05-27 10:37:30 -04:00
xarmian	f48c99e421	fix(auth): scope admin platform role to cookie session auth (BUG-1616) (#632) ... The admin platform role granted owner-level access to every workspace on every surface, including bearer-borne callers (PATs on /api/v1, CLI session bearers, PATs/OAuth on /mcp). A user with "All current workspaces" consent on an MCP client — or a leaked admin token of any kind — could reach data the admin never joined. Policy: the admin global bypass now fires only for cookie session auth (web UI / SPA / /console/admin). Bearer-borne callers fall back to a strict workspace_members check (membership-only; no guest-grants fallback either). Gated four sites in lockstep: - RequireWorkspaceAccess (internal/server/middleware_auth.go) — covers /api/v1/* routes; emits the existing not_a_member MCP authz denial metric on bearer-admin denials. - handleSSE entry (internal/server/handlers_events.go) — adds an explicit GetWorkspaceMember check for bearer-borne admin after resolveWorkspace's global slug lookup. - sseSubscriberStillHasAccess (internal/server/handlers_events.go) — per-tick revalidation now matches entry-time policy. - computeSSEVisibility (internal/server/handlers_events.go) — bearer-admin gets a real VisibleCollectionIDs filter instead of "no filtering"; a bearer-admin who's a member with collection_access=specific is now correctly scoped. - authorizeCollabAccess (internal/server/handlers_collab.go) — WebSocket collab upgrade gate; same membership-only stance. New shared helper isBearerAuth(r) folds two signals (Authorization: Bearer header OR ctxIsAPIToken stash) so MCP-dispatcher synthesized requests and CLI session bearers are both covered. Mirrors the dual check middleware_csrf.go already uses. Tests (9 total): - middleware_auth_admin_token_gate_test.go (5) — PAT denied/allowed permutations, CLI session-bearer denial, cookie-session bypass preserved. - handlers_admin_bearer_gate_test.go (4) — SSE revalidation, visibility filter, collab WebSocket auth. Companion BUG-1617 (store-layer admin bypass in backlinks visibility) tracked separately.	2026-05-27 10:09:30 -04:00
xarmian	225fb4a53f	Wire upgrade CTAs with Stripe-ready billing flow (TASK-800) (#629) ... * feat(billing): add billing_available session flag gated on PAD_BILLING_AVAILABLE (TASK-800) Add Server.billingAvailable field set by SetBillingAvailable(), called from cmd/pad/main.go when PAD_BILLING_AVAILABLE=true|1. Expose the flag as billing_available in both the setup-state and authenticated session payloads (value: cloudMode && billingAvailable) so the web UI can gate Stripe CTAs without a code change at deploy time. False by default. * feat(billing): wire upgrade CTAs, checkout POST flow, plan section, clickable limit toasts (TASK-800) Frontend prep work gated on authStore.billingAvailable (from billing_available session field). When false, upgrade buttons remain hidden and the "coming soon" note stays in place — flip PAD_BILLING_AVAILABLE=true at deploy time. Changes: - client.ts: add billing_available to AuthSession; add api.billing.createCheckoutSession() (POST /billing/checkout → parse {url} → caller does window.location.href) - auth.svelte.ts: billingAvailable getter - console/billing: replace STRIPE_AVAILABLE=false with $derived(authStore.billingAvailable); fix GET→POST on upgrade buttons; add ?checkout=cancelled banner; add cancelled style - console/settings: new cloud-mode-gated "Plan" section with current plan + upgrade/manage link - All 11 limit-hit sites: replace plain-text '/console/billing' appendage with toastStore.show(msg, 'error', 6000, '/console/billing') so the toast is clickable * docs(billing): document pad-cloud CSRF and error-envelope contract divergences in createCheckoutSession (TASK-800)	2026-05-25 13:18:43 -04:00
xarmian	342679a364	Standardize plan-limit error envelope across HTTP/MCP/CLI/UI (TASK-788) (#628) ... * fix: limit-hit responses were actively broken — garbled toasts, no upgrade signal The limit enforcement responses (plan_limit_exceeded on 403) used a flat body shape {"error": "plan_limit_exceeded", ...} that is incompatible with every consumer: the frontend PadApiError parser, the CLI parseError path, and the MCP classifyHTTPStatusKind all expect {"error": {"code": ..., "message": ...}}. As a result, hitting any of the 5 plan limits (items, members, workspaces, api_tokens, webhooks) produced garbled toasts with undefined message text and zero upgrade signal. Fix: - writePlanLimitError now emits the standard nested error envelope with a human-readable message sentence and limit details in error.details. - CLI parseError now correctly surfacing the message (net positive, no code change needed). - MCP classifyHTTPStatusKind: adds ErrPlanLimitExceeded to the taxonomy and the allowedStructuredErrorCodes whitelist so 403 plan-limit errors pass through with code + details rather than collapsing to ErrPermissionDenied (TASK-788). - Frontend: exports isPlanLimitError() type-guard and planLimitMessage() formatter from client.ts; all 4 limit-hit write call sites (item create, member invite, workspace create, token create) now branch on the code and show an upgrade-signal message pointing at /console/billing. - Test: updates handlers_workspace_cap_test.go to the new body shape; adds TestPlanLimitError_ResponseShape covering members_per_workspace limit hit. TASK-788 * fix(R1): cover MCP stdio path, 5 more item-create sites, polish message wording Finding A — MCP stdio transport was missing plan-limit coverage: - cli/client.go: add PlanLimitDetails struct, AsPlanLimit() helper, and WritePlanLimitError() that emits the pad-structured-error/v1 marker so the MCP stdio classifier can lift code + details instead of falling through to ErrServerError. - cmd/pad/main.go: wire the WritePlanLimitError branch into all three CreateItem call sites (item create, convention activate, playbook activate). - internal/mcp: add TestClassifyHTTPStatus_PlanLimitPreservesCodeAndDetails, TestClassifyHTTPStatus_Generic403FallsToPermissionDenied, TestClassifyExecError_PlanLimitMarkerLiftsStructuredPayload, and TestClassifyExecError_PlanLimitWithoutMarkerFallsThrough. Note: extractUpstreamErrorEnvelope already parses details (json.RawMessage field) — the codex concern about it being silently empty was a false alarm; no fix needed there. Finding B — 5 more item-create entry points were unguarded: - EditorBubbleMenu.svelte (inline wiki-link capture) - Sidebar.svelte (quick-add) - roles/+page.svelte (board new-item, was console.error only; adds toastStore) - conventions/+page.svelte - playbooks/+page.svelte (both create and duplicate paths) B1/B2 polish — server message is now statement-of-fact only, no doubled upgrade verb. planLimitMessage() drops "Upgrade to Pro to add more." (each surface appends its own CTA). limitStr uses hyphenated adjective form "3-member" / "10-item" (compound modifier before "limit"). TASK-788 * feat(task-788): extend MCP-stdio plan-limit coverage to workspace, invite, webhook Wire WritePlanLimitError into three additional CLI command error paths so the MCP stdio classifier surfaces ErrPlanLimitExceeded with details instead of falling through to ErrServerError: - workspaceCreateCmd: check before fmt.Errorf wraps the APIError - inviteCmd: check before returning the raw error - webhooksCreateCmd: check before returning the raw error Add TestClassifyExecError_PlanLimitWorkspaceCreate to exercise the full workspace-create stdio round-trip through classifyExecError, asserting ErrPlanLimitExceeded code, feature="workspaces", limit, and upgrade_url. Token create intentionally left bare (agents don't drive token creation).	2026-05-25 11:46:41 -04:00
xarmian	a7fb14ee14	Reduce free-tier workspace cap from 5 to 3 (TASK-1609) (#627) ... Lower DefaultFreeLimits.Workspaces from 5 to 3 to sharpen the price-discrimination delta between Free and Pro ahead of Stripe Live (PLAN-1570 Phase a). Add IDEA-1611 comment on the soft-deleted workspace count behavior. Add store-level and handler-level tests covering the new boundary, pro-tier bypass, override path, and self-hosted no-limit path.	2026-05-25 09:24:57 -04:00
xarmian	3a04c06684	fix(backlinks): suppression must query item_links, not items.parent_id (TASK-1607 followup) (#626) ... * fix(backlinks): suppression must query item_links, not items.parent_id (TASK-1607 followup) The initial TASK-1607 fix routed the parent↔child "Mentioned in" suppression through `items.parent_id`. That column is empty in production — 0 of 3,923 items in a live workspace had it set — because the API path (handlers_items.go::handleCreateItem) writes parent relationships via Store.SetParentLink, which only touches the `item_links` table with link_type='parent' (source=child, target=parent — see migration 023). The deprecated parent_id column is vestigial. Result: the filter was a no-op everywhere the user actually encountered the duplication. Caught when testing on fhir-core/TASK-397 — a task with ~20 children all wiki-linking it, none filtered. Direct sqlite inspection confirmed: parent_id NULL on every row, but 2,022 'parent' rows in item_links workspace-wide. The fix: - Replace the SQL predicate in GetBacklinks and CountBacklinks with two NOT EXISTS subqueries against item_links — one for "source is not a child of target", one for "source is not the target's parent". Both use idx_links_source / idx_links_target for O(1) lookups per candidate row. - Drop the TargetParentID field from BacklinksVisibility. The store no longer needs the handler to plumb it through — the parent relationship is queried from item_links directly. Same pattern as children-suppression: both are unconditional and self-contained. - Update handlers_backlinks.go to remove the dead plumbing. - Rewrite createChildItem test helper to use SetParentLink (the production path) instead of CreateItem with ParentID (the dead column path). The original test helper made all the suppression tests pass falsely against the wrong storage. - Add TestWikiLinks_SuppressionUsesItemLinksNotParentIDColumn: sets up the parent ONLY via SetParentLink, asserts parent_id column stays NULL, asserts suppression still works. Regression guard against routing the filter back through the dead column. The existing 4 suppression tests still pass against the corrected mechanism (they now exercise SetParentLink under the hood). Cross-workspace path remains untouched — item_links is workspace- scoped same as parent_id was, so the doc comment on GetCrossWorkspaceBacklinks stays valid. Refs IDEA-1601. * fix(backlinks): consult items.parent_id alongside item_links for suppression (Codex P2) Codex review of the prior commit raised a P2: ItemCreate.ParentID and ItemUpdate.ParentID still write to items.parent_id directly without creating an item_links row (handlers_items.go only calls SetParentLink for the `parent` field path, not for direct ParentID JSON). The HTTP/CLI path is unaffected — empirically zero items in production have parent_id set — but a direct store-API caller (test, future import path, or any code that bypasses the handler) could leave the suppression invisible to one of the two storage shapes. Belt-and-suspenders: the relClause now AND's two pairs of predicates so suppression triggers whichever shape the parent relationship lives in. - Children-suppression: NOT EXISTS item_links AND items.parent_id comparison - Parent-suppression: NOT EXISTS item_links AND a correlated subquery against items (NOT EXISTS items t WHERE t.id = ? AND t.parent_id = s.id) so we don't have to plumb target's parent_id through New regression test (TestWikiLinks_SuppressionFallsBackToItemsParentIDColumn) exercises the column-only path: CreateItem with ParentID, NO SetParentLink, asserts items.parent_id IS set and item_links is empty, then asserts suppression still works in both directions. CountBacklinks updated identically to stay in lockstep with GetBacklinks pagination math. * fix(backlinks): suppression must cover childLinkTypes ('parent' AND 'implements') (Codex round 2) Codex round 2 caught: the Child Items panel and GetChildItems both inflate `childLinkTypes = {"parent", "implements"}` (see items.go:18). My filter only suppressed link_type='parent'. An 'implements' child therefore still appeared in Mentioned in even though it was visually duplicated in the Children section above. Use childLinkTypeSQL() in both NOT EXISTS subqueries (children direction and parent direction) so the filter stays in lockstep with the canonical inclusion rule. If a future link type is added to childLinkTypes, the suppression picks it up automatically. New regression test TestWikiLinks_SuppressionCoversImplementsChildLinkType wires a child via CreateItemLink(link_type=implements) and asserts the mention is suppressed — pins the lockstep with childLinkTypes. This was the actual mechanism for the original fhir-core/TASK-397 report: TASK-441 et al. carry "**Implements:** [[TASK-397]]" in their bodies AND are linked as 'parent' (Pad uses 'parent' for the explicit hierarchy in that workspace), but a sibling case where only 'implements' was set would have leaked through round 1's fix.	2026-05-24 21:27:38 -04:00
xarmian	4aff8c70a0	feat(backlinks): suppress parent↔child mentions from "Mentioned in" panel (TASK-1607) (#625) ... On a parent's page (typically a PLAN with many child TASKs), the "Mentioned in" panel was dominated by child tasks that wiki-link back to their parent. Those children are already listed in the Children section directly above the panel, so the same items appeared twice on screen and buried genuine cross-references (sibling plans, retro docs, etc.). Symmetric problem on a child's page: the parent shows up in "Mentioned in" even though it's already in the "Parent: …" header. Suppression is server-side in two parts, modeled on the existing self-link filter in GetBacklinks (s.id != targetItemID): 1. Children-suppression — always-on. Source rows where s.parent_id = targetItemID are dropped. Uses the existing targetItemID parameter; no API change. NULL-safe form so orphan items still surface (raw `s.parent_id != ?` would silently drop NULL parent_id rows under SQL three-valued logic). 2. Parent-suppression — opt-in. New TargetParentID *string field on BacklinksVisibility (zero value = nil = no parent suppression, keeping the ~50 existing test callsites valid). When set, the source row whose id == TargetParentID is dropped. The handler passes item.ParentID from the resolved target. Both apply to CountBacklinks identically so the handlers_backlinks.go same-ws/cross-ws pagination math (which depends on count-vs-fetch agreeing) stays correct. GetCrossWorkspaceBacklinks is unaffected — parent_id is workspace-scoped, so a cross-ws source can't be the target's parent or child by construction. Doc comment added noting this. Tests: - ChildMentionOfParentSuppressed: headline case - ParentMentionOnChildPageSuppressed: symmetric case with/without TargetParentID - SiblingMentionsNotSuppressed: control — siblings of a shared parent that wiki-link each other still surface - OrphanBacklinksUnaffected: regression for the NULL-safe form - PaginationStableAfterSuppression: page 1 + page 2 hit all filtered rows exactly once with no duplicates Promoted from IDEA-1601. Follow-up to [[PLAN-1593]] (which shipped the original wiki-link backlinks reverse index).	2026-05-24 20:52:11 -04:00
xarmian	c1c9f2ef97	fix(share): guard nil grants/share-link slices to prevent ShareDialog TypeError (BUG-1598) (#624) ... The Share dialog threw "Cannot read properties of null (reading 'length')" on any item/collection with zero grants. Store.ListCollectionGrants / ListItemGrants return a nil slice when there are no rows, which writeJSON encoded as JSON null; the client then hit grants.length on null. Server: nil-to-[]{} guard in handleListCollectionGrants and handleListItemGrants, mirroring the existing pattern in handlers_share_links.go. Client: defensive ?? [] in ShareDialog.svelte's loadGrants and loadShareLinks so a null body can't reach .length checks.	2026-05-24 16:12:38 -04:00
xarmian	35ac7552eb	feat(backlinks): UI panel + mention badge + CLI + MCP (Phase 3) (#623) ... * feat(backlinks): UI panel + mention badge + CLI + MCP (Phase 3) Phase 3 of PLAN-1593 (TASK-1596). Surfaces the backlinks index shipped in Phases 1/2a/2b across every place users live: web UI, CLI, MCP. What changed Web UI - New BacklinksPanel.svelte at web/src/lib/components/. Fetches via api.items.backlinks (new method) and renders inbound `[[...]]` references grouped by source collection. Per-row: collection icon, ref + title, snippet, relative timestamp, optional `(displayed as)` override, faint workspace badge on cross-ws rows. Pagination via "Show older" when the page is full. Collapses entirely when the count is zero — no header, no whitespace, no empty surface for items with no inbound links. - Mention badge ("📎 N") in the item-page action bar next to the Timeline button. Hidden when N=0; smooth-scrolls to the panel. Wired via onCountChange callback so badge + panel stay in sync. - New Backlink TypeScript type at web/src/lib/types/index.ts mirroring internal/models/backlink.go; new api.items.backlinks(ws, slug, opts) client method. CLI - `pad item show <ref>` enriched with inline top-5 "Mentioned in" section in TTY mode (skipped when empty), and a backlinks_top array in JSON output. Hint at the dedicated `pad item backlinks` command when the inline list hits the 5-row cap. MCP - New `pad_item.action: backlinks` — passes through to `pad item backlinks <ref>` with optional `limit` (default 50, max 300) + `offset` params. Bumps ToolSurfaceVersion 0.5 → 0.6 with a backwards-compatible additive note in version.go. Updated the catalog_readonly_test fixtures so the cmdhelp drift check passes. Test plan - [x] go build ./... + go test ./internal/mcp/ green - [x] make check (lint + Go + web) green - [x] make install + restart - [x] pad item show TASK-1596 shows --- Mentioned in --- inline - [x] pad item show TASK-1596 --format json includes backlinks_top - [x] svelte-check 0 errors - [ ] /codex review --loop → CLEAN Out of scope (filed as separate ideas if anyone asks) - Force-directed graph visualization of the link network - Broken-links report (target_item_id IS NULL feeds it but it's its own feature) PLAN-1593 / TASK-1596. * fix(backlinks): unique each-block key for multi-occurrence rows (Codex round 1) Codex round 1 P1: BacklinksPanel keyed each row by source_item_id, but the server preserves multiplicity — a source body that mentions the target three times produces three Backlink rows (Phase 1 design decision, covered by TestWikiLinks_RepeatedRefStoresMultipleRows). Duplicate keys in Svelte's #each are rejected at dev time and silently reuse DOM in prod, so the panel would render only one of the N rows from a multi-mention source. Fix: compose a unique key per row via new rowKey(bl, index) helper: `${source_item_id}|${snippet}|${index}`. The snippet usually differs across positions (centered on the bracket byte offset); the index suffix is the unconditional tie-breaker. PLAN-1593 / TASK-1596.	2026-05-24 15:02:06 -04:00
xarmian	905876af04	feat(backlinks): cross-workspace wiki-links + request-independent ACL (Phase 2b) (#622) ... * feat(backlinks): cross-workspace wiki-links + request-independent ACL (Phase 2b) Phase 2b of PLAN-1593 (TASK-1597). Completes the wiki-link reverse index by indexing and surfacing `[[workspace::REF]]` cross-workspace references. Builds on Phase 2a's title work (PR #621). Phase 3 (TASK-1596) owns the UI/MCP/CLI rendering changes. What changed - internal/store/backlinks_visibility.go (new): request-independent ACL helper `Store.ResolveBacklinksVisibility(userID, workspaceID, includeDeletedItems)`. Mirrors the role-determination + collection- merge logic from server.guestResourceFilterCore but doesn't depend on a request context, so cross-ws traversal can compute per-source- workspace ACLs without a `workspaceRole(r)` lookup. The Codex planning-round review caught the prior plan reusing the request- scoped helper as a hidden architectural cost; this is the resolution. - internal/server/server.go: guestResourceFilterCore refactored to delegate to the new store helper. Keeps the request-scoped wrapper signature stable for all existing handler call sites; only the internals move. - internal/links/extract.go: lift the Phase-2a workspace_ref emit gate. WikiLinkKindWorkspaceRef now flows through ExtractWikiLinks alongside ref and title kinds. parseBody recognition was already in place from earlier rounds. - internal/store/wiki_links.go: WikiLinkKindWorkspaceRef branch in replaceWikiLinks stores (target_workspace_id, target_ref) verbatim, resolving the slug→ID via new resolveWorkspaceSlugTx (with per-call cache so repeated `[[ws::X]]` in one body don't re-query). Unknown slugs persist with target_workspace_id=NULL — broken-link semantics, identical to existing ref/title patterns. - internal/store/wiki_links.go: new `Store.GetCrossWorkspaceBacklinks` enumerates accessible workspaces via Store.GetUserWorkspaces (which includes guest-only access — broader than membership query), then per-workspace computes visibility via ResolveBacklinksVisibility and runs the SQL backlinks query with the per-ws (FullCollectionIDs, GrantedItemIDs) predicate inline. Results sorted by updated_at DESC in Go, paginated globally. Per-workspace safety cap (offset+limit) prevents one workspace from dominating the global slice. - internal/store/wiki_links.go: new `Store.CountBacklinks` for same-ws pagination boundary detection. Needed so the handler knows where the cross-ws tier begins for pages 2+. - internal/models/backlink.go: new `SourceWorkspaceSlug string` (omitempty) field. Populated only by cross-ws rows; same-ws rows leave it empty so the existing wire shape is preserved. - internal/server/handlers_backlinks.go: union pagination across same-ws and cross-ws tiers. Same-ws first (matches the renderer's UI mental model — your own workspace's links at the top of the panel). Count-based slice math handles pages 2+ correctly when same-ws is exhausted. Tests - internal/links/extract_test.go: workspace_ref forms emit correctly (bare, display alias, mixed case, invalid-slug fallback to title). - internal/store/wiki_links_xws_test.go (new): six cross-ws scenarios plus a role-matrix test: - end-to-end cross-ws index + query - non-member sees nothing - guest with collection grant sees only that collection - guest with item grant sees only the granted item - unknown workspace slug → broken row, no query results - same-ws rows leave SourceWorkspaceSlug empty - ResolveBacklinksVisibility role matrix (admin/full member/guest with grants/non-member non-grant) Out of scope (Phase 3 / TASK-1596) UI rendering of cross-ws backlinks (workspace badge + workspace- prefixed ref), MCP `pad_item.action: backlinks` cross-ws fields, CLI display tweaks. PLAN-1593 / TASK-1597. * fix(backlinks): admin enumeration + cross-prefix ref fallback + unbounded perWsCap (Codex round 1) Three P2 findings from Codex round 1 against PR #622: Finding 1 — admin users miss cross-ws backlinks. `GetUserWorkspaces` returns only memberships + grant-only guest workspaces, but RequireWorkspaceAccess (middleware_auth.go:481) gives admins implicit access to every workspace. An admin querying for backlinks would silently miss links from workspaces they're not explicitly a member of. Fix: in GetCrossWorkspaceBacklinks, branch on user.Role: - admin → s.ListWorkspaces() (every non-deleted workspace) - non-admin → s.GetUserWorkspaces (memberships + grants) Stale user IDs return empty result rather than erroring. Finding 2 — cross-ws ref matching doesn't handle cross-prefix moves. Same-ws is immune because target_item_id is resolved at parse time and survives renames/moves; cross-ws resolves at query time, so a `[[other-ws::OLD-42]]` row written before the target moved from OLD→NEW collection wouldn't match a query under the NEW ref. Fix: in queryCrossWorkspaceBacklinksForWorkspace, dual ref-match clause: exact `LOWER(wl.target_ref) = LOWER(?)` OR `LOWER(wl.target_ref) LIKE LOWER('%-N')` where N is the item_number from the target ref. Pad prefixes are alphanumeric with no internal `-`, so trailing `-N` uniquely identifies the number suffix — no false positives like "TASK-142" matching "%-42" (LIKE anchors to the trailing literal). Finding 3 — per-workspace cap of 1000 silently broke pagination beyond offset>=1000. The 1000 ceiling was defensive paranoia; the correct math is offset+limit per workspace (worst case all rows come from one workspace and the global slice still needs that many). Fix: drop the 1000 ceiling. perWsCap = offset+limit unconditionally. For runaway offsets the per-workspace transfer cost is proportional; documented as a known characteristic (callers shouldn't be paging past offset=10000 anyway). Regression tests: - TestWikiLinks_CrossWorkspaceAdminSeesAllWorkspaces: admin sees cross-ws backlink without being a workspace member. - TestWikiLinks_CrossWorkspaceRefNumberFallback: move target to new collection, query under new ref, old-ref-stored row still surfaces. PLAN-1593 / TASK-1597. * fix(backlinks): honor OAuth/MCP token workspace allow-list (Codex round 2) Codex round 2 P1: cross-workspace backlinks bypassed the OAuth/MCP token's workspace allow-list (TASK-952). A token consented for workspace A but with the underlying user having access to B would still surface source rows from B via the cross-ws query — leaking data outside the token's consent scope. Fix: thread `allowedWorkspaceSlugs []string` through GetCrossWorkspaceBacklinks. Handler populates it from TokenAllowedWorkspacesFromContext(r.Context()): - nil → no token gate (PAT or pre-TASK-952 token, allow all) - "*" wildcard → allow all - explicit list → strict slug membership Workspace enumeration skips any source workspace whose slug isn't in the allowlist. The same-ws path is unchanged because RequireWorkspaceAccess already gated the target workspace against the allow-list (so we only reach this handler when the target IS in the list). Regression test in wiki_links_xws_test.go covers four shapes: nil, wildcard, target-only (blocks cross-ws), explicit source-workspace (allows cross-ws). PLAN-1593 / TASK-1597. * fix(backlinks): normalize limit at handler boundary (Codex round 3) Codex round 3 P2: the backlinks handler parsed ?limit=N but didn't normalize it before computing the same-ws/cross-ws pagination split. GetBacklinks and GetCrossWorkspaceBacklinks each clamp >300 internally, but the handler's 'remaining := limit - len(sameWs)' used the original (potentially huge) value. With ?limit=301 and more than 50 same-ws backlinks, the first page would mix cross-ws in before same-ws was exhausted, violating the documented tier order. Fix: clamp 'limit' to <=300 at the handler boundary, before any pagination math runs. PLAN-1593 / TASK-1597. * fix(backlinks): normalize same-workspace [[ws::REF]] to ref-kind (Codex round 4) Codex round 4 P2: `[[<current-ws>::TASK-1]]` was being indexed as a workspace_ref row with target_workspace_id = current workspace. But the same-ws GetBacklinks query requires target_item_id (workspace_ref rows leave it NULL), AND GetCrossWorkspaceBacklinks explicitly skips the target workspace — so the link rendered and navigated correctly in the UI but no backlink ever surfaced. The renderer's L307 short-circuits same-workspace fully-qualified form to behave identically to `[[REF]]`; the index must follow. Fix: in replaceWikiLinks, normalize a workspace_ref link to ref-kind when its slug resolves to the current workspace. The promotion canonicalizes the ref (via new links.CanonicalizeRef exported alias) so `[[ws::task-5]]` stores the same canonical shape as `[[TASK-5]]`. Tests: - TestWikiLinks_CrossWorkspaceSameWorkspaceQualifiedNormalized: same-ws fully-qualified `[[ws::REF]]` surfaces in same-ws backlinks and is absent from cross-ws backlinks. PLAN-1593 / TASK-1597. * fix(backlinks): same-ws qualified ref miss doesn't title-fallback (Codex round 5) Codex round 5 P2: my round-4 normalization was too aggressive. It promoted `[[<current-ws>::REF]]` to ref-kind and let the regular ref branch handle it — including the title-fallback path that runs on ref miss. But the renderer's same-ws qualified branch (markdown.ts:472-481) does NOT title-fallback: a ref miss in that path returns the wiki-link verbatim (broken). Only the bare `[[REF]]` path (markdown.ts:513) falls through to title lookup. So my normalization could create ghost backlinks for source bodies like `[[ws::ISO-9001]]` when an item titled "ISO-9001" exists but no ISO collection — the renderer renders broken text, but the index would point at the title-matching item. Fix: handle same-ws qualified refs inline at the top of the loop, BEFORE the switch dispatches. Insert as ref-kind row (resolved or NULL) and `continue` past the switch. Bypasses the title-fallback path entirely, mirroring the renderer's behavior. Regression test in wiki_links_xws_test.go pairs same-ws qualified miss (must NOT title-fallback) with bare ref miss (SHOULD title-fallback) to lock the asymmetry in. PLAN-1593 / TASK-1597.	2026-05-24 13:27:40 -04:00
xarmian	c67c167c43	feat(backlinks): title-form wiki-links + rename cascade (Phase 2a) (#621) ... * feat(backlinks): title-form wiki-links + rename cascade (Phase 2a) Phase 2a of PLAN-1593 (TASK-1595). Extends the server-side wiki-link reverse index from Phase 1's `[[REF-N]]` coverage to also handle `[[Title]]` and `[[collection/Title]]`. Cross-workspace `[[ws::REF]]` forms stay gated until TASK-1597 (Phase 2b) ships the request- independent ACL helper. What changed - internal/links/extract.go: lift the Phase-1 emit gate for WikiLinkKindTitle; keep WikiLinkKindWorkspaceRef gated. - internal/store/wiki_links.go: title branch in replaceWikiLinks (verbatim target_title storage + case-insensitive resolution), resolveTitleTx with full-key match first / `/`-split fallback (mirrors renderer order — Codex review caught the inverse on the planning round), cascadeTitleRename + resolveBrokenTitleLinks. - internal/store/items.go: rename cascade fires in-tx on title change; create path flips pre-existing broken title rows. - internal/store/migrations/062 + pgmigrations/041: one-shot `DELETE FROM item_wiki_links` so the startup backfill repopulates with the Phase 2a vocabulary. - Tests cover title round-trip, case-insensitive resolution, broken- link persistence + later resolution, full rename cascade with content rewrite, collection-qualified form, full-key-beats-split precedence (regresses Codex finding #3), broken-row flip on rename, no-op rename, and self-reference filtering. PLAN-1593 / TASK-1595. * fix(backlinks): rename cascade preserves display aliases (Codex round 1) Codex round 1 against PR #621 caught: the title-rename cascade selects sources via target_item_id (correctly hitting all rows that resolve to the renamed item — including aliased and mixed-case shapes), but the literal `strings.ReplaceAll` rewrite step only matched `[[Old Title]]` / `[[<slug>/Old Title]]`. Rows from `[[Old Title|alias]]`, `[[old title]]` (mixed case), or `[[<slug>/Old Title|alias]]` slipped past the rewrite; the trailing replaceWikiLinks re-parse then saw the same body, failed to resolve under the new title, and converted the row to broken — exactly the regression the cascade exists to prevent. Fix: new internal/links/RewriteWikiTitle helper handles all four title-form shapes with case-insensitive title matching and verbatim display-alias preservation. Cascade swaps from ReplaceAll to this helper. Unit tests in links_test.go cover the matrix; integration test in wiki_links_test.go regresses the original failure mode by mixing all four shapes in one source and asserting all four stay resolved after rename. Known limitation documented in the helper: titles containing wiki- link escape characters (`]`, `|`, `\`) — stored escaped in source content — don't match the regex's literal old-title segment. Same limitation exists in the legacy ReplaceTitle helper; promotable if a real user hits it. PLAN-1593 / TASK-1595. * fix(backlinks): retarget qualified-fallback rows on literal-title arrival (Codex round 2) Codex round 2 against PR #621 (P2): resolveBrokenTitleLinks only flipped target_item_id IS NULL rows, missing the arrival-order case where a literal `[[tasks/Setup]]` should win stage 1 over a row previously resolved via stage 2 (qualified fallback to item "Setup" in collection "tasks"). The index would stay stale until the source's content was rewritten — a latent inconsistency between the persisted backlink and what the renderer would actually show. Fix: drop the IS NULL constraint on the stage-1 UPDATE. Stage 1 ALWAYS wins per the renderer's order at markdown.ts:541, so any row with a matching literal title flips to the new item — including rows currently pointing at a qualified-fallback target. Added a target_item_id != ? guard so we don't churn rows that already point at us. Stage 2 keeps the NULL constraint so already-resolved qualified rows don't churn when another fallback candidate appears. Regression test in wiki_links_test.go reproduces the exact scenario Codex described: fallback resolves first, literal arrival steals the row from the fallback target. PLAN-1593 / TASK-1595. * fix(backlinks): renderer parity for [[A|B]] + scope arrival retarget (Codex round 3) Two findings from Codex round 3 against PR #621: P1 — parser/renderer parity for [[A|B]] matching literal title "A|B". The renderer (web/src/lib/utils/markdown.ts:516-525) tries the FULL body as a title FIRST when a pipe is present, only falling through to the split interpretation on miss. Our parseBody always split on the first unescaped pipe, so an item literally titled "A|B" was indexed as title="A" with display="B" — the index would miss backlinks the UI shows, or point them at a different "A" item. Fix: in replaceWikiLinks for title kind, when HasDisplay, try the full body (Title+"|"+Display) as a title FIRST via resolveTitleTx; on hit, store target_title=fullBody and drop the display override (the display segment was actually part of the title). On miss, fall back to the existing split-key interpretation. The cascade's RewriteWikiTitle regex correctly handles both storage shapes via QuoteMeta on the title. P2 — stage-1 UPDATE was too aggressive after the round-2 fix. The broad UPDATE (no IS NULL constraint) silently stole backlinks from legitimately-resolved rows when a SECOND item was created/renamed to the same title. Titles aren't unique, the renderer's Array.find() is order-dependent, and silent churn is worse than no-op stability. Fix: split resolveBrokenTitleLinks into three updates: (1) Plain literal flip — NULL only. (2) Qualified literal flip — NULL only. (3) Literal-arrival retarget — gated on title containing `/`. Only fires for `[[<slug>/Title]]` rows, which are the only ones that COULD have been stage-2 qualified-fallback resolved. Rows with target_title='Foo' (no slash) can only have been stage-1 literal — we don't steal those. Regression tests: - TestWikiLinks_LiteralPipeInTitleResolves — `[[A|B]]` resolves to item literally titled "A|B". - TestWikiLinks_LiteralPipeInTitleFallsThroughToSplit — `[[A|B]]` falls back to item "A" when no "A|B" item exists. - TestWikiLinks_SecondItemSameTitleDoesNotStealBacklinks — adding a second "Foo" item doesn't redirect the existing backlink. PLAN-1593 / TASK-1595. * fix(backlinks): broken pipe-in-body rows keyed on full body (Codex round 4) Codex round 4 against PR #621: when a source body `[[A|B]]` is written before any matching item exists, the broken row was stored with target_title="A" (the split key). If an item literally titled "A|B" was later created, resolveBrokenTitleLinks looking for target_title="A|B" couldn't find the row — index went stale while the renderer's preferred full-body interpretation would correctly resolve the link. Fix: when nothing resolves AND a pipe was present (HasDisplay), key the broken row on the FULL body (Title+"|"+Display) instead of the split key. resolveBrokenTitleLinks then naturally finds it via the literal-arrival path. The remaining asymmetry — a broken row keyed on full body won't pick up a future split-fallback resolution to a new item titled "A" — is documented in the code as a v3-promotable limitation. The full-body path is the renderer's PREFERRED interpretation (markdown.ts:516), so prioritizing it is the right tradeoff in the rare case both interpretations could apply. Regression test in wiki_links_test.go reproduces the scenario: source written first with `[[A|B]]`, item titled "A|B" created later, backlink should resolve. PLAN-1593 / TASK-1595. * fix(backlinks): scope stage-3 retarget + cascade self-refs (Codex round 5) Two findings from Codex round 5 against PR #621: Finding 1 — stage-3 literal-arrival retarget could still steal backlinks from a legitimate stage-1 literal-match row when a SECOND item with the same slash-containing title is created. The previous fix (round 3) gated stage-3 on title containing `/`, which let through the qualified-fallback retarget case correctly but didn't distinguish stage-1-resolved rows pointing at a literal twin from stage-2-resolved rows pointing at the fallback target. Fix: add an EXISTS check that scopes the flip to rows whose CURRENT target has a title NOT matching ours. Stage-1 (literal) resolutions point at items literally titled the same as the row's target_title; stage-2 (qualified-fallback) resolutions point at items titled just the trailing segment. The EXISTS clause picks out only the latter. Finding 2 — self-references on the renamed item went stale on title-only renames. The cascade's `s.id != renamedItemID` filter excluded self, but items.go only re-indexes content when input.Content != nil. So a title-only rename of an item whose body mentions itself by its old title kept the body's now-broken `[[Old Title]]` literal in place while the index still recorded a "working" backlink — drift between renderer state and index state. Fix: drop the self-exclusion from the cascade SELECT. RewriteWikiTitle rewrites the renamed item's own content along with everyone else's; GetBacklinks still hides self-links at query time, so the backlinks panel behavior is unchanged. Tests: - TestWikiLinks_DuplicateSlashTitleNoTheft regresses Finding 1. - TestWikiLinks_TitleRenameRewritesSelfReferences asserts Finding 2's new correct behavior (replaces the prior test that asserted the old buggy behavior). PLAN-1593 / TASK-1595. * fix(backlinks): ref→title fallback + reorder cascade self-ref (Codex round 6) Two findings from Codex round 6 against PR #621: Finding 1 — ref-shaped title fallback missing. parseBody returns WikiLinkKindRef for `[[ISO-9001]]` (matches refPattern), but if no ISO-9001 ref-item exists, the renderer falls through to legacy title lookup (markdown.ts:513) and resolves to an item literally titled "ISO-9001". The store inserted only a broken ref-kind row with target_title=NULL, so GetBacklinks never surfaced the backlink even when the renderer rendered it. Fix: in replaceWikiLinks' WikiLinkKindRef branch, when resolveRefTx misses, try resolveTitleTx on the same body. If title resolves, INSERT as title-kind row with target_title=ref-shaped-body. The rename cascade catches these correctly via target_kind='title' + target_item_id. The asymmetry — a future ref-item creation can't auto-retarget these title-stored rows — is documented as a v3 limitation. Finding 2 — combined title+content update broke self-ref cascade. The original order (main UPDATE → replaceWikiLinks(self) → cascade) wiped self's `target_item_id=renamedItemID` row before cascade ran: when input.Content contains `[[Old Title]]`, re-indexing self resolved it as broken (target_item_id=NULL), so cascade's SELECT missed self for the title+content path. Fix: reorder so cascade runs BEFORE the self re-index — the pre-existing wl rows are still intact at cascade time. The final re-index re-reads items.content from the DB (since cascade may have rewritten it in-band) rather than using *input.Content directly; otherwise the re-index would undo the cascade's self-ref rewrite. Tests: - TestWikiLinks_RefShapedFallsThroughToTitle — `[[ISO-9001]]` resolves to an item titled "ISO-9001". - TestWikiLinks_TitleAndContentRenameCascadesSelfRef — combined title+content rename with self-ref in new content gets the self-ref rewritten by cascade. PLAN-1593 / TASK-1595. * fix(backlinks): ref+pipe→title parity, position-based cascade, scoped self-rewrite (Codex round 7) Three intertwined fixes addressing Codex round 7 findings against PR #621: Finding 1 — ref→title fallback missed pipe-bodies. For `[[ISO-9001|Spec]]`, renderer tries full-body title "ISO-9001|Spec" BEFORE falling to bare "ISO-9001" (markdown.ts:516). Our ref-fallback only tried bare. Extended ref-branch's fallback to try full body first when HasDisplay, then bare — same order as the title-branch's stage (a)/(b) pattern. Finding 2 — cascade corrupted UNRELATED literal-pipe titles. Items A "Old Title" and B "Old Title|alias" both referenced from one source; renaming A previously triggered RewriteWikiTitle's regex `(?i:Old Title)((?:\|...)?)` which matched BOTH A's `[[Old Title]]` AND B's `[[Old Title|alias]]` — corrupting the B link. Refactored cascadeTitleRename to be POSITION-BASED: SELECT each individual wl row with its position + target_title (no longer DISTINCT sources). Per-row, rewrite the bracket AT THAT EXACT POSITION via new links.RewriteBracketAt helper. Process rows in descending position order per source so earlier offsets don't shift. Brackets whose wl row doesn't resolve to the renamed item are never visited. Scoped self-rewrite — title-only renames cascade self (input.Content == nil); combined title+content renames EXCLUDE self (input.Content != nil). User-supplied content is authoritative; auto-rewriting their just-submitted brackets would surprise them. Mirrors documents.go::updateLinksInTx, which also leaves the renamed entity's own content alone. Codex round 6 finding 2 is fully addressed: title-only path still rewrites self-refs, combined path respects user submission and the index correctly records the bracket as broken (matching what the renderer would render). New links.RewriteBracketAt helper with full unit-test coverage: plain/aliased/qualified/qualified+aliased shapes, case-insensitive matching, slug-prefix preservation, full-body vs split-key target disambiguation, out-of-bounds defensive guards. Integration test TestWikiLinks_CascadeDoesNotCorruptLiteralPipeNeighbor reproduces Codex round 7 finding 2's scenario. PLAN-1593 / TASK-1595. * fix(backlinks): retarget rows pointing at soft-deleted targets (Codex round 8) Codex round 8 P2: resolveBrokenTitleLinks only considered target_item_id IS NULL rows as eligible for flip. A row that resolved to item A and then had A soft-deleted stayed pointing at deleted A; creating a new item B titled the same as A wouldn't flip the row, so GetBacklinks(B) missed the backlink the renderer would actually show (renderer hides deleted-target links). Fix: introduce a "broken-in-practice" predicate (target_item_id IS NULL OR NOT EXISTS ( SELECT 1 FROM items t WHERE t.id = item_wiki_links.target_item_id AND t.deleted_at IS NULL )) applied to stages 1 (plain literal flip) and 2 (qualified literal flip). Stage 3 (slash-title literal-arrival retarget) already considered "current target deleted" implicitly via its title- mismatch EXISTS check. Regression test in wiki_links_test.go covers the exact scenario: A "Foo" resolves a backlink → A soft-deleted → B "Foo" created → backlink flips to B. Known limitation deferred to v3: the symmetric case (delete A while B with same title already exists) doesn't fire any hook that re-resolves the row. A dedicated DeleteItem hook would close that gap; not blocking Phase 2a. PLAN-1593 / TASK-1595. * fix(backlinks): preserve title whitespace to match renderer (Codex round 9) Codex round 9 P2: parseBody trimmed whitespace from the body BEFORE deciding it was a title kind. The renderer doesn't trim before title matching (markdown.ts:541-543) — `[[ Foo ]]` is matched against items.title with the surrounding spaces intact, so an item titled "Foo" wouldn't match. Trimming server-side created index entries the UI couldn't click — backlinks showed in the panel for links that the renderer rendered as broken. Fix: keep an UNTRIMMED unescaped body for title-kind fallthrough, and a trimmed copy only for ref / workspace_ref SHAPE detection (refs are whitespace-free by construction, the renderer's key.trim() at L503 is just typing forgiveness for the ref form). Tests in extract_test.go: - `[[ Foo ]]` emits title-kind with Title=" Foo " (whitespace preserved). - `[[ TASK-5 ]]` still parses as ref (shape detection trims). - `[[Project Goals]]` (two spaces inside) preserves internal whitespace. PLAN-1593 / TASK-1595. * fix(backlinks): ref→title fallback uses raw untrimmed key (Codex round 10) Codex round 10 P2: after round 9's title-kind whitespace fix, the ref→title FALLBACK path still used canonical-trimmed link.Ref for its title lookup. The renderer's fallback at markdown.ts:541-543 uses the UNTRIMMED key (no .trim() on the title-lookup path), so `[[ TASK-5 ]]` falling through to title would search " TASK-5 " (with whitespace) — an item literally titled " TASK-5 " resolves in the UI but not in our index. Fix: add a RawKey field to WikiLinkRef capturing the untrimmed unescaped key for ref kinds. parseBody populates it alongside the canonical Ref. replaceWikiLinks' ref→title fallback path uses RawKey (with defensive fallback to Ref for old rows) when constructing title candidates. Mirrors the renderer's untrimmed title lookup across both bare and pipe forms. Regression test in wiki_links_test.go covers the exact scenario: item titled " TASK-5 " (with whitespace), source body `[[ TASK-5 ]]`, backlink resolves via the untrimmed ref→title fallback. PLAN-1593 / TASK-1595.	2026-05-24 12:01:05 -04:00
xarmian	8e7d4040fd	feat(backlinks): server-side reverse index for [[...]] (Phase 1) (#620) ... * feat(backlinks): server-side reverse index for [[...]] wiki-links (Phase 1) First phase of PLAN-1593. Today [[REF]] is parsed only at render time on the client and there's no way to ask "who links to TASK-5?" without a full-text scan. This change adds a materialized reverse index (item_wiki_links) that's written every time an item's content changes and exposes it via REST + CLI. Phase 1 covers ref-form links only (`[[TASK-5]]` / `[[TASK-5|Display]]`). Phase 2 (TASK-1595) will extend to titles + cross-workspace; Phase 3 (TASK-1596) adds the web UI panel + MCP action. What lands here: * Migrations 061 (SQLite) and 040 (Postgres) create item_wiki_links with partial indexes on target_item_id, (target_workspace_id, target_ref), and target_title — the schema accommodates all 5 wiki-link forms up-front so Phase 2 doesn't ALTER. * internal/links/extract.go is the canonical parser. It strips fenced and inline code regions before extracting [[...]] occurrences, so example refs in docs / code blocks don't pollute the index. Phase 1 emits only WikiLinkKindRef rows; title and workspace_ref kinds parse successfully but are gated out until Phase 2. * internal/store/wiki_links.go (replaceWikiLinks + GetBacklinks + helpers) handles write-time bookkeeping and the read query. Resolution to target_item_id happens at parse time inside the same transaction as the items INSERT/UPDATE, so partial state never lands. Broken refs (target_item_id IS NULL) intentionally persist — they feed a future broken-links report. * internal/store/wiki_links_backfill.go + cmd/pad/main.go hook the idempotent backfill into server startup. Existing items get indexed on first boot after the migration; subsequent boots are near-no-ops via an EXISTS short-circuit. * internal/store/items.go is amended in two places: tryCreateItem always calls replaceWikiLinks (empty content → no-op DELETE), and UpdateItemWithPreCheck re-parses whenever input.Content was supplied. * internal/server/handlers_backlinks.go serves `GET /api/v1/workspaces/{ws}/items/{itemSlug}/backlinks` with visibility + guest-grant filtering on the source items. * internal/cli/client.go adds GetBacklinks; cmd/pad/main.go adds the `pad item backlinks <ref>` command (registered in groups.go). Behavior decisions (per PLAN-1593): - code blocks excluded (fenced + inline) - self-links filtered at query time (kept in storage) - repeated mentions stored as separate rows by position - ordering: source updated_at DESC, position ASC Tests: - internal/links/extract_test.go: 26 sub-cases covering ref/title/ workspace-ref discrimination, code-block exclusion (fenced + inline + unclosed fence), position-is-byte-offset (UTF-8 safety), and edge inputs. - internal/store/wiki_links_test.go: 8 integration tests covering the create/update/delete/self-link/broken-ref/repeated/code-block scenarios plus backfill idempotence. All pass. `make check` clean (lint + go test + web build). Refs: TASK-1594, PLAN-1593, IDEA-1577 * fix(backlinks): visibility-aware pagination + case-insensitive refs per Codex review (round 1) Two fixes from Codex code review: P1 — GetBacklinks now takes a visibleCollectionIDs []string argument that's applied INSIDE the SQL WHERE clause. Previously the handler fetched LIMIT raw rows and filtered visible ones in Go, so a restricted user asking for limit=50 could receive an empty page even when later visible backlinks existed. Pushing visibility into SQL makes LIMIT/OFFSET count visible rows. nil → no restriction (owners, editors, root tokens) [] → see nothing (returns early, no SQL) [..] → AND s.collection_id IN (?, ?, ...) Item-level guest grants still apply post-fetch — they're rare enough that the residual page shrink is acceptable and pushing them into SQL would balloon the query. P2 — refPattern now accepts mixed/lowercase refs and parseBody canonicalizes the prefix to uppercase at the single chokepoint. Previously the renderer accepted `[[task-5]]` as a real link (its REF_PATTERN is case-insensitive) but the indexer's ^[A-Z]... pattern silently dropped it — divergent parsing on the same input. Storage shape is canonical uppercase so the (workspace, prefix, number) lookup against collections.prefix (also uppercase) has one shape. New helper: canonicalizeRef("task-5") → "TASK-5". Regressions: internal/links/extract_test.go + TestCanonicalizeRef — helper unit tests + TestExtractWikiLinks_RefVsTitleFallback updated to assert mixed/lowercase parses-as-ref-and-uppercases + edge-case test renamed from "lowercase ref" to "number-led not a ref" (lowercase IS a ref now per Codex P2) internal/store/wiki_links_test.go + TestWikiLinks_MixedCaseRefIndexed — `[[task-5]]` produces a backlink row whose target_ref is "TASK-5" + TestWikiLinks_VisibilityAwarePagination — three sub-cases: nil → all 3, visible-only limit=2 → 2 visible rows (not 1 with hidden one consuming a slot), empty → 0 All call sites updated (8 in tests + 1 in handler). `make check` clean (lint + tests + web build). Refs: TASK-1594, PLAN-1593 * fix(backlinks): SQL-level item-grant filter per Codex review (round 2) Round 1 fixed pagination for collection-level visibility but Codex round 2 correctly flagged the same class of bug at the item-grant layer: `visibleCollectionIDs` returns the UNION (full grants ∪ collections containing granted items), and the handler then filtered each row's item-level visibility in Go AFTER fetching — letting hidden rows in a granted-item's collection consume LIMIT slots. The refactor moves the precise predicate into SQL. New shape: type BacklinksVisibility struct { Unrestricted bool // admin / full-access member FullCollectionIDs []string // direct collection grants GrantedItemIDs []string // item-level grants } // SQL predicate when Unrestricted=false: // AND (s.collection_id IN (?...) OR s.id IN (?...)) This matches `guestResourceFilter` (which returns the precise primitives), so the handler now passes them straight through and drops the post-fetch filter loop entirely. Pagination is correct for guests, restricted members, and unrestricted users alike. New test: TestWikiLinks_ItemGrantPagination — guest with item-grant on ONE item in an otherwise-hidden collection sees exactly that one item; hidden siblings in the same collection do NOT leak in, and limit=2 returns 1 row (not silently shrunken). Other call sites updated: - TestWikiLinks_VisibilityAwarePagination → uses BacklinksVisibility{FullCollectionIDs: ...} and BacklinksVisibility{} for the no-access case. - 8 existing tests → BacklinksVisibility{Unrestricted: true}. - handlers_backlinks.go → no longer calls visibleCollectionIDs; uses guestResourceFilter exclusively and skips the Go-side filter. Verification: - make check clean - All TestWikiLinks_* pass Refs: TASK-1594, PLAN-1593 * fix(backlinks): scan EXISTS into bool not int for Postgres parity (Codex round 3) `SELECT EXISTS(...)` returns boolean on Postgres but integer 0/1 on SQLite. Scanning into `int` happened to work on SQLite (the modernc.org driver coerces) but would fail on Postgres — silently disabling the backfill short-circuit there and meaning upgraded Postgres installs wouldn't populate backlinks for pre-existing content until each item got edited. Fix: scan into bool. Both database/sql drivers in use (modernc.org/ sqlite and lib/pq) coerce their native representation into Go's bool, so this single shape works on both engines. make check clean. Refs: TASK-1594, PLAN-1593 * fix(backlinks): allow CommonMark 0-3 space indented fences per Codex (round 5) Round 5 flagged two edge cases in the code-stripping pass: 1. Multi-backtick inline code (``see [[X]]``) — traced through the parser; my permissive close-on-next-backtick logic already covers it correctly (range = [opener-start, after-closer-run]). Added a regression test to lock this in: TestExtractWikiLinks_CodeBlocksExcluded / "multi-backtick inline code excludes ref" 2. Indented fenced blocks — CommonMark allows 0-3 leading spaces of indentation before a fence opener (4+ spaces makes it an indented code block, a different construct). My fencedCodeRanges only matched fences at column 0, so ` ```\n[[X]]\n```` ` would render as code in the UI but leak a false backlink. Fixed both fencedCodeRanges (opener) and findFenceCloser (closer) to skip up to 3 leading spaces, with a hard cap at 4 (which would be indented-code, not a fence). Regression test: TestExtractWikiLinks_CodeBlocksExcluded / "indented fenced block (CommonMark 0-3 spaces)" Not addressed: - Round-4 escape-body parity finding. extract.go mirrors renderMarkdown's regex (web/src/lib/utils/markdown.ts:300), which is the actual render-time link parser; wikiLinksToMarkdown's more permissive escape grammar is editor-serializer-side and the renderer can't even consume its escaped output. Indexing what the user actually sees as a link is the correct invariant. make check clean. Refs: TASK-1594, PLAN-1593 * fix(backlinks): tilde fences + strict closer lines per Codex (round 6) Two CommonMark conformance gaps in the code-block stripping pass: 1. Tilde-fenced code blocks (~~~) were ignored. marked() treats them the same as backtick fences, so a [[REF]] inside a tilde block would render as code in the UI but leak as a false backlink. Fixed by parameterizing fenceChar across fencedCodeRanges and findFenceCloser, with separate handling for the backtick-specific "no backtick in info string" rule (CommonMark §4.5). 2. Closer-line strictness — CommonMark requires the closing fence line to contain only the fence + optional trailing spaces. The previous accept-any-fence-prefixed-line check would terminate a still-open fence prematurely on a line like ```not-closed, leaking later refs in the still-rendered code block. Refs reside in 4 new sub-tests under TestExtractWikiLinks_CodeBlocksExcluded: - tilde fence excludes refs inside - tilde fence with language tag - mixed fence types don't pair - closer-line strictness — backticks plus other text is not a closer - closer-line strictness — trailing spaces OK make check clean. Refs: TASK-1594, PLAN-1593 * fix(backlinks): inline code closer must match opener length per Codex (round 7) CommonMark §6.1 requires an inline-code span opened with N backticks to close on a run of EXACTLY N backticks. The previous "close on next backtick run of any length" logic would prematurely end the excluded range on a stray single backtick inside a ``...`` span, leaking any [[REF]] in the latter half of the code text as a false backlink. Concrete failure case: ``has ` inside [[X-1]] and more`` → old: range [0, 7], [[X-1]] indexed (bug) → new: range [0, end-of-closer], [[X-1]] excluded (correct) Fix: track the opener-run length and scan only for matching-length closer runs. Wrong-length runs in between are code text. Two new regression sub-cases under TestExtractWikiLinks_CodeBlocksExcluded: - inline code closer matches opener length — the main case - single-backtick span unaffected by adjacent multi-backtick run — asserts the opposite direction (opener=1 doesn't close on ``) Not addressed: - Re-flagged round-4/round-7 escape-body parity finding. extract.go intentionally mirrors renderMarkdown's regex (markdown.ts:300), not wikiLinksToMarkdown's more permissive escape grammar (markdown.ts:461). renderMarkdown is the actual link parser at display time; its regex rejects escaped-`]` bodies, so any link with an escaped `]` in its body is NOT shown as a clickable link in the UI. Indexing it would produce phantom backlinks the user can't see. The wikiLinksToMarkdown permissive grammar is paranoid serialization that the renderer can't consume — that's a pre-existing inconsistency in the editor pipeline, not a backlinks bug. make check clean (lint + tests + web build). Refs: TASK-1594, PLAN-1593 * fix(backlinks): rune-align snippet end-edge to keep UTF-8 valid (Codex round 8) The previous snippetAround() trimmed `start` to a rune boundary (so the leading edge of the snippet was always at a valid codepoint) but left `end` as a raw +40-byte clamp. When that landed in the middle of a multi-byte rune — common around emoji or accented text — the resulting slice was invalid UTF-8 and the JSON encoder would emit replacement characters in backlink snippets. Fix: same forward-advance pattern at the end as at the start. Continuation bytes (10xxxxxx) get skipped until we land on a leading byte. Going forward keeps the snippet anchored slightly past the match rather than slightly before it, which is a small UX win (emoji or accented text right after the link survives intact). Regression test: TestWikiLinks_SnippetIsValidUTF8 — pads body with enough 4-byte emoji on each side that the ±40-byte window cuts through one; asserts utf8.ValidString on the resulting snippet. make check clean (lint + tests + web build). Refs: TASK-1594, PLAN-1593 * fix(backlinks): inline code spans cross newlines, break on blank lines (Codex round 9) CommonMark §6.1: an inline-code span can cross single newlines but terminates at a blank line (a line containing no chars or only whitespace, which ends the enclosing paragraph). My previous scanner broke at every newline, so multi-line spans like `pre [[INSIDE-1]] post` would treat the opener as unclosed and leak [[INSIDE-1]] as a false backlink. Fixed by: 1. The newline branch in the closer scan now peeks ahead via the new isBlankLineAt() helper. Same-paragraph newlines are traversed; blank-line breaks terminate the span unmatched. 2. isBlankLineAt() treats any line with only space/tab as blank (mirroring CommonMark's blank-line definition). Three new regression sub-cases under TestExtractWikiLinks_CodeBlocksExcluded: - inline code spans single newline (CommonMark §6.1) - inline code breaks at blank line (paragraph boundary) - inline code breaks at whitespace-only blank line Trade-off: a truly-unclosed inline backtick now consumes from the opener up to the next blank line instead of just the rest of the line. False-positive on wiki-links in that span, but the surface area is small (unclosed backticks are rare in published prose) and matches the renderer's behavior. make check clean. Refs: TASK-1594, PLAN-1593 * fix(backlinks): accept escaped wiki-link bodies per editor grammar (Codex round 10) After 3 rounds of disagreement, capitulating on the escape-body parity finding. My position was technically correct for the CURRENT renderMarkdown behavior (which uses [^\]]+ and can't parse escaped- bracket bodies), but the editor's wikiLinksToMarkdown grammar at markdown.ts:461 explicitly produces such bodies — making the renderer's regex the inconsistent half of the pipeline, not mine. Mirroring the editor's grammar in the extractor makes the index forward-compatible: when the renderer eventually gets fixed, no change here is needed. The cost is a few "phantom" rows in the interim (indexed links the renderer doesn't currently display as clickable), but those are harmless and aligned with author intent. Changes: - wikiLinkPattern now uses `\[\[((?:\\.|[^\]\\])+)\]\]` — mirrors markdown.ts:461 verbatim. - New splitOnUnescapedPipe() helper — scans for the first `|` that isn't preceded by `\`. Mirrors splitWikiBody at markdown.ts:664. - New unescapeWikiBody() helper — undoes `\]`, `\|`, `\\` escapes in display text and key. Mirrors unescapeWikiBody at markdown.ts:657. - parseBody() now uses both helpers — split on unescaped `|`, unescape both sides. Regression coverage: - TestExtractWikiLinks_EscapedBodyChars (5 sub-cases): escaped `]`, escaped `|`, escaped `\`, non-escape backslash passes through, Position still points at opening `[[` despite escapes. - TestSplitOnUnescapedPipe + TestUnescapeWikiBody: direct unit tests for the helpers (round-trip safety vs the editor's escape/unescape pair). make check clean. Refs: TASK-1594, PLAN-1593 * fix(backlinks): preserve display text verbatim per Codex round 11 P3 The previous parseBody trimmed the display side of [[X|Display]] but the WikiLinkRef.Display contract promises verbatim storage and the renderer at markdown.ts doesn't trim either. Trimming would silently diverge on padded display text like [[TASK-1| spaces ]] (renderer keeps the spaces, extractor stripped them). Fix: drop TrimSpace from the suffix half of the split. Keep trimming the key/ref side because refPattern is anchored — a leading or trailing space in the key would force the body to fall through to the title kind even though the renderer resolves it as a ref. Regression test: TestExtractWikiLinks_EscapedBodyChars / "display text preserved verbatim (no TrimSpace)" make check clean. Refs: TASK-1594, PLAN-1593 * fix(backlinks): distinguish empty display override from no-pipe per Codex round 12 [[REF|]] (explicit empty display) and [[REF]] (no display) are distinct shapes in the editor: splitWikiBody returns displayOverride="" for the former, null for the latter, and the renderer uses `displayOverride ?? title` (nullish coalescing, NOT empty-string fallback) so "" is preserved. The previous extractor collapsed both into display_text=NULL, violating verbatim-display preservation for the empty-string edge case. Fix: - WikiLinkRef gains a HasDisplay bool. parseBody sets HasDisplay=true iff splitOnUnescapedPipe found a pipe; downstream uses HasDisplay (not Display!="") to decide whether to persist the override. - replaceWikiLinks in store: NullString.Valid is keyed off HasDisplay. display_text='' for explicit empty, NULL for no override. Regression coverage: - internal/links/extract_test.go: "explicit empty display override is distinguished from no pipe" - internal/store/wiki_links_test.go: TestWikiLinks_EmptyDisplayDistinct (two-source assert: NOT NULL for [[REF|]], NULL for [[REF]]) make check clean. Refs: TASK-1594, PLAN-1593 * fix(backlinks): pointer-typed DisplayText to preserve empty distinction over JSON (Codex round 13) Round 12 added HasDisplay on the parser side and made the store preserve display_text='' vs NULL on the DB row, but the wire model collapsed the distinction at JSON-serialization time: DisplayText string `json:"display_text,omitempty"` `omitempty` drops empty strings, so [[REF|]] (empty override) and [[REF]] (no override) serialized identically on the API and CLI JSON output. The end-to-end goal of round 12 wasn't reached. Fix: change DisplayText to *string. nil → no override (field omitted from JSON via omitempty), pointer to "" → explicit empty override (field present with empty value). The store's NullString.Valid drives the assignment, so the SQL round-trip matches the JSON shape. Knock-on: the CLI's `pad item backlinks` now dereferences the pointer and prints both populated and empty overrides ("displayed as: "). Regression coverage: - TestWikiLinks_EmptyDisplayDistinct extended to assert withBL.DisplayText is non-nil-pointing-at-"" and noBL.DisplayText is nil after a GetBacklinks round-trip. make check clean. Refs: TASK-1594, PLAN-1593	2026-05-23 23:09:44 -04:00
xarmian	a6ca1f3910	docs: add cross-link nav row to README (site, blog, changelog, X, Bluesky) (#617) ... A single centered nav line right under the badges so readers can reach the marketing site, docs, blog, changelog, and social accounts from the top of the repo without scrolling. Covers TASK-1569 (blog/X/Bluesky added) and TASK-1574 (getpad.dev/docs/ changelog quick-nav). Bundled into one PR since both edit the same file and TASK-1574 explicitly called out the overlap. Refs: PLAN-1571, TASK-1569, TASK-1574	2026-05-23 11:42:47 -04:00
xarmian	8cf460b381	fix(ci): unbreak Go (PostgreSQL) test + clear x/net govulncheck findings (#618) ... * fix(test): encode bools as bools in TestGetUserWorkspacesDetailed (BUG-1582) The raw INSERT in this test passed integer literals `0, 0, 1` for `sort_order, is_default, is_system`. SQLite coerces int → bool but pgx refuses, so the Go (PostgreSQL) CI job has failed every run since #600 landed: workspace_members_admin_detail_test.go:42: seed system collection: failed to encode args[10]: unable to encode 0 into binary format for bool (OID 16): cannot find encode plan Pass `false, true` for the two bool columns so both drivers accept the args. * chore(deps): bump golang.org/x/net to v0.55.0 (TASK-1583) Clears 5 govulncheck findings (GO-2026-5025..5030) reachable via internal/urlimport/generic.go's call to html.Parse. The Go CI job has been failing on every main run since these advisories were published. Vulnerability #1: GO-2026-5030 — XSS via duplicate attributes Vulnerability #2: GO-2026-5029 — character refs in DOCTYPE Vulnerability #3: GO-2026-5028 — DoS parsing arbitrary HTML Vulnerability #4: GO-2026-5027 — HTML elements in foreign content Vulnerability #5: GO-2026-5025 — namespaced elements in foreign content `go mod tidy` pulls along the usual x/* sibling bumps. Local govulncheck after the bump: *No vulnerabilities found.* Full `go test ./...` passes. * fix(store): is_system check uses NOT bool, not = 0 (BUG-1582) GetUserWorkspacesDetailed's collections_count subquery had `c.is_system = 0`. SQLite stores BOOLEAN as INTEGER so the comparison worked there, but Postgres' boolean column rejects the integer literal: ERROR: operator does not exist: boolean = integer STATEMENT: SELECT ... AND c.is_system = 0) The first push at this BUG only fixed the test-side encoder issue; this commit fixes the production query that the test exercises. `NOT c.is_system` evaluates correctly on both drivers without needing to thread another placeholder through the args. Verified locally against a real Postgres 17 instance: TestGetUser- WorkspacesDetailed and the full ./internal/store/... suite pass.	2026-05-23 11:36:16 -04:00
xarmian	66dce32c7e	chore(mcp): point library-activate not-found hint at pad_library (TASK-1564) (#616) ... The dispatcher's `library activate` not-found error referenced a `pad_project action=library-list` action that never existed — leftover from an earlier design draft for IDEA-1514 (the now-shipped `pad_library` tool, PLAN-1560 / TASK-1563). The correct hint is `pad_library action=list`. One-line hint fix; no behavior change. Closes the remaining scope of TASK-1564 — the onboard playbook body update shipped in PR #615 (commit 1638bdf) per Codex review. No CHANGELOG entry added: the project has no top-level CHANGELOG.md and tracks history via git + Pad items. Closes TASK-1564 → completes PLAN-1560 (IDEA-1514).	2026-05-21 20:14:58 -04:00
xarmian	6433cc51ea	feat(mcp): pad_library catalog tool + ToolSurfaceVersion 0.5 (TASK-1563) (#615) ... * feat(mcp): pad_library catalog tool + ToolSurfaceVersion 0.5 (TASK-1563) MCP catalog wiring for PLAN-1560 (`pad_library` MCP tool + matching CLI surface). Closes IDEA-1514 — pure-MCP agents (notably the /pad onboard playbook from PLAN-1496) can now browse and activate library entries without shelling out. ## New tool `pad_library` joins the v0.5 catalog as the ninth resource × action tool. Three actions, all passThrough to the `pad library` CLI: - `list` — Browse conventions + playbooks. Defaults to summary mode for playbooks (compact bodies via the ?summary=true endpoint flag); conventions always carry full content. Optional type / category / full inputs. - `get` — Full body of one entry by exact title. Conventions-first precedence mirrors `activate`. - `activate` — Create a workspace item from a library entry by title. `Workspace: true` on the tool — list/get ignore it; activate validates and uses it. The schema-level declaration gives activate automatic pad_set_workspace session-default resolution (same precedent as pad_meta's mixed-workspace actions). ## Dispatcher extensions - `dispatchLibraryList` forwards `category` to BOTH endpoints and passes `summary=true` to the playbook endpoint by default (unless input.full=true). MCP-default summary mode keeps agent context budgets tight; CLI default already aligned in TASK-1562. - `library get` added to the routeTable as a clean GET to /api/v1/library/entry with `title` mapped to the query string. Cleaner than another explicit dispatcher case — matches playbook list / playbook show shape. ## Version bump ToolSurfaceVersion bumped from 0.4 → 0.5. Pure addition; no existing tool/action/param/bootstrap shapes changed. Backwards-compatible for any v0.4 consumer that doesn't enumerate the new tool. Documented in version.go with the same comment-block structure as prior bumps. ## Test coverage - catalog_readonly_test.go — pad_library added to the want{} map; three library action → cmdPath entries in expected{}; library list / get / activate added to liveCmdhelpDoc stubs. - dispatch_http_project_test.go — 4-case table test (defaults, category, full=true, category+full) pins category/summary query-param forwarding; library get routing test confirms the routeTable entry resolves. ## Live MCP verification - `initialize` handshake advertises padToolSurface.version=0.5. - `pad_meta version` returns tool_surface_version=0.5. - `pad_library list type=playbooks category=agent-workflows` returns 4 playbooks in summary mode (content stripped, summary populated, invocation_slug + arguments present). - `pad_library get title='Ship tasks'` returns {type: playbook, playbook: {…, content (9512 chars), invocation_slug: ship}}. Parent: PLAN-1560. Unblocks TASK-1564 (cleanups). * fix(onboard): update playbook body to use pad_library MCP tool per Codex review (round 1) Codex P2 on PR #615: the /pad onboard playbook body in internal/collections/playbook_library_onboard.go still told MCP-only agents that the library catalog was "not yet exposed as an MCP tool" and to work from memory — directly contradicting the pad_library tool this PR just landed and breaking the main advertised consumer of the new surface. Updated step B3 (conventions) to mention both surfaces side-by-side (`pad library list --type conventions` / `pad_library` with `action: list, type: conventions`), and rewrote step B5 (playbooks) the same way so the activate path doesn't drift either. Pre-PLAN-1560 IDEA-1514 reference removed from the body — the idea is now closed. No test pins the playbook body content; `make check` passes; the playbook seed still validates against the playbooks collection schema since trigger/scope/invocation_slug/arguments are unchanged. Closes the onboard-side scope of TASK-1564 (stale dispatch_http_slice4 hint + CHANGELOG still pending there).	2026-05-21 19:18:19 -04:00
xarmian	9a47c36ea6	chore(store): rephrase comment to unblock gofmt (BUG-1565) (#614) ... Go 1.26's gofmt rewrites paired ASCII apostrophes (''), used here as a literal SQL empty string, to a typographic right-curly double quote (U+201D). The rewrite is applied even inside markdown backticks, so escaping the SQL fragment in code-span syntax doesn't help. Rephrase the comment to describe the COALESCE/LOWER pattern in words instead of embedding the SQL literal, preserving the original meaning while sidestepping the heuristic. The behavior of adminOpenItemsCountClause is unchanged — comment-only edit. Unblocks `make check` for local pre-commit and CI gates. Surfaced during PLAN-1560's TASK-1561 ship loop.	2026-05-21 17:14:21 -04:00
xarmian	de1beb47a9	feat(cli): pad library get + list --full + server-side category filter (TASK-1562) (#613) ... CLI layer for PLAN-1560 (`pad_library` MCP tool + matching CLI surface). Wires the HTTP work landed in TASK-1561 through to the `pad library` subcommands. ## `pad library list` changes - `--category` is now a server-side filter (the old client-side display-only skip-loop is dead and removed). - New `--full` flag. Default JSON output for playbooks now returns the `summary` field (first non-heading paragraph, ~240 char cap) instead of the full `content`; `--full` opts back into full bodies for callers that want to pipe everything. - Table output gains a summary hint line under each playbook and a `/pad <slug>` chip when an invocation slug is declared, so the library becomes self-documenting as a discovery surface. - `--type` now validates explicitly instead of silently producing an empty list for unknown values. ## NEW `pad library get <title>` Calls `GET /api/v1/library/entry?title=X` and renders either a conventions card (title, category, trigger, surfaces, enforcement, commands, body) or a playbooks card (title, category, trigger, scope, invocation slug, argument count, body). Conventions-first precedence matches `pad library activate`. JSON output returns the full envelope. 404 errors return a clean `not found in library: "<title>"` message with exit code 1. ## CLI client - `GetConventionLibrary(category)` — pass category as a server-side query param. - `GetPlaybookLibrary(category, summary)` — same plus the summary toggle; `summary=true` strips Content and returns Summary instead. - NEW `GetLibraryEntry(title)` returning `*LibraryEntryResponse`. - `LibraryPlaybook` gained an omitempty `Summary` field so a single type round-trips both the legacy and summary shapes. ## Drive-by Switched `/library/entry` 400/404 from a flat `{error: "..."}` body to the canonical `writeError(code, message)` envelope used by the rest of the API. The CLI's `parseError` now hands back a typed `APIError` that `pad library get` pattern-matches on `Code=="not_found"` for the clean 404 message. Updated `TestLibraryEntry_MissingTitle` and `_NotFound` to assert the new envelope. ## Verification go build / go vet / go test ./... all green. golangci-lint clean on cmd/pad/..., internal/cli/..., internal/server/.... End-to-end smoke tests via the installed binary confirmed: list summary mode, list --full, list --category filter, get convention card, get playbook envelope, get 404 exit-1, --type validation. Parent: PLAN-1560. Unblocks TASK-1563 (MCP catalog wiring).	2026-05-21 16:59:57 -04:00
xarmian	2df6edeaab	feat(server): library endpoints gain ?category, ?summary, /library/entry (TASK-1561) (#612) ... Extends the convention + playbook library HTTP layer to support the shape the upcoming `pad_library` MCP tool and the updated `pad library` CLI need: - `GET /api/v1/convention-library?category=X` — server-side filter, case-sensitive exact match. Unknown categories return an empty slice, not 404. - `GET /api/v1/playbook-library?category=X&summary=true` — same filter plus a new summary mode that strips Content and injects Summary (first non-heading paragraph, ~240 char cap). Web UI and existing consumers omit the flag and see the legacy full-body shape. Summary mode deep-copies category slices so a request never mutates the package-level library data — TestPlaybookLibrary_SummaryDoesNotMutate Global pins this. - `GET /api/v1/library/entry?title=X` — NEW. Returns one matched entry in a `{type, convention|playbook}` envelope. Conventions-first precedence mirrors the dispatcher's `library activate` so a title resolves to the same kind in both surfaces. 400 on missing title, 404 on no match. Hoisted `playbookSummary` to `collections.PlaybookSummary` so the bootstrap handler and the new library endpoints share one algorithm. Bootstrap continues to call it for every playbook entry it returns. Adds 12 handler tests + the existing bootstrap-summary test stays green after the move. Lint clean on touched packages; `make check` gate is blocked by a pre-existing gofmt issue in internal/store/workspace_members.go captured as BUG-1565. Parent: PLAN-1560. Unblocks TASK-1562 (CLI) and TASK-1563 (MCP catalog).	2026-05-21 13:08:02 -04:00
xarmian	ef7eabaddc	fix(web): admin modal reactive loop on open (#611) ... * fix(web): admin modal reactive loop on open (PLAN-1542 follow-up) User reported the modal froze the page on open. Root cause was the UserSettingsForm hydration \$effect added in TASK-1551 — it read prop fields then reassigned editOverrides to a fresh object and mutated per-key inside the same effect, while the template's {#each overrideFields} block subscribed via bind:value to those exact keys. The reassign+mutate pattern thrashed the bind subscriptions which scheduled the effect's tracking owner, looping until effect_update_depth_exceeded. Fixes (defense in depth): 1. UserSettingsForm hydration \$effect now gates on user.id only (a primitive read with no proxy churn) and wraps all writes in untrack(). The objects are built locally then assigned ONCE per state var — no "reassign then mutate" pattern. Side effect: form state isn't nuked by the parent's modalUser = { ...modalUser, ...updated } spread after every save. The form's own state is authoritative for unsaved edits; re- hydration only happens when the modal swaps to a different user. 2. UserModal now mounts only the active tab's body. The earlier {#each TABS} + hidden pattern instantiated all four tab components on every modal open, amplifying the UserSettingsForm loop and firing three lazy-fetch effects in parallel for tabs the admin may never click. Tabs are now mount-on-active — lazy fetch happens when the admin selects the tab. 3. Lazy-fetch tabs (Overview/Workspaces/Activity) now claim fetchedForUserId BEFORE the await, not after. A re-trigger of the gating effect (parent passes a new modalUser object with the same id) can no longer race in and fire a duplicate concurrent fetch. On failure, the claim is cleared so a retry-via-reactivation still works. Verified via svelte-check (0 errors) and npm run build. * fix: address Codex review on modal reactive-loop fix 1. Keep UserSettingsForm mounted across tab switches (hidden toggle rather than {#if}). Mount-on-active for the Settings tab meant an admin who tabbed away to Workspaces and back lost unsaved overrides and the typed-email disable input. The form has no fetch — its hydration $effect is now gated on user.id so the original loop stays fixed, and keeping it alive is cheap. Lazy-fetch tabs (Overview / Workspaces / Activity) still mount-on-active so their network calls fire only when needed. 2. UserOverviewTab now clears fetchedForUserId when EITHER metrics or recent-items branch fails (was: only when both failed). A partial failure left the failed half stuck on the next activation until the admin force-refreshed the page. The successful half gets re-fetched on retry, which is cheap. * fix: Overview tab partial-failure retry is now explicit, not auto Clearing fetchedForUserId on failure while the tab is still active re-triggered the gating \$effect immediately — under a persistent outage that meant a continuous refetch loop and the successful half's data getting wiped on every iteration. fetchedForUserId now stays set on failure; the failed branch renders an inline Retry button that calls loadAll() directly. * fix: apply the same retry-claim fix to Workspaces + Activity tabs Codex caught the same auto-retry-loop pattern in UserWorkspacesTab and UserActivityTab: clearing fetchedForUserId on failure while the tab is active re-triggers the gating $effect → immediate refetch → repeat. Both already render explicit Retry buttons (calling loadDetail() / loadInitial() directly), so the claim can stay set on failure.	2026-05-20 20:17:51 -04:00
xarmian	a2c8c67a9c	refactor(web): delete inline-expand DOM (TASK-1555) (#610) ... * refactor(web): delete inline-expand DOM from admin users page (TASK-1555) Closes the parallel-UI broken state opened in TASK-1550 and maintained through T1551-T1554. The modal (UserModal + UserOverviewTab + UserWorkspacesTab + UserActivityTab + UserSettingsForm) now owns every admin-user-detail surface; the inline expand goes away. Removed from +page.svelte: - The {#if selectedId === user.id} ... edit-row block (220 LOC of DOM) - selectedId state + class:selected binding on the row - selectUser() helper (the row click now goes directly to openUserModal) - All inline-expand-only state: editRole, editPlan, editOverrides, extraOverrides, editStorageOverride, storageOverrideError, saving, saveMsg, roleConfirm/roleSaving/roleMsg, resetConfirm/resetSaving/ resetResult/resetError, disableConfirm/disableSaving/disableMsg, userWorkspaces, workspacesLoading - All inline-expand-only handlers: loadUserWorkspaces, selectedUser, roleAction, changeRole, resetPassword, toggleDisable, saveUser - Dead helpers: parsePlanOverrides, parseStorageInput, storageOverridePreview (the parse-side now lives in UserSettingsForm.svelte where it's actually used) - ~190 lines of dead CSS (.edit-row, .edit-panel, .edit-field*, .overrides-grid, .override-field*, .storage-input*, .storage-preview*, .role-row, .role-confirm*, .reset-result, .temp-password*, .ws-list/.ws-item/.ws-name/.ws-joined, .badge.owner, .user-row.selected) Kept (still in use by the table): - formatStorageBytes — Storage cell renders bytes per row - relativeTime, writeRecency — Last Write / Last Active columns File LOC drops from 1482 → 735 (~750 lines removed, 50% smaller). Part of PLAN-1542. With this merge the plan is complete — backend foundation (T1543–T1547), table UX (T1548–T1549), and the modal (T1550–T1555) are all live. * fix: address Codex review on TASK-1555 Three cleanup misses from the original deletion sweep: 1. Drop unused imports: adminPatch, adminPost — only adminFetch is still used by the page (for the list + status counts), the mutating calls all live in UserSettingsForm now. 2. Update stale modal comments. The "both UIs coexist" / "shell only" notes were accurate during T1550-T1554 but now describe the past; replaced with a one-line description of the modal's role. 3. Drop dead CSS rules that survived the inline-expand removal: .btn.primary, .btn.primary:hover, .badge.disabled (replaced by .badge.status-disabled in T1548), .btn.danger, .btn.danger:hover, .btn.primary.danger, .btn.primary.danger:hover. None had any remaining users in the markup; svelte-check now reports zero admin/+page.svelte warnings. * fix: drop one more stale comment caught by Codex re-review on TASK-1555	2026-05-20 19:18:13 -04:00
xarmian	88a30e54a4	feat(web): admin modal Activity tab (TASK-1554) (#609) ... * feat(web): admin modal Activity tab with pagination (TASK-1554) New UserActivityTab.svelte renders the full chronological feed from GET /admin/users/{id}/activity (T1546). Each row shows an action icon (emoji glyph as a visual scan aid), a human-readable action summary, the source channel (web / cli / agent / etc.), and a relative timestamp. Pagination via the API's next_offset field — "Load more" button at the foot appends pages until next_offset is null. Page size 20. Note at the top of the tab explains the scope: this is activities AUTHORED by this user (item writes, comments, logins/logouts) — admin actions targeting them as a subject (e.g. another admin disabling them) live in the activities table under a different user_id and aren't shown here. Documented as a known follow-up per T1546's PR. Lazy-fetched on first activation; refetches on user swap; defensive race-condition guards on each request (snapshot userId; commit only if user.id is still the same when the response arrives). Also drops the dead .placeholder CSS rule from UserModal.svelte — all four tabs now render real content as of this task. Part of PLAN-1542. * fix: address Codex review on TASK-1554 1. loadMore failure no longer hides the existing feed. Append errors now go to a separate loadMoreError surfaced inline next to the Load more button, so an admin scrolling through a long activity history keeps everything they've already loaded if the next page fails. Button label switches to "Retry" when a load-more error is set. 2. iconFor + describe coverage extended to every action constant in internal/models/activity.go that can land in a user-authored feed: register, login_failed, password_changed/reset, token_created/ revoked/rotated, totp_enabled/disabled, oauth_login/oauth_login_ failed, member_invited/removed, role_changed, settings_changed, session_ip_changed, account_deleted. Fallback for unknown actions pretty-prints the raw snake_case rather than rendering as-is. * fix: complete admin-on-user audit action coverage for activity tab Adds icons + describe entries for the admin-on-user audit events that land in the user-authored feed when an admin acts on another user (activities.user_id is the actor, target_user_id is in metadata): plan_changed, plan_overrides_changed, password_reset_by_admin, user_disabled, user_enabled. Closes the remaining Codex finding on PR #609.	2026-05-20 19:07:46 -04:00
xarmian	6765542b13	feat(web): admin modal Overview tab (TASK-1553) (#608) ... New UserOverviewTab.svelte — the first thing an admin sees when opening the modal. Three blocks: - Vitals header: name + email + role pill + plan pill + account age. Disabled badge is on the modal header so we don't duplicate it here. - Metric tiles (3): Last write (color-coded by writeRecency bucket — green <7d, yellow ≤30d, red >30d, gray italic for Never), 7d writes, 30d collection breadth. Sourced from GET /admin/users/{id}/metrics (T1547). Tiles fall back to "—" placeholders on fetch failure rather than blocking the rest of the tab. - Recent items: top 5 from /admin/users/{id}/activity?limit=20 filtered client-side to item-write actions (created/updated/archived/restored/ moved). Comments and admin actions stay in the Activity tab (T1554) where they belong. Sparkline deliberately omitted per the locked-in plan decision; same for api_requests_7d (pending IDEA-1556 to add per-request tracking). Lazy fetch on first activation, refetches on user swap, defensive against in-flight modal swaps (commits only if user.id is unchanged). Metrics + recent items fire in parallel — slow metrics endpoint doesn't block the recent list or vice versa. Part of PLAN-1542.	2026-05-20 18:59:26 -04:00
xarmian	46ca27cf28	feat(web): admin modal Workspaces tab (TASK-1552) (#607) ... New UserWorkspacesTab.svelte consumes GET /api/v1/admin/users/{id}/detail (T1545) and renders the per-workspace breakdown: name + slug (link to the workspace), role badge, collections count (excludes system — playbooks + conventions), items open / total, members count, storage in human-readable units, last-activity relative time. Lazy load — fetches only on first activation of the Workspaces tab, not on modal open. Refetches if the modal swaps to a different user without closing. Defensive against in-flight user swaps (commits the result only if user.id hasn't changed underneath). Empty state for users with no workspaces. Retry on error. Server caps at 50; UI caps visible at 20 with "Show all N" affordance for users who own a long tail of workspaces. Wiring: replaces the Workspaces placeholder in UserModal.svelte; passes `active={activeTab === 'workspaces'}` so the component can gate its fetch. Part of PLAN-1542.	2026-05-20 18:55:49 -04:00
xarmian	430872314b	feat(web): admin modal Settings & overrides tab (TASK-1551) (#606) ... * feat(web): admin modal Settings & overrides tab (TASK-1551) Lifts the inline-expand form into the modal's Settings tab via a new UserSettingsForm.svelte component. Per the plan, this is a parallel implementation — the inline expand in +page.svelte is intentionally left intact so behavior parity can be verified side-by-side before T1555 deletes it. Behaviour parity (all matches the existing inline-expand): - Role selector + promote/demote confirm with separate confirm button - Password reset (email path → "message" string from server; temp-password path → revealed code with a "share via secure channel" hint) - Account disable/enable toggle - Plan selector + structured plan_overrides grid + storage override with shorthand parsing (10 GB / 500 MB / -1 / raw bytes) and the live preview chip - Save button writes plan + plan_overrides in one PATCH; empty overrides → empty-string send (preserves the SetUserPlanOverrides clear path from the inline-expand) New for T1551 — typed-email confirmation on the destructive side of disable (matches the destructive-action confirm pattern called out in the task body). Disable button stays disabled until the typed input matches the user's email exactly; Cancel resets the typed value. Enable side does not require typing (it's reversible). Wiring: - UserModal.svelte gains an optional onUserUpdated callback prop. The Settings form bubbles every successful save through it; the page merges the refetched row into its users[] and the bound modalUser so both UIs (modal + inline-expand) see the same state. - Helpers (parsePlanOverrides, parseStorageInput, formatStorageBytes) are duplicated inside UserSettingsForm for this PR — collapsing back to one home happens in T1555 when the inline expand goes away. Two short-lived copies is cheaper than refactoring a soon-to-be-deleted block. Part of PLAN-1542. * fix: address Codex review on TASK-1551 1. Snapshot user.id at each async handler entry (changeRole, resetPassword, toggleDisable, saveUser) and the email at toggleDisable's entry. If the modal closes/swaps to another user mid-PATCH, the in-flight request now updates the row it was originally targeting rather than whatever user is currently displayed. Mirrors the inline-expand handlers' pattern. 2. Typed-email gate stays trim()-tolerant — paste from various sources often picks up whitespace. The button title and message copy now explicitly say "paste tolerant" so the behavior matches what's documented.	2026-05-20 18:52:20 -04:00
xarmian	02491476e2	feat(web): admin user modal shell with empty tabs (TASK-1550) (#605) ... * feat(web): admin user modal shell with empty tabs (TASK-1550) New component: web/src/lib/components/admin/UserModal.svelte. Shell-only in this PR; tab content arrives across T1551–T1554, and T1555 deletes the parallel inline-expand block. Features: - Backdrop + centered modal at 720px default width (--user-modal-width CSS variable so T1552 can widen for the Workspaces table without forking layout). - Four tabs: Overview / Workspaces / Activity / Settings & overrides. Each tab renders a placeholder telling future readers which TASK fills it. Tabs as <button role="tab"> inside a <div role="tablist">; panels carry aria-labelledby + tabindex="0" so screen readers can navigate them. - ESC closes. Click on backdrop closes (stopPropagation guards the modal body). Tab key is trapped within the modal so focus can't escape into the table behind it. - Tab key restored from window.location.hash (?...#tab=workspaces) so reload preserves the active tab. Tab change writes back via history.replaceState (no back-button history pollution). - Body scroll-locked while open. - Focus pulled to the close button on open; restored to the originating trigger element on close. Wiring in +page.svelte: - Row click now opens the modal AND triggers the existing inline-expand (selectUser). The "double UI" is the explicit broken state from the plan, documented in the row-click comment. T1555 deletes the inline expand once T1551-T1554 have lifted everything into the modal. Part of PLAN-1542. * fix: address Codex review on TASK-1550 1. Focus trap now excludes hidden tabpanels and other off-screen focusables. Previous querySelectorAll picked up every panel's tabindex=0 element including the three off-screen ones, so the "last" reference pointed at the wrong place and Tab could escape the trap on Overview/Workspaces/Activity. 2. writeHashTab + parseHashTab now use URLSearchParams over the raw hash string. Previous regex-replace corrupted hashes where tab= was first but other params followed (#tab=X&foo=1 became &foo=1#tab=Y, moving foo outside the hash entirely). Single source of truth for hash param decoding/encoding. 3. Focus capture only runs on the open=false→true transition. Previous \$effect re-ran whenever initialTab changed and would recapture previousFocus into the modal itself; on close it would then "restore" focus to an element inside the modal that no longer exists. wasOpen latch makes the open/close behavior strictly edge-triggered. ARIA arrow-key navigation between tabs (ArrowLeft/Right/Home/End) acknowledged as a follow-up enhancement; tabs are still individually keyboard-focusable via Tab, which meets the baseline.	2026-05-20 18:43:21 -04:00
xarmian	5990008028	feat(web): admin user list — pagination + sort + filter UI (TASK-1549) (#604) ... * feat(web): admin user list — pagination + sort + filter UI (TASK-1549) Wires up the table-scale UX promised by T1544's API extensions: Filter bar above the table — Role / Plan (cloud_mode only) / Status / Active within / Has workspaces. A "Clear filters" affordance appears when anything is set. Status filter is mostly server-mapped (disabled → disabled=true; no-workspace → has_workspaces=false); "active" and "inactive" don't have a direct API param so they apply as a client-side narrow over the page (applyClientStatusFilter — documented as a known limitation in the pager footer with a "(client-filtered)" caveat). Sortable column headers — Workspaces, Email, Storage, Last Write, Last Active, Created. Click to sort; second click reverses direction. Default direction is "asc" for email, "desc" for time/numeric. Active column shows a ▲/▼ indicator in accent-blue. Pager — "Load more" at table foot appends the next page rather than replacing. Shows "Showing N of M" with a hint about client-filtered narrows. Filter or sort change resets offset=0. URL state — filter and sort sync to the URL via SvelteKit goto with replaceState. hydrateFromURL() on mount restores from a pasted link. Search query is intentionally NOT synced (changes per keystroke). Refactor: loadUsers + searchUsers collapsed into loadList(reset). buildQueryParams centralizes the URLSearchParams build for both the fetch call and the URL sync. searchUsers() now just delegates to onFilterChange so the search input and the filter bar share the same reset/reload semantics. Part of PLAN-1542. Frontend purely additive — no API or backend changes. * fix: address Codex review on TASK-1549 Five real findings, all fixed: 1. statusFilter no longer auto-maps to has_workspaces in the query — that conflated server status precedence (disabled > no-workspace) so a disabled user with 0 workspaces would surface in the "no-workspace" bucket. statusFilter is now always applied client-side via applyClientStatusFilter against the row's server-computed status field. The "disabled" case still passes a server hint (disabled=true) to shrink the result set, but the client filter remains authoritative. 2. statusFilter ↔ hasWorkspacesFilter conflict resolved by point 1. The two controls are now genuinely independent — hasWorkspacesFilter only sets the dedicated has_workspaces param. 3. URL round-trip is now lossless. statusFilter has its own ?status= param rather than being smuggled through has_workspaces/disabled. All four buckets (active/inactive/disabled/no-workspace) serialize and re-hydrate identically. 4. loadMore() bumps offset BEFORE the fetch but rolls back on failure (try/catch + error-flag double-check). Added a re-entrancy guard so rapid clicks while a page is in flight no-op. 5. Sortable headers are now real <button>s inside the <th>, with aria-sort on the th. Keyboard-focusable + Enter/Space activation come from the native button; .sort-btn:focus-visible carries a visible outline.	2026-05-20 18:35:08 -04:00
xarmian	8a85eca713	feat(web): admin user table — cheap aggregation columns (TASK-1548) (#603) ... * feat(web): add cheap aggregation columns to admin user table (TASK-1548) Surfaces the per-user aggregations T1544 added to GET /admin/users: - Workspaces (numeric, after Role) - Storage (used bytes, formatted via the existing formatStorageBytes helper — same units the storage-override field accepts) - Last Write (relative time, color-coded by writeRecency: green <7d, yellow <30d, red ≥30d, gray italic when never) - Status pill (replaces the standalone "disabled" badge; renders for disabled / no-workspace / inactive; suppressed for "active" to keep the table calm) Implementation: - AdminUser interface in admin.svelte.ts gains last_write_at, workspace_count, storage_bytes, status — matching the server-side JSON shape from T1544. - writeRecency() helper in +page.svelte buckets the timestamp into a CSS class. Visual half of the API's status pill; same age windows. - .num-cell utility: right-aligned, tabular-nums so digits line up across rows (workspace_count and storage cells share it). - edit-row colspan updated to 9 (cloud_mode) / 8 (self-hosted) to span the new columns. Frontend purely additive — no API changes, no Go changes. T1549 wires up pagination + sort + filter; T1550-T1555 build the modal. Part of PLAN-1542. * fix: address Codex review on TASK-1548 1. handleAdminGetUser response now includes last_write_at, storage_bytes, and status — matching handleAdminListUsers' shape. Without these, the row-merge after PATCH (role change, disable, plan edit) kept stale values; e.g. disabling an active user wouldn't show the new "disabled" status pill until the full list reloaded. - Store.UserStorageUsage: new helper that sums attachments across all workspaces owned by the user. Mirrors WorkspaceStorageUsage's definition. - Store.ComputeAdminUserStatusValue: exported wrapper around the existing private helper so handler code can compute the pill value without round-tripping through SearchUsers. 2. writeRecency threshold: 30d is now "stale" (inclusive) rather than "cold". Matches server-side computeAdminUserStatus which only flips to "inactive" on > 30 days. Eliminates a 1-day boundary mismatch between the recency color and the status pill. 3. A11y: write-recency cell now carries aria-label with the bucket name ("Last write: 12d ago (stale)"), so screen-reader users get the same meaning the color conveys.	2026-05-20 18:10:25 -04:00
xarmian	48323e229e	feat(admin): GET /admin/users/{id}/metrics windowed engagement metrics (TASK-1547) (#602) ... Final backend task for PLAN-1542. Returns three engagement signals that power the metric tiles on the admin user modal's Overview tab (T1553): - days_since_write: derived from users.last_write_at (T1543). nil when the user has never had a write recorded. - writes_7d: COUNT of write-class activities (created/updated/archived/ restored/moved/commented) authored in the last 7 days. - collections_touched_30d: COUNT(DISTINCT collection_id) of items the user has authored writes for in the last 30 days. Goes through activities.user_id (not items.last_modified_by, which is an attribution string — see T1543's architecture note). api_requests_7d is intentionally NOT included; no per-request log exists. Filed as a follow-up (IDEA-1556) that will add this as an additive, non-breaking field once the request-log table lands. Implementation: - Store.GetUserMetrics in users.go runs three small queries: scalar SELECT for last_write_at, one COUNT(*) over activities, and a JOIN(activities, items) for the DISTINCT collection count. All three are index-backed (idx_activities_user from migration 022). - No caching layer in this PR. The queries are cheap, and a per-user short cache fits more naturally at the handler boundary if needed — premature here. - Handler handleAdminGetUserMetrics wired at GET /admin/users/{userID}/metrics. requireAdmin gate; 404 on missing user. Tests: TestGetUserMetrics seeds a workspace with two collections, six activities (five inside 7d, one ancient outside both windows), verifies all three metrics. TestGetUserMetricsEmptyUser covers the no-activity case (nil days_since_write, zero counts, no error).	2026-05-20 17:13:28 -04:00
xarmian	a04e5217c6	feat(admin): GET /admin/users/{id}/activity paginated feed (TASK-1546) (#601) ... * feat(admin): GET /admin/users/{id}/activity paginated feed (TASK-1546) New endpoint returns activities originated by the user — item writes, comments, account-level actions the user took themselves — in reverse- chronological order with offset pagination. Scope decision: feed shows activities where activities.user_id = userID (events the user authored). Admin actions targeting this user as a subject (role_changed where target_user_id is in metadata) are NOT included; that "received" sub-feed needs a JSON predicate and is filed as a follow-up. T1554 (modal Activity tab) consumes the current shape. Implementation: - Store.ListUserActivity mirrors the existing ListWorkspaceActivity / ListDocumentActivity helpers in activities.go. Same column projection, same LEFT JOIN users u for actor name. Hard cap at limit=50. - Handler asks for limit+1 rows so it can flag "next_offset" without a separate COUNT query — trims the extra before responding. Returns next_offset=null when the page is the last. - Route wired at GET /api/v1/admin/users/{userID}/activity. 404 on missing user; requireAdmin gate. - Pagination is offset-based (matching the sibling endpoints) rather than cursor-based as the task body suggested. For per-user feeds the dataset is bounded and between-page drift is acceptable for an admin tool. Cursor can be added later if needed; the response shape is forward-compatible (next_offset → next_cursor would just rename). Test: TestListUserActivity covers cross-user isolation, action filter, offset pagination across pages without overlap, and the 50-row hard cap. Part of PLAN-1542. * fix: address Codex review on TASK-1546 Lift the store-side cap on ListUserActivity from 50 to 100. The handler caps the public per-page at 50 and asks the store for limit+1 (51) to flag "more available" without a separate COUNT. Previous store-side cap of 50 silently truncated that probe, so next_offset would be null even when row 51 existed — clients iterating at page-size=50 would stop one page short of the actual end. The HTTP layer remains the source of truth for the per-page maximum; the inner cap is now just protection against pathological internal callers. Regression test seeds 51 activities and verifies the store returns all 51 when asked.	2026-05-20 17:07:57 -04:00
xarmian	eff0824238	feat(admin): GET /admin/users/{id}/detail per-workspace breakdown (TASK-1545) (#600) ... * feat(admin): GET /admin/users/{id}/detail per-workspace breakdown (TASK-1545) New endpoint that returns the user vitals plus a per-workspace breakdown enriched with the aggregations the admin user modal's Workspaces tab needs: collections_count (excluding system collections — playbooks, conventions, anything else is_system=1), items_open (status NOT IN a hardcoded terminal set), items_total, members_count, storage_bytes (matches WorkspaceStorageUsage's definition), and last_activity_at (MAX items.updated_at across non-deleted items). Implementation: - Store.GetUserWorkspacesDetailed in workspace_members.go uses correlated subqueries rather than a wide JOIN+GROUP BY — a single user belongs to at most tens of workspaces in practice, so the readability wins over micro-optimizing. Caps at 50 rows (frontend caps at 20 in T1552). - AdminUserWorkspaceDetail embeds the existing AdminUserWorkspace to keep the JSON shape backward-compatible with /workspaces consumers. - adminOpenItemTerminalStatuses is a hardcoded list (done/completed/ rejected/archived/implemented/cancelled). A schema-aware terminal_options check is a separate follow-up — flagged in the struct doc. - Handler handleAdminGetUserDetail wired at GET /admin/users/{userID}/detail. Returns 404 on missing user; defensive []AdminUserWorkspaceDetail{} serialization so JSON consumers see [] rather than null. Test: TestGetUserWorkspacesDetailed seeds a workspace with two user-facing collections, one system collection, three items (two open, one terminal), two members, and one attachment; verifies all six aggregations. Part of PLAN-1542. T1552 (modal Workspaces tab) consumes this. * fix: address Codex review on TASK-1545 Three real issues in the items_open count clause, all in the same SQL fragment: - Use s.dialect.JSONExtractText("i.fields", "status") instead of the SQLite-only JSON_EXTRACT(i.fields, '$.status'). The endpoint would have failed on Postgres deployments. - Wrap the extracted value in LOWER(COALESCE(..., '')) so items with NULL/missing status fields still register as "open" (NULL NOT IN (...) is not TRUE in SQL, which would have undercounted), and so case-variant statuses match. Matches the interpretation used in search.go and items.go. - Source the terminal list from models.DefaultTerminalStatuses rather than a private list — previous local list was missing 'resolved', 'wontfix', 'fixed', 'disabled', 'deprecated' (overcounting open items in workspaces that use those statuses). adminOpenItemsCountClause is now a Store method (was a package-level fn) because it needs the dialect.	2026-05-20 17:00:57 -04:00
xarmian	0c5ec04fac	feat(admin): extend user list with aggregations + sort/filter (TASK-1544) (#599) ... * feat(admin): extend user list with aggregations + sort/filter (TASK-1544) GET /admin/users now returns per-user workspace_count, storage_bytes, last_write_at, and a computed status pill (disabled / no-workspace / inactive / active, with documented precedence). Adds sort and filter knobs so the table can scale beyond the existing fixed offset/limit. Store layer: - AdminUserSearchParams gains Role, Sort, Order, ActiveWithinDays, HasWorkspaces, Disabled. Pointer types where tri-state ("no filter" vs. "filter to false") matters. - AdminUserListEntry wraps models.User with WorkspaceCount, StorageBytes, Status — returned in AdminUserSearchResult.Users. - SearchUsers SQL rewritten: LEFT JOIN against grouped subqueries so one user owning N workspaces with M attachments each still produces exactly one row (no aggregation explosion). Both subqueries filter deleted_at IS NULL to match WorkspaceStorageUsage's existing definition. Allow-listed sort clause prevents injection. - computeAdminUserStatus exported for unit tests; precedence locked in by TestComputeAdminUserStatus. - TestSearchUsersAggregations covers workspace_count + storage_bytes + status across a three-user fixture and each new filter/sort knob. Model + scanner: - models.User gains LastWriteAt. userColumns + scanUser updated; the legacy callers (GetUser, ListUsers, etc.) inherit the new field for free via the shared scanner. Handler: - handleAdminListUsers accepts the new params: role, disabled, has_workspaces, active_within_days, sort, order. Tri-state bools only fire when the query param is present. Response now embeds workspace_count / storage_bytes / last_write_at / status. Part of PLAN-1542. Frontend consumption lands in T1548 (cheap columns) and T1549 (sort/filter UI). * fix: address Codex review on TASK-1544 - Tri-state bool parsing in handler now uses strconv.ParseBool — accepts the canonical truthy/falsy variants ("True"/"TRUE"/"t"/"1" and the parallel falses), and silently ignores garbage values rather than treating them as false. Closes the "disabled=TRUE silently means enabled-only" surprise. - SearchUsers count query no longer joins the storage aggregation when HasWorkspaces isn't an active filter. The page query still needs both joins (the row carries the data), but a typical "give me a count" call no longer scans every live attachment. The workspace_count join remains conditional on HasWorkspaces filtering. Status threshold (>30d vs >=30d): the documented spec and impl both say ">30d" — no change.	2026-05-20 16:50:16 -04:00
xarmian	0a09c1dca7	feat(store): add users.last_write_at column + write-path hook (TASK-1543) (#598) ... * feat(store): add users.last_write_at column + write-path hook (TASK-1543) Engagement metrics need a "last write" signal distinct from last_active_at (which is bumped on any authenticated request, so it includes reads). Adds: - Migration 060: users.last_write_at + index. Backfills from activities table (the canonical record of who-did-what) using the action set that handlers_items.go / handlers_comments.go actually emit: created/updated/archived/restored/moved/commented. - Store.TouchUserWrite(ctx, userID): mirrors TouchUserActivity. Same 5-minute throttle to avoid write-amplification, silent no-op on empty userID so callers don't have to guard. - Hook in logActivityWithMetaReturningID (handlers_documents.go) — every item-write action funnels through this single helper, so one TouchUserWrite call covers item create/update/archive/restore/move and comment authoring. - Explicit hook in handlers_attachments.go after CreateAttachment, since uploads don't go through logActivity. Test: TestTouchUserWrite covers empty-userID no-op, first-write set, in-throttle suppression, and out-of-throttle advance. Architecture note: items.last_modified_by / comments.created_by are attribution strings ("user"/"agent"/"cli"), not user IDs. The user identity lives in activities.user_id, populated at the handler layer where currentUser is in scope. That's why the hook lives in handlers, not the store layer — and why the backfill reads from activities. Part of PLAN-1542 (admin user management enhancements). * fix: address Codex review on TASK-1543 - Add Postgres migration 039 (pgmigrations counterpart to 060). Same ALTER + index + activities-backfill, using TEXT to match the existing last_active_at / disabled_at column types in pgmigrations/020-021. Without this, Postgres deployments silently no-op TouchUserWrite because the column doesn't exist (and the call's UPDATE error is swallowed by design). - Hook TouchUserWrite in handleCreateCommentReply. The reply handler doesn't go through logActivity (no "commented" activity emitted for replies — verified by grep), so the activity-helper hook misses it. Explicit call after a successful CreateComment.	2026-05-20 16:39:23 -04:00
xarmian	8fb46ca1b5	fix(web): surface open_children 409 + offer force override (BUG-1538) (#597) ... * fix(web): surface open_children 409 + offer force override on status changes (BUG-1538) The server's open-children guard (IDEA-1494) returns a structured 409 when transitioning a parent to a terminal status while it still has non-terminal children. The web UI was catching this generically and toasting "Failed to update status" — users couldn't tell why the change was rejected or that --force exists. Now the API client preserves the structured `details` payload, and a singleton OpenChildrenDialog (mounted at +layout.svelte) lists the blocking children as links to their detail pages, surfaces hidden_blocker_count, and offers an "Override and mark <value>" button that retries the PATCH with force=true — same semantics as the CLI's --force flag. Wired into the two PATCH sites that change the done-field: the collection page's handleStatusChange (covers Board drag-drop + inline status changes) and the detail page's updateField (FieldEditor on the item detail page). TASK-1539. * fix(web): address self+codex review (round 1) - Item moves (POST /move) also hit the open-children guard server-side. Wire the same modal + force-override path through api.items.move() and handleMove() on the detail page (Codex finding 1). - OpenChildrenDialog: add a focus trap so Tab / Shift-Tab cycle within the modal, and restore focus to the previously-focused element on close (Codex finding 2 + self-review a11y note). - Simplify the nested try/catch in handleStatusChange / updateField: branch on isOpenChildrenError early so cancelling the modal stops emitting console.error noise and the retry-failure path stays distinct from the original-guard path (self-review nits 2 + 3). - updateField: assign item = { ...item } on cancel to force a fresh prop pass to FieldEditor in case it caches by identity. BUG-1538 / TASK-1539. * fix(web): capture full route identity in handleMove pre-modal (Codex round 2) Captures sourceItem/sourceWs/sourceUsername at move kickoff so a confirmed force-retry after navigation can't move the wrong item. Adds navIfStillCurrent helper that gates the success-path goto on identity match — stale resolutions complete silently rather than yanking the user away from the new page. BUG-1538. * fix(web): also gate handleMove navigation on route params (Codex round 3) Compare page.params.{collection,slug} in addition to item.id and workspace — during same-component navigation `item` can briefly still hold the source object after the URL has advanced. Route- param check closes that race. BUG-1538. v0.5.0	2026-05-19 20:54:55 -04:00
xarmian	a3799a19a9	feat(cli): add --sort-order flag to pad item update (BUG-1536) (#596) ... The only way to set items.sort_order from the CLI was --field sort_order=N, which silently writes into the per-collection fields JSON blob (dead data) instead of the top-level column the parent view's ORDER BY reads. Add a first-class --sort-order int flag so agents discover the proper path via pad item update --help. The --field route is intentionally left alone — collection-schema fields and top-level item columns share a namespace by accident, and silently rerouting one key without the others would be more surprising than the current behavior.	2026-05-19 17:28:31 -04:00
xarmian	db87b47754	fix(cli): pin server URL in .pad.toml for remote workspaces (BUG-1535) (#595) ... * fix(cli): pin server URL in .pad.toml for remote workspaces (BUG-1535) Two fixes: 1. Replace stale api.getpad.dev references with app.getpad.dev in the --url flag help, NewClientFromURL doc, and Config.URL doc. Also fix internal/mcp/dispatch_http.go's comment to use the canonical mcp.getpad.dev/mcp URL. 2. Persist the server URL into .pad.toml when linking a directory to a non-local workspace. WriteWorkspaceLink now takes a serverURL arg; pad init / workspace link / workspace switch pass cfg.BaseURL() when Mode != local. getConfig() reads .pad.toml's URL as an override above ~/.pad/config.toml and below the --url flag, so commands like `pad collection list` from a remote-linked directory hit the right server without --url on every call. Passing --url explicitly also promotes local → remote so the directory pin is written even when the existing global config has mode=local. * fix(cli): scope .pad.toml URL override to client paths per Codex review (round 1) Round 1 review flagged that applying the .pad.toml URL override inside getConfig() contaminates server/admin commands: pad server start would advertise the wrong PublicLinkBaseURL, and pad auth setup would refuse to run locally because Mode flipped to remote. Extract the override into applyPadTomlOverride() and call it only from client-API entry points — getConfiguredConfig() and the pad init client phase. Server/admin commands (pad server start/stop, pad auth setup, pad auth configure) keep using raw getConfig() and are unaffected. Also skip the override when --url was explicitly passed (LoadedFromFlags), so the flag retains unambiguous priority. * fix(cli): preserve .pad.toml URL on workspace link/switch per Codex review (round 2) Round 2 review noted workspace link / workspace switch reached the server via getClient() (override applied) but then wrote the new .pad.toml URL using a raw getConfig() — which would drop or miswrite the url field when relinking inside a remote-pinned directory whose global config is local. Reuse the cfg returned by getClient() for padTomlURLFor so the write matches the API client.	2026-05-19 17:02:46 -04:00
xarmian	e27f805ffc	feat(web): unify dashboard onboarding banners around needs_onboarding signal (TASK-1530) (#594) ... IDEA-1516 Phase 3. The pre-IDEA-1516 design split workspace onboarding guidance across two banners — OnboardingIdeaBanner (gated on the retired IDEA-1 / BACK-1 / FEAT-1 seed-item pattern from PLAN-1496) and OnboardingChecklist (gated on a totalItems === 0 heuristic that predates the canonical needs_onboarding flag from TASK-1504). Both fired competing CTAs on the same screen; neither read the canonical signal. Backend (internal/server/handlers_dashboard.go): - Add `NeedsOnboarding bool json:"needs_onboarding"` to DashboardResponse, populated via the existing Store.WorkspaceHasUserCreatedItems EXISTS query (same predicate AgentBootstrap.NeedsOnboarding uses). Web reads it from the dashboard fetch the page already does — no second round-trip against the heavier bootstrap endpoint. Frontend: - Add `needs_onboarding: boolean` to TS DashboardResponse type - Delete OnboardingIdeaBanner.svelte entirely (signal retired, no remaining consumers); the back-end onboarding_seed field stays for now per spec — separate cleanup - Delete OnboardingChecklist.svelte; replace with OnboardingNudgeBanner.svelte — single message + "Connect agent →" CTA that opens the workspace's already-mounted ConnectWorkspaceModal. Dismissible, preserves the existing `pad-onboarding-dismissed-{wsSlug}` localStorage key so users who dismissed the old checklist don't get re-prompted - Workspace +page.svelte: collapse the two banner blocks into one gated on `needsOnboarding && !onboardingDismissed`; reshow button follows the same signal. Drop the now-orphaned `.connect-card` CSS — its function is subsumed by the banner's CTA. Drop the unused OnboardingIdeaBanner / OnboardingChecklist imports and the `onboardingSeed` derived state Smart-suppression deferred to a follow-up. The existing api.workspaces.claimCode endpoint returns suppression info but generates a real claim code as a side effect in the not-suppressed case — calling it on every workspace page-load with needs_onboarding=true is awkward. The CTA still opens the modal, which renders its own suppression state correctly; users get the right experience with one extra click on the rare suppressed case. A dedicated read-only GET /workspaces/{ws}/connect-status endpoint is a separate piece of work.	2026-05-19 13:17:57 -04:00
xarmian	b969dd7557	feat(web): consolidate workspace-create surfaces — redirect /console/new to modal (#593) ... * feat(web): consolidate workspace-create surfaces — redirect /console/new to modal (TASK-1529) IDEA-1516 Phase 2. The modal is the single create-workspace surface now (Phase 1 redesign already shipped in TASK-1528/1526); the /console/new dedicated page from TASK-1506 / PR #579 becomes dead duplication. - Replace /console/new/+page.svelte with a +page.ts that `throw redirect(307, '/console?openCreate=1')`. 307 preserves request-method semantics; in SPA mode (adapter-static + fallback index.html) the load function runs in-browser and SvelteKit handles the redirect as a client-side navigation, so direct visits and bookmarks both end up at /console with the create modal opened. - /console/+page.svelte onMount reads the ?openCreate=1 query param, fires `uiStore.openCreateWorkspace()`, then scrubs the param via goto({ replaceState: true, noScroll: true, keepFocus: true }) so a refresh doesn't re-open the modal. - Replace the two `<a href="/console/new">` links (header CTA + empty-state) with `<button onclick={openCreateWorkspace}>` — direct callers skip the route round-trip entirely. Added the necessary button resets (border:none / cursor:pointer / font:inherit) to the existing classes so visual rendering is identical. - Verified no other refs to /console/new in web/src. * fix(web): mount CreateWorkspaceModal on console pages per Codex review (round 1) Codex flagged that uiStore.openCreateWorkspace() from /console's new CTA buttons (+ /console/new redirect) sets state with no observer — the modal lives in the non-console branch of +layout.svelte and was never mounted on console routes. Split the isConsolePage case into its own branch that renders children + CreateWorkspaceModal + ToastContainer (toast surface needed for create-failure feedback).	2026-05-19 12:56:20 -04:00
xarmian	f8af6cb889	feat(web): blank-as-default workspace modal + auto-open Connect modal post-create (TASK-1528, TASK-1526) (#592) ... Combines IDEA-1516 Phase 1 (CreateWorkspaceModal redesign) with PLAN-1519 Phase F (Connect-modal auto-open wiring) — Phase 1's callback API and Phase F's consumer are the same integration surface, so they ship together to stay PR-sized per CONVE-2. Phase 1 — CreateWorkspaceModal redesign (TASK-1528): - Default selection flips from `startup` to `blank` - New primary "Start blank" card per IDEA-1516 §2 (recommended path, agent-driven onboarding) - Templates section collapsed by default; expanding pre-selects `startup` to match pre-redesign behavior for users who explicitly want a template - Existing grouped categories preserved inside the section, with Blank filtered out (it's the primary card now) - Footnote on the expanded section pointing at `/pad onboard` - New optional `onWorkspaceCreated` callback prop fired after successful create OR import, before close+goto Phase F — auto-open wiring (TASK-1526): - New `uiStore.requestConnectAfterNavigate(slug)` + matching single-shot `consumeConnectAfterNavigate()` getter — mirrors the existing `requestQuickAdd` / `clearQuickAddRequest` pattern in the same store - `+layout.svelte` wires the modal's `onWorkspaceCreated` callback to `uiStore.requestConnectAfterNavigate(ws.slug)` - Workspace `+page.svelte` consumes the signal in a dedicated effect (kept separate per CONVE-606) when its slug matches, opening the already-mounted ConnectWorkspaceModal - Import flow gets the same treatment — claim-code value is independent of how the workspace got created No backend changes; Phase E (TASK-1525) already shipped the claim-code generation + smart suppression that the auto-opened modal renders.	2026-05-19 08:00:48 -04:00
xarmian	8041b46e36	fix(sse): surface write errors and link keepalive to IdleTimeout (BUG-1532) (#590) ... Two SSE-handler tidy-ups flagged during the BUG-1531 investigation. 1. writeSSEEvent now returns the underlying fmt.Fprintf error. Previously every event/keepalive write swallowed any error from the response writer — when the client TCP went away the handler kept looping, pulling events off the bus channel, and discarding them while waiting for the ctx.Done() cancellation to propagate. Now any write failure exits the handler immediately so the bus subscription is released and we stop fanning broadcast traffic into a dead socket. Marshal errors stay local (don't tear down a healthy stream over one un-marshalable payload). All five callsites + the keepalive Fprintf are updated to log at DEBUG (broken-pipe on client disconnect is normal traffic, not an error worth WARN-level noise) and return. 2. The 30s keepalive interval and the 120s IdleTimeout now live in named constants (sseKeepaliveInterval, httpIdleTimeout) with an init() guard that panics if `3 × keepalive >= IdleTimeout`. Used to be magic numbers in two files; bumping one without the other in lockstep silently created a window where idle SSE streams would get TCP-reset by the http.Server's idle deadline. The init guard fires at process start so a misconfigured constant is visible immediately, not three months from now when someone notices intermittent reconnect storms. Three new tests pin the contracts: - TestWriteSSEEvent_SurfacesWriteErrors - TestWriteSSEEvent_MarshalErrorIsLocal - TestSSEKeepaliveIdleTimeoutInvariant Closes BUG-1532. Full ./internal/server suite passes (~78s).	2026-05-18 18:29:28 -04:00
xarmian	b801867053	chore(web): npm audit fix — svelte 5.55.8, devalue 5.8.1, mermaid 11.15.0 (#589) ... * chore(web): npm audit fix — patch-bump svelte, devalue, mermaid Resolves three advisories surfaced by the Web CI job (and visible on recent main commits before this): - svelte: 5.55.5 → 5.55.8 (moderate × 4: SSR XSS via spread, hydratable Promise XSS, DOM clobbering, ReDoS in <svelte:element>) - devalue: 5.6.x → 5.8.1 (high: DoS via sparse array deser) - mermaid: 11.14 → 11.15.0 (moderate × 4: Gantt infinite-loop DoS, classDef CSS/HTML injection, config CSS injection) All three are in-range patch bumps — package.json untouched, only the lockfile changes. No major bumps, no API churn. Tiptap deliberately untouched: @tiptap/core, @tiptap/extension- collaboration, @tiptap/y-tiptap stay at 3.22.5 — the CLAUDE.md lockstep rule (coordinated bumps + Y.Doc schema-version bump) only applies when those three move together, and nothing here does. Verified: - npm audit: 0 vulnerabilities - npm run check: 0 errors (existing 6 warnings unchanged) - make build + make install: clean, server boots and serves the new bundle. * test(oauth): TestConsent_ApproveWithSpecificWorkspaces locates connection by shape The test asserts the persisted oauth_connections row from a specific- workspaces consent (allowed_workspaces=[alpha,beta]) has AllCurrentWorkspaces=false and the right slug list. It indexed via conns[0], which only worked when the approve flow was the most-recent connection — but the test also calls runAuthCodeFlow above the assertion to mint a bearer for the introspect call, and that helper posts allowed_workspaces=["*"] → wildcard connection. With ListUserOAuthConnections ordered ConnectedAt DESC, conns[0] is now the wildcard bearer row, not the alpha+beta row this test is verifying. We can't move the bearer-mint below the assertion (the introspect call needs the bearer first), and the auth-code response doesn't surface the request_id we'd need to look up the right connection directly. The specific-workspaces flow is the only one in this test with AllCurrentWorkspaces=false, so locate by that shape — surgical fix that matches the test's actual intent. Verified with `go test -count=3 -run TestConsent_ApproveWithSpecificWorkspaces` (deflakes across orderings) and the full ./internal/server suite.	2026-05-18 17:17:42 -04:00
xarmian	bfce069f28	fix(web,build): search palette hang on numeric query + graceful SSE shutdown (BUG-1531) (#588) ... * fix(web,build): search palette hang on numeric query + graceful SSE shutdown (BUG-1531) CommandPalette's reactive `$effect` subscribed to every workspace's `localSearch.epoch` + `localIndex.bootstrapStateFor`. Bare-digit queries short-circuit to `exactItemNumberLookup` (synchronous, very fast) and stacked re-fires of the effect inside one microtask tick whenever an SSE delta arrived — Svelte tripped `effect_update_depth_exceeded` and the palette froze. Treat bare-digit queries the same as `body:` queries (skip the subscription reads) and wrap `doSearch()` in `untrack()` so its internal reactive reads can't smuggle hidden dependencies into the effect. The SSE churn that fanned the loop was rooted in `make install` using `killall -9` — SIGKILL drops every open SSE stream mid-chunk so every browser tab logs `ERR_INCOMPLETE_CHUNKED_ENCODING` and reconnects. Switch to SIGTERM + 5s wait + SIGKILL fallback so the server's existing graceful-shutdown path (cmd/pad/main.go:811-857) actually runs and the http.Server writes a final 0-chunk on each open stream. Follow-up tidy-ups (unchecked write errors in writeSSEEvent, link the 30s keepalive to the 120s IdleTimeout in code) tracked in BUG-1532. * fix(web): track workspace slug in palette $effect per Codex review (round 1) After wrapping doSearch() in untrack(), the workspaceStore.current?.slug read that doSearch performs at line 209 no longer registered as a tracked dep of the search effect. The non-body / non-bare-digit branch still reads the slug via localIndex.bootstrapStateFor(...), so workspace switches re-fire the effect for that branch — but body: and bare-digit queries skip that block entirely. Without an explicit slug subscription they wouldn't re-dispatch on workspace switch; an in-flight server response would land stale, get discarded by isSameDispatch(), and loading could stick true. Hoist `void workspaceStore.current?.slug` into the unconditional void block so all four query shapes re-fire on workspace switch. Refs BUG-1531. * chore: gofmt handlers_claim_code_test.go Drive-by formatting fix to unblock CI on this PR. The file landed slightly unaligned in #586 (TASK-1525) — gofmt straightens the struct tag column on claimCodeResponse.	2026-05-18 15:25:27 -04:00
xarmian	ab5b68c7c3	fix(connect): correct self-host copy in claim-code disabled state (TASK-1525 follow-up) (#587) ... The `claim_disabled` path only fires on self-host deployments without PAD_MCP_PUBLIC_URL — the OAuth server (and with it the claim secret and the OAuth-grant model itself) only mounts under that env var. In that configuration: - Agents authenticate as the user via session tokens from ~/.pad/credentials.json — stdio MCP (`pad mcp serve`) and the CLI both inherit it. - The agent sees every workspace the user is a member of by default. There is no per-workspace OAuth grant to claim into. - /console/connected-apps is empty by definition (it lists OAuth grants). So the prior copy ("Use the MCP tab to authorize an agent from scratch") was misdirection on two counts: 1. The MCP tab is filtered out entirely when mcpPublicUrl is empty (visibleTabs). The button it pointed at didn't exist. 2. There's no further "setup" required — the agent already has access. Two fixes: - Rewrite the disabled-state copy to tell self-host users they're already done: "No claim code needed on this deployment. Agents connected via the CLI or stdio MCP use your user session and already have access to every workspace you're a member of." - Hide the "Connected agents →" footer link when mcpPublicUrl is empty so we don't point self-host users at an empty page. The Open Connected apps link inside the suppression-state panel stays — suppression only fires when an active OAuth grant covers the workspace, which by definition only happens on cloud / remote- MCP-enabled deployments.	2026-05-18 10:49:19 -04:00
xarmian	fc6afd01be	feat(connect): unified Connect-to-agent modal + claim-code endpoint (TASK-1525) (#586) ... * feat(connect): unified Connect-to-agent modal + claim-code endpoint (TASK-1525) Phase E of PLAN-1519. Repurposes the avatar-menu "Connect a project…" modal as a one-stop hub where users can connect ANY agent surface (claim-code → existing OAuth grant, fresh MCP OAuth, or local CLI) to the current workspace. Backend - GET /api/v1/workspaces/{slug}/claim-code — generates a stateless 6-digit HMAC claim code (re-uses the verifier's secret + bucket math) for the calling member, OR reports `suppressed: true` when smart-suppression detects the workspace is already covered by one of the user's active OAuth connections (wildcard OR explicit allow-list rows). Returns `expires_at` at the current bucket boundary so the UI can drive a countdown. - store.IsWorkspaceCoveredForUser — single indexed query against oauth_connections + oauth_*_tokens; filters by ACTIVE tokens so a dangling revoked connection row doesn't suppress fresh modals. - Tests cover 412 (disabled), 404 (non-member), 200 + matching code, wildcard suppression, explicit-allow-list suppression, and the revoked-connection-doesn't-suppress invariant. Frontend - ConnectWorkspaceModal rewritten as a tabbed unified modal: - Agent (claim code) — fetches on activate, live countdown, auto-refetches at bucket roll-over, renders smart-suppression panel that links to /console/connected-apps, and renders the locked prompt block `Authorize the pad workspace '<slug>' with claim code <code>.` per IDEA-1517 §4. - MCP setup — subsumed from the now-deleted ConnectMCPModal: URL block + client-card grid linking to per-client docs. - CLI — existing install + `pad init` flow, unchanged. - Default tab: Agent when mcpPublicUrl is set; CLI when not. MCP tab hidden entirely on self-host without a public MCP URL. - ConnectBanner simplified: single modal state, generic "Connect an AI agent to this workspace" copy, no MCP/CLI dual-modal branching. - ConnectMCPModal.svelte deleted (fully subsumed). - API client gets `workspaces.claimCode(slug)` + `ClaimCodeResponse` TypeScript type. - TopBar + workspace home callsites pass `mcpPublicUrl` from authStore so the unified modal can pick the right default tab. Verification - go build ./... clean - go test ./... — all packages green (server + store) - cd web && npm run build — clean Parent: PLAN-1519. Phases A-D already shipped (oauth_connections schema, MCP claim action, /authorize redesign, connections-page mutation UI); this lands Phase E. Phase F (TASK-1526) will wire post-create auto-open from IDEA-1516's new-workspace modal; Phase G (TASK-1527) is cross-agent paste validation of the locked prompt string. * fix(connect): require membership at claim-code generation; guard modal against stale-response races per Codex review (round 1) 1. Guest-grant generation gap. RequireWorkspaceAccess admits item-grant guests who aren't workspace members; claim-code REDEMPTION requires full membership. Generating without the same check handed guests a valid-looking code + prompt that the claim endpoint always 404s. Add an explicit GetWorkspaceMember check after getWorkspace and return 403 not_a_member to fail closed on the same response shape the redemption path would have used. 2. Stale-response race in the modal. ConnectWorkspaceModal stays mounted across workspace switches (TopBar reuses the same instance), so an older claimCode fetch can resolve AFTER a newer one and stomp claimState with suppression or a code for the wrong workspace. Add a monotonic seq + captured-slug guard mirroring the refreshHasAgentActivity pattern already in ConnectBanner. Test additions: - TestHandleWorkspaceClaimCode_GrantOnlyGuest_403 asserts a non-member authenticated caller never gets a 200 + code from the generation endpoint.	2026-05-18 10:25:08 -04:00
xarmian	26aa800b3b	feat(oauth): connected-apps mutation endpoints + edit UI (TASK-1524) (#585) ... * feat(oauth): connected-apps mutation endpoints + edit UI (TASK-1524) Phase D for PLAN-1519. Adds four mutation endpoints under /api/v1/connected-apps/{id}/... and extends the console page with an inline Edit panel per connection card. Backend (internal/server/handlers_connected_apps.go + server.go) - PATCH .../name — rename, trim + cap at 120 chars - PATCH .../flags — atomic set of may_create / all_current / include_future (rejects toggling all_current=false when the join table is empty — empty-allow-list invariant from IDEA-1517 §3 Acceptance) - POST .../workspaces — add workspace; membership-checked, 404 uniform when the user isn't a member (no enumeration leak) - DELETE .../workspaces/{slug} — remove; idempotent for missing slugs; rejects last-workspace removal when all_current=false (same orphan guard as the flags handler) All four route through requireConnectionOwner which returns the same 404 envelope for non-owned connections as the existing Revoke endpoint. Each handler echoes the updated DTO so the page can re-render in place; respondWithConnection handles both active-token chains (via ListUserOAuthConnections) and connections without token rows (direct fetch of oauth_connections + the access projection). DTO (connectedAppDTO) gains name + the three scope flags. Model already carried the fields (TASK-1522). Frontend (web/...) - ConnectedApp TS type extended; api.connectedApps gains rename/updateFlags/addWorkspace/removeWorkspace methods. - Connections page: Edit button per card opens an inline panel with: connection name input (debounced save), three scope-flag toggles (auto-save), allow-list chips with X-to-remove plus a workspace picker that lists memberships not already in the list. - Last-workspace removal disabled at the UI level (chip-remove disabled when list length <= 1); API enforces the same invariant if a tampered call slips through. - Workspaces fetched lazily on first Edit open (cached for the page lifetime). Tests - 7 handler tests cover happy paths + edge cases: - rename trims/caps, non-owner 404 - flags happy path + empty-allow-list block - add workspace happy path + non-member 404 - remove workspace happy path + last-blocked + idempotent missing - loginTestUserAs helper to seed a second user for the non-owner case (the existing loginTestUser hardcodes a single email). Parent: PLAN-1519. * fix(oauth): wildcard→specific toggle now works after pre-stage per Codex review (round 1) PR #585 round 1 caught that switching all_current_workspaces from true to false was effectively impossible: the flags handler's empty-allow-list guard called GetOAuthConnectionAccess, which intentionally short-circuits on wildcard and reports zero slugs. Even with join rows present, the guard always tripped. The UI compounded the issue by hiding the allow-list editor while in wildcard mode, so users had no way to pre-stage workspaces. Backend fix: new Store.ConnectionWorkspaceCount(requestID) returns the raw join-row count regardless of the parent's wildcard flag. The flags handler now uses this; the remove-workspace handler's "orphan guard" also routes through the new count (plus an IsConnectionWorkspaceAllowed probe so a no-op removal of a slug that isn't even in the list doesn't trip the guard). Frontend fix: the allow-list editor renders unconditionally inside the Edit panel. While wildcard is on, an "Inert while wildcard is on" badge clarifies that staged workspaces don't take effect until the user flips the flag. Chip-remove disabled only when actively in specific mode AND about to drop to zero — wildcard-mode removes are always allowed. Added TestHandleUpdateConnectedAppFlags_WildcardToSpecific_AfterPrestage as the regression guard: seeds a wildcard connection, adds one workspace via the API, then asserts the flag flip succeeds and the resulting DTO carries the pre-staged slug. Parent: PLAN-1519. * fix(oauth): staged-while-wildcard rows now visible per Codex review (round 2) PR #585 round 2 caught that the round-1 fix (always-rendered allow-list editor + Backend ConnectionWorkspaceCount) was incomplete: pre-staging a workspace while wildcard=true succeeded on the server but the response DTO still suppressed the staged slugs (ListUserOAuthConnections + respondWithConnection both set AllowedWorkspaces=nil when AllCurrentWorkspaces=true). The UI rendered "No workspaces staged" even after a successful add, so users had no way to see or remove a mistaken staged row. Backend fix: new Store.ListConnectionWorkspaceSlugs returns the join table's slugs regardless of the wildcard flag. ListUserOAuthConnections + respondWithConnection both route through it now. The hot-path GetOAuthConnectionAccess still short-circuits on wildcard (correct for the introspection path — when wildcard is on, slugs are irrelevant for gating); the read- for-display path needs to surface them. Frontend fix: isAnyWorkspace now reads the all_current_workspaces flag directly (drives the "Any workspace" badge), independent of the slug list. The slug list drives the edit panel chips. Legacy fallback for missing-flag wire shapes preserved. Added TestHandleAddConnectedAppWorkspace_VisibleUnderWildcard as the regression guard: seeds a wildcard connection, adds a workspace, asserts the slug appears in DTO.AllowedWorkspaces while AllCurrentWorkspaces stays true. Parent: PLAN-1519.	2026-05-18 08:33:39 -04:00
xarmian	93b9590bf9	feat(oauth): consent screen rewrite + new-tables write path (TASK-1523) (#584) ... Phase C2 for PLAN-1519. Rewrites the /oauth/authorize consent flow per IDEA-1517 §2a: collects a display name, three scope flags (may_create_workspaces, all_current_workspaces, include_future_workspaces), and the per-workspace allow-list — then writes oauth_connections + oauth_connection_workspaces instead of session.Extra. Backend (internal/server/handlers_oauth.go) - parseConsentPayload returns a structured consentDecision carrying name, three flags, and resolved workspace IDs. Two-radio + checkbox UI maps to flags per §2a's table; backward-compat shim still accepts the legacy allowed_workspaces=["*"] / explicit-list shape so pre-TASK-1523 clients and fixtures keep working. - handleOAuthAuthorizeDecide INSERTs oauth_connections (parent) + oauth_connection_workspaces (children) BEFORE NewAuthorizeResponse. Ordering matters: an INSERT failure prevents code minting, so the failure mode is "user retries" rather than "token issued with no connection-level state" (which the dual-read gate would silently broaden to no allow-list). A successful INSERT followed by NewAuthorizeResponse failure leaves an orphan oauth_connections row — harmless, no token references it. Per-slug INSERT failures roll back via DeleteOAuthConnection (FK CASCADE clears the join). - session.SetAllowedWorkspaces call retired; the dual-read introspection gate from Phase A still consults legacy session.Extra for pre-TASK-1523 tokens during the soak period. - /oauth/authorize accepts ?suggested_name= URL param, trimmed + 120-char-capped, threaded through to the template for prefill. Frontend (consentTmpl) - Per §2a mockup: identity header, name input (with prefill), data-permissions block (capability_tier radios — unchanged from pre-TASK-1523), workspace-access radio ("All my workspaces" default vs "Only specific workspaces"), conditional picker that slides in for the specific mode, may_create_workspaces checkbox (default-on), footer disclosure, and Authorize/Deny buttons. - Picker shows a search input when the user has >10 workspaces; per-row role label. - Mobile layout (max-width: 480px) stacks the actions vertically with Authorize above Deny. - "Pick at least one workspace" helper text appears when the user is in specific mode with zero workspaces selected. - Empty-workspace case still renders the "create or join one first" empty state; server validates regardless of JS. Tests - 2 new tests: TestConsent_ApproveWithNewShape (end-to-end with connection_name + workspace_access + may_create_workspaces + allowed_workspaces — asserts the values land on the oauth_connections row and join table); TestConsent_SuggestedNamePrefill (?suggested_name= URL param prefills the form input). - Existing approve-flow tests updated: the introspection response no longer carries allowed_workspaces (we stopped writing session.Extra), so assertions now read the connection row + join table via ListUserOAuthConnections — same guarantee, new shape. TestOAuth_Authorize_RendersConsentWhenLoggedIn seeds a workspace so the consent form renders its allow-list section (the new template hides it when the user has zero memberships). Parent: PLAN-1519.	2026-05-18 07:51:12 -04:00
xarmian	905baaa010	feat(oauth): backfill session.Extra into oauth_connections + switch read path (TASK-1522) (#583) ... * feat(oauth): backfill session.Extra into oauth_connections + switch read path (TASK-1522) Phase C1 for PLAN-1519. Seeds existing OAuth grant chains into the new connection tables (Phase A) and switches /console/connected-apps to read from them, retiring the session.Extra parse on the read path. Backfill (internal/store/oauth_connections_backfill.go) - Walks oauth_access_tokens + oauth_refresh_tokens to find every distinct request_id chain (including refresh-only chains). - Picks the newest token row per chain — its session.Extra drives the seeded shape, so a chain whose user re-scoped recently reflects the latest decision. - Maps session.Extra shapes to the new tables per IDEA-1517 §2: no key → all_current=1; ["*"] → all_current=1; explicit slugs → all_current=0 + one join row per slug (added_by='user'). - Resolves slugs → workspace IDs; unresolved slugs (deleted / renamed workspace) are counted + logged at WARN, not fatal. - Idempotent on every INSERT (OR IGNORE / ON CONFLICT DO NOTHING) so re-running on every startup is a cheap no-op once stable. - Returns a BackfillOAuthConnectionsResult so the startup log reports chains_seen / connections_created / workspaces_added / unresolved_slugs — operators see fresh work and notice drift. Read-path rewrite (internal/store/connected_apps.go) - ListUserOAuthConnections projects AllowedWorkspaces from GetOAuthConnectionAccess (oauth_connection_workspaces JOIN workspaces) instead of parsing session.Extra strings. - Hydrates Name + MayCreate + AllCurrent + IncludeFuture from oauth_connections so Phase D's mutation UI has them. - Defensive fallback for chains without an oauth_connections row (any leftover the backfill missed): treats as legacy "any workspace, default-on flags" so the connection still renders. Backfill at startup keeps this branch unreachable in production. - Retires parseAllowedWorkspacesFromSession; the new extractAllowedWorkspacesFromSessionExtra helper in oauth_connections_backfill.go is the only consumer of the session.Extra shape on the store side. Model (internal/models/connected_apps.go) - Adds Name / MayCreateWorkspaces / AllCurrentWorkspaces / IncludeFutureWorkspaces. AllowedWorkspaces semantics stay stable (nil = "any"; explicit slugs = chip list) so the existing DTO + frontend continue working unchanged. Phase D exposes the new fields on the wire. Startup wiring (cmd/pad/main.go) - After srv.SetOAuthServer / SetClaimSecret, run the backfill once. Non-fatal on error (partial state is consistent and the next run completes). Quiet at the Debug level on steady-state re-runs; INFO when fresh work landed. Tests - 8 BackfillOAuthConnections cases: empty DB, pre-TASK-952 (no key), wildcard, explicit list, mixed resolvable/unresolved slugs, multi-row chain newest-row-wins, refresh-only chain, idempotent re-run (verified via post-run row count). - TestExtractAllowedWorkspacesFromSessionExtra replaces the retired parseAllowedWorkspacesFromSession test — covers all three IDEA-1517 §2 input shapes + malformed/non-array defensive cases. - TestListUserOAuthConnections_DeduplicatesChain + TestHandleListConnectedApps_DTOShapeAndAuditEnrichment updated to call BackfillOAuthConnections (the production startup hook) before asserting on AllowedWorkspaces — mirrors the real-world flow now that the read path no longer parses session.Extra inline. Parent: PLAN-1519. * fix(oauth): backfill counters reflect actual new rows per Codex review (round 1) PR #583 Codex review round 1 flagged that the backfill counters over-report on steady-state restarts: - wasFreshlyInserted compared updated_at vs created_at — true for every untouched existing row, so every restart counted every pre-existing connection as "created." - slugsAdded++ ran after AddConnectionWorkspace regardless of whether the INSERT OR IGNORE / ON CONFLICT DO NOTHING hit an existing row. Net effect: startup logs "backfill complete" with non-zero counts on every restart instead of the intended quiet "no-op" path — making real fresh work indistinguishable from steady-state. Fix: probe existence BEFORE the insert on both sides. - backfillOneChain reads GetOAuthConnection first; only sets created=true and runs insertOAuthConnectionIfAbsent on a miss. - Per-slug: IsConnectionWorkspaceAllowed pre-check; skip + don't increment when the row already exists. Two cheap PK / indexed lookups per chain. Pre-Phase-C deployments have small chain counts so the added cost is well below the scan already running. Removed the now-unused wasFreshlyInserted helper. Added an assertion in TestBackfillOAuthConnections_Idempotent that both ConnectionsCreated and WorkspacesAdded report 0 on the second run — the regression guard for this exact finding. Parent: PLAN-1519. * fix(oauth): backfill skips slug re-seed on existing rows per Codex review (round 2) PR #583 round 2 caught that the round-1 fix protected the parent oauth_connections row from re-seed but left the join table mutable from stale session.Extra: When a user removes a workspace from their connection's allow-list via Phase D's mutation UI (RemoveConnectionWorkspace), the next server restart would re-run the backfill, find the parent row intact, and re-INSERT the removed slug from the original session.Extra. The user's removal would silently revert every restart. Fix: backfill is a one-shot seed. Once the parent row exists, the new tables are authoritative — legacy session.Extra is frozen reference data, not a reconciliation source. The slug loop only runs when we just inserted a fresh parent row. Added TestBackfillOAuthConnections_DoesNotResurrectRemovedWorkspace as the regression guard: seeds two slugs, removes one, runs backfill again, asserts the removed slug stays gone and the kept slug is untouched. Parent: PLAN-1519. * fix(oauth): atomic per-chain backfill transaction per Codex review (round 3) PR #583 round 3 caught that round 2's "only seed slugs on fresh parent" gate introduced a permanent-partial-state risk: if the process crashes (or AddConnectionWorkspace errors) between inserting the parent row and finishing the slug loop, the next backfill sees created=false, short-circuits the slug seeding, and leaves the connection permanently scoped to a partial allow-list. Fix: per-chain transaction. Parent insert + every slug insert land in one BEGIN/COMMIT pair; any mid-loop failure rolls everything back. The next backfill then sees the chain as un-seeded and retries from scratch — preserving both round 2's "no-resurrection of user-removed slugs" (existence probe inside the tx) and round 3's "no permanent partial seed" (atomic commit). Scope: per-chain (small tx), not whole-backfill. The original no-transaction rationale was about lock-hold duration across thousands of chains; that doesn't apply at chain granularity (one parent + a handful of join rows = sub-millisecond hold). Removed the now-unused insertOAuthConnectionIfAbsent helper; the INSERTs live inline within the transaction. Added TestBackfillOAuthConnections_AtomicOnMidLoopFailure as the regression guard: forces a mid-loop INSERT failure via a duplicate slug in session.Extra (which violates the join table's PK on the second insert), asserts the parent row rolled back, then runs a clean retry and verifies full seed completion. Parent: PLAN-1519. * fix(oauth): surface store errors from backfill + list path per Codex review (round 4) PR #583 round 4 caught two silent-fallthrough paths that could leak partial/incorrect state instead of failing loudly: 1. Backfill slug loop: GetWorkspaceBySlug errors were treated the same as "workspace not found" — both incremented slugsMissed and continued. A real I/O error mid-loop would commit a partial allow-list, and the next backfill's parent-exists short-circuit would make that partial scope permanent. Fix: distinguish (nil, nil) "not found" from (nil, err) "real failure" — return the error so the per-chain transaction rolls back and the next run retries cleanly. 2. ListUserOAuthConnections hydration: GetOAuthConnectionAccess and GetOAuthConnection errors collapsed into the "no oauth_connections row" defensive-fallback branch, returning the legacy "any workspace, default-on flags" shape. On a real store failure that silently broadens a user's scope — e.g. a connection the user explicitly removed a slug from would render as "Any workspace" until the store recovered. Fix: surface store errors from both calls; the defensive fallback path is now exclusively for HasConnection=false, not for error masking. Both findings tighten the failure mode from "silently emit broadened/partial state" to "surface the error so retries happen against accurate data." Existing tests cover the happy paths; the failure paths are exercised by I/O errors against the same store interfaces (no new test added — the change is "return err instead of swallow it" and the assertion of NOT swallowing is the diff itself). Parent: PLAN-1519.	2026-05-18 03:32:42 -04:00
xarmian	aec67e202e	feat(mcp): workspace.create + workspace.claim actions + claim-code mechanics (TASK-1521) (#582) ... Phase B for PLAN-1519. Adds two MCP actions so agents can bring a workspace into an OAuth connection without re-auth — the agent-first onboarding story IDEA-1517 §1 set out to fix. pad_workspace.action: create - New action on the shared catalog (stdio + cloud both pick it up). - POSTs to /api/v1/workspaces; handler auto-adds the new workspace to the calling OAuth connection's allow-list (added_by='agent-create') when the grant carries may_create_workspaces=true. Phase A wired the oauth_connection_workspaces table this writes to. PAT / CLI-session callers fall through silently (no request_id → no side effect). - Backed by a new non-interactive `pad workspace create <name>` Cobra command for the stdio MCP shell-out path. `pad workspace init` remains the guided human flow. pad_workspace.action: claim - New POST /api/v1/oauth/claim endpoint redeems a 6-digit stateless HMAC code minted from (user_id, workspace_id, 5-min time bucket) with a sliding 5–10 minute lifetime. Constant-time compare. Code format derived per IDEA-1517 §4. - Verifies workspace membership before code (privilege-escalation guard); uniform 404 envelope so the endpoint can't be used to probe existence vs. membership. - Side effect inserts a row in oauth_connection_workspaces with added_by='claim'. Idempotent — re-claiming returns 200 with already_added=true. - 412 connection_not_persisted when the OAuth grant predates Phase C (no oauth_connections row); 412 claim_disabled when the deployment hasn't wired SetClaimSecret. - `pad workspace claim <code> --workspace <slug>` Cobra command backs the stdio MCP shell-out. MCP server instructions - Appended IDEA-1517 §5 paragraph teaching agents the claim flow as a peer top-level section. Same string lands universally on every MCP handshake response (stdio + cloud both read instructions.md). Tests - 10 claim-code unit tests: determinism, zero-pad, length-prefix collision guard, current/previous bucket accept, aged-out reject, wrong-everything rejects, short-secret fails closed. - 7 handler tests: 412 when secret disabled, 400/404/401 vocabulary, PAT caller note path, 412 connection_not_persisted, idempotent insert. - 5 MCP-catalog tests: actions registered, schema params advertised, description mentions both actions, route mappers produce correct HTTP shape, routeTable carries the entries. - Bumped the existing read-only catalog bijection + fixture-input fixtures so the new actions resolve cleanly. Parent: PLAN-1519.	2026-05-18 00:43:46 -04:00
xarmian	3f6bcc0ff2	feat(oauth): per-connection state tables + dual-read introspection gate (TASK-1520) (#581) ... * feat(oauth): per-connection state tables + dual-read introspection gate (TASK-1520) Phase A foundation for PLAN-1519 / IDEA-1517's per-OAuth-connection state overhaul. Promotes the consent-time workspace allow-list out of session.Extra (per-token, re-minted on every refresh-token rotation) into dedicated tables keyed by request_id (the grant chain identifier preserved across rotations). Schema (SQLite migration 059 + Postgres migration 038): - oauth_connections: one row per grant chain with name + three scope flags (may_create_workspaces, all_current_workspaces, include_future_workspaces). - oauth_connection_workspaces: mutable allow-list join table; PK on (request_id, workspace_id); FK ON DELETE CASCADE; added_by audit column. Store (internal/store/oauth_connections.go): Create/Get/Rename/SetScopeFlags/ Add+Remove+IsAllowed/Delete CRUD. GetOAuthConnectionAccess is the hot-path projection — one PK lookup + one indexed join when the wildcard flag is off, nothing else when it's on. Dual-read gate (internal/server/middleware_mcp_auth.go): OR-merges the legacy session.Extra allow-list with the new-table projection. A workspace is allowed iff either source allows it; either source's wildcard makes the gate unrestricted. New tables stay empty until Phase C writes the consent screen, so the dual-read is a no-op until then — and existing OAuth grants keep working unchanged through the Extra path. I/O errors on the new path fall back to the Extra path so a transient outage of the new tables can't regress existing connections. Tests: - 10 store tests cover CRUD, FK cascade, wildcard short-circuit, sorted slug projection, idempotent add/remove, ErrOAuthConnectionNotFound on missing rows. - 11-case table-driven test on mergeAllowedWorkspaces directly verifies PLAN-1519's acceptance criterion: "token with allow-list in session.Extra still passes; token with empty session.Extra but row in oauth_connection_workspaces also passes." Plus wildcard precedence, union dedup, and fail-closed-on-empty-scope. - BenchmarkMergeAllowedWorkspaces measures policy-function overhead on the hot path (the store-side lookup is the other half of the dual-read cost). Parent: PLAN-1519. * fix(oauth): fail-closed on connection lookup error per Codex review (round 1) PR #581 Codex review round 1 caught two issues: 1. middleware_mcp_auth.go: GetOAuthConnectionAccess errors fell through to "no connection" + nil allow-list = unrestricted. Post-Phase-C (when the new tables are authoritative and session.Extra is empty), a DB read error on a scoped token would silently grant access to every workspace the user belongs to. Now fails closed with a 401 matching the IntrospectToken storage-error policy, increments MCPAuthzDenialsTotal{connection_lookup_error} for ops visibility. 2. oauth_connections.go: CreateOAuthConnection's docstring claimed "scope flags default ON if not supplied," but the method writes the three Go bools verbatim — and Go zero-values for bool are false, not true. The schema-level DEFAULT TRUE is unreachable through this path. Docstring updated to clarify that defaults live at the form-rendering layer; the store is a faithful pass-through. Parent: PLAN-1519. * test(oauth): add store-side bench for GetOAuthConnectionAccess per Codex review (round 2) PR #581 Codex review round 2 flagged the docstring references to bench_oauth_connections_test.go pointing at a file that didn't exist — only the in-memory mergeAllowedWorkspaces bench was wired. Add the store-side bench so the documented file is real and PLAN-1519 Phase A's "Hot-path benchmark: dual-read overhead measured and documented" acceptance bullet is satisfied end-to-end. Three shapes covered: Wildcard (PK lookup, join short-circuited), Explicit (PK + indexed scan + small workspaces join), and NoRow (the dominant Phase-A path until Phase C wires the write path). Local numbers (Ryzen 3 5300U, SQLite WAL): 12µs/12µs/37µs respectively — all comfortably sub-millisecond. Parent: PLAN-1519.	2026-05-18 00:03:20 -04:00