mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-22 02:23:46 +00:00
e8e1822bfc5ac48fb29eee7baad2cb4c5f86fa49
196 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 xarmian
|
bfa32dde5a |
fix(security): encrypt webhook HMAC secrets at rest, mask in responses (BUG-2057) (#915)
Webhook signing secrets were stored plaintext in the webhooks.secret column and echoed back in every API response. Encrypt them at rest (reusing the existing AES-256-GCM store helpers, same pattern as TOTP secrets) and return the raw secret ONLY in the creation response; list responses now mask it and expose a has_secret flag instead. - store: encrypt on CreateWebhook, decrypt on Get/ListWebhooks so the dispatcher still signs with the plaintext secret. Reuses the secret column with the "enc:" prefix — no new column/migration. Keyless self-host stays a no-op fallback (encrypt returns plaintext; decrypt passes legacy rows through unchanged). - BackfillEncryptWebhookSecrets encrypts pre-existing plaintext rows on startup once a key is configured (idempotent), mirroring the TOTP backfill. - model: add HasSecret so masked responses still signal presence. - handlers: mask secret on list; document raw-only-on-create. - tests: encrypt-at-rest round-trip + HMAC validity, list decrypt, plaintext backfill/back-compat, and the API mask-except-on-create contract. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF |
||
|
xarmian
|
43c31c826e |
feat(cli): add claude-code + codex targets to pad mcp install (TASK-2040) (#909)
Extend `pad mcp install/uninstall/status` beyond the three JSON desktop clients to cover the two most prominent CLI agents: - claude-code — writes a project-local `.mcp.json` in the current directory (JSON, same mcpServers shape as the other clients). Because the config is project-scoped, it's install-on-request only: excluded from `--all` and `pad mcp status`, which cover the per-user clients. - codex — writes an `[mcp_servers.pad]` table into `~/.codex/config.toml` (TOML). New load/merge/write path (BurntSushi/toml) that preserves unrelated top-level keys and other mcp_servers entries, is idempotent, tightens perms to 0600, and refuses to clobber a non-table mcp_servers. Generalizes the Agent struct with a Format discriminator (JSON/TOML) and a CWDBased flag; Install/Uninstall/Status dispatch to the right reader/writer and resolve cwd-vs-home per agent. Existing claude-desktop/cursor/windsurf behavior is unchanged. FindAgent's error string is now built from the agent list. Docs updated in README. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF |
||
|
xarmian
|
c62658a11b |
fix(cli): root usage/error/format hygiene (TASK-2031, BUG-2032) (#893)
* fix(cli): silence usage on runtime errors, echo not-found input, validate --format TASK-2031 + BUG-2032 (PLAN-1985). SilenceUsage + FlagErrorFunc keeps flag-error help; GetItem/UpdateItem/DeleteItem wrap not_found with ref+workspace; PersistentPreRunE rejects invalid --format; honest markdown advertising. * fix(cli): return enriched *APIError for not_found to preserve concrete type Team-lead P2: itemNotFoundError changed the concrete error type, so direct err.(*cli.APIError) assertions (which do not unwrap) stopped matching not_found — notably bulk-update's per-row code capture (cmd_item.go:2043), dropping code:"not_found" from the JSON envelope. Return a fresh *APIError (same Code/Details, enriched Message) instead of a wrapper type; APIError.Error() returns Message so the clean one-line message is unchanged. Both err.(*APIError) and errors.As now match. Test adds a direct-assertion + Details-passthrough lock-in. |
||
|
xarmian
|
502d87b890 |
docs(cli): document playbooks+conventions-only export/import constraint (#892)
TASK-2033 (PLAN-1985). Clarify that only playbooks and conventions have a portable-artifact form; point other item types at 'pad item show --format json'. |
||
|
Zak Molloy
|
744e3791e9 |
fix: parent commands exit non-zero on unknown subcommand (#850)
Parent command groups now return a non-zero exit on an unrecognized subcommand (e.g. `pad item bogus`, `pad role lst`) instead of printing help and exiting 0 — a silent failure for an agent-first CLI. Bare parents still show help; valid subcommands unaffected. Covers all 16 command groups plus a regression test that walks the real command tree so a future group can't silently regress. Fixes #850 Co-authored-by: Dave <xarmian@gmail.com> |
||
|
xarmian
|
bed933d7fd |
feat(items): field-level PATCH + conflict envelope + read-only version history (TASK-2022) (#876)
* feat(items): field-level PATCH + conflict envelope + version history Adds three related item-update primitives (TASK-2022 / IDEA-1480): - Field-level merge: PATCH `fields_patch` shallow-merges onto the item's current fields INSIDE the write transaction (null deletes a key), so concurrent single-field updates no longer clobber each other via the full-blob read-modify-write. `pad item update` and the MCP `pad_item.update` action now send only the changed keys. - Optimistic concurrency: optional `expected_updated_at` on update; on mismatch the store returns *UpdateConflictError and the handler emits the pad-structured-error/v1 conflict envelope (HTTP 409, code=update_conflict). Surfaced on CLI (`--expected-updated-at`) and MCP (`expected_updated_at`). - Read-only version history: `pad item history <ref>` (alias `versions`) and MCP `pad_item.history`, reusing the existing item_versions store + versions endpoint (no new store, no schema change). MCP ToolSurfaceVersion bumped 0.9 -> 1.0 (new action + param; update behavior change). No migration required. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(items): address Codex review — dispatcher fields_patch, OCC ordering, date/required guards Round 1+2 review fixes for TASK-2022: - HTTP MCP dispatcher (dispatch_http_advanced.go) now sends fields_patch (only changed keys) instead of a client-side merged full fields blob, and forwards expected_updated_at — remote MCP callers get the same race-free merge + optimistic concurrency the CLI/HTTP paths do. - ValidatePartialFields rejects null-deleting a schema-declared REQUIRED field (would otherwise persist a blob the full-update validator rejects). - Open-children guard on the fields_patch path merges the patch onto the IN-TX locked row inside the precheck (not a stale pre-lock preview), so a priority-only patch can't false-fire the guard. - Optimistic-concurrency check now runs BEFORE the open-children precheck in the store, so a stale expected_updated_at yields update_conflict (not open_children) — single in-tx re-read shared by both. - Date auto-population on the patch path only fills an EMPTY current date; an existing end_date the caller isn't touching is preserved. Tests added for each fix. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
c846cff4fd |
feat(project): agent-accessible activity feed (pad project activity + MCP action) (#877)
* feat(project): agent-accessible activity feed (pad project activity + MCP action)
Add a non-streaming, bounded activity query so agents can catch up on
what other agents/users changed since they last worked — the query
counterpart to the live `pad project watch` SSE stream.
- CLI: `pad project activity [--limit N] [--actor user|agent] [--since DATE]`
backed by the existing GET /workspaces/{ws}/activity feed.
- MCP: `pad_project.activity` action (passThrough) + cloud HTTP route.
- Extend the activity endpoint with a server-side `since` date filter
(handler parse + store SQL clause) so limit/actor/since behave
identically across CLI, stdio MCP, and cloud HTTP transports.
- Bump ToolSurfaceVersion 0.11 -> 0.12 (drift guard, README, CLAUDE.md,
instructions.md) and add a SKILL.md querying-guidance line.
Tests: store since-filter test, HTTP dispatch test, catalog action test.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(mcp): mark pad_project.activity read-only in tool surface
Add activity to readOnlyActions so the serialized MCP tool surface emits
read_only:true (missing entries default to write). Spot-check it in
tool_surface_test.go to guard against regression.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
|
||
|
xarmian
|
2e6538ac34 |
feat(mcp): add read-only attachments surface (pad_attachment) (#875)
Wire the existing attachment HTTP dispatchers onto the MCP catalog as a new read-only pad_attachment tool with list/show actions, mirroring the CLI `pad attachment list` / `pad attachment show`. Both dispatch paths already existed (ExecDispatcher via passThrough, HTTPHandlerDispatcher via dispatch_http_attachments.go) — this exposes them on the tool surface. - New tool rather than pad_item actions: an attachment is its own workspace-scoped resource, not an item property; a dedicated tool keeps pad_item's action enum focused. - Read-only only: upload/download/view stay CLI-only (filesystem-bound), matching the catalog's exclusion rules. - Bumps ToolSurfaceVersion 0.10 -> 0.11; updates instructions.md, README, CLAUDE.md, readOnlyActions, and the drift-guard fixtures. - The base64 image RESOURCE for multimodal agents is deferred to TASK-2076 (ResourceFetcher returns strings; no CLI base64-to-stdout path exists — non-trivial, out of scope here). TASK-2017 Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
c127a5f965 |
fix(playbooks): enforce draft gate server-side + expose status (BUG-2020) (#874)
* fix(playbooks): enforce draft gate server-side + expose status (BUG-2020)
pad_playbook run / POST /playbooks/{ref}/run now refuse a playbook whose
status isn't "active" with a structured playbook_not_active error. Adds
an allow_draft escape hatch across all surfaces: JSON body field, CLI
--allow-draft flag, MCP boolean param, and an "allow-draft" bareword in
raw_args (stripped before strict parsing). status is now echoed on both
the run and get responses.
Bumps ToolSurfaceVersion 0.9 -> 0.10 and updates the drift-guarded docs
(instructions.md, README.md) plus CLAUDE.md.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(playbooks): forward allow_draft on WebMCP + refresh mcp serve help
Codex review follow-up for BUG-2020:
- WebMCP dispatcher + api client now forward allow_draft so the browser
surface can use the draft-gate escape hatch the catalog advertises.
- `pad mcp serve --help` refreshed from the stale "v0.4 / eight tools"
text to the current v0.10 / nine-tool surface (incl. pad_library).
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(playbooks): type playbooks.get() with top-level status (BUG-2020)
Codex P3 follow-up: the get response now returns Item & { status }.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
|
||
|
xarmian
|
7aa5cb98f3 |
perf(bootstrap): compact JSON for agents; trim SKILL.md reference sections (#873)
Part A: `pad bootstrap --format json` now emits compact (no-indent) JSON via a new cli.PrintJSONCompact helper. Its canonical consumer is the /pad agent skill; pretty-print indentation was ~29% of the payload (49696 -> 35118 bytes on this workspace, saving 14578 bytes). Humans keep --format markdown. Part B (conservative): condense the Role Awareness section and the playbook-authoring guidance in skills/pad/SKILL.md to on-demand pointers, keeping the load-bearing core behavior + activation gotcha inline and ALL routing behavior intact. Saves 2764 bytes of fixed per-session overhead. No MCP tool-surface change; ToolSurfaceVersion unchanged. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
ac6e05d1e3 |
refactor(cli): split cmd/pad/main.go by resource (TASK-2015) (#866)
Mechanically split the 9,841-line cmd/pad/main.go god file into cohesive per-resource files (all package main): cmd_item.go, cmd_collection.go, cmd_workspace.go, cmd_auth.go, cmd_project.go, cmd_playbook.go, cmd_role.go, cmd_tag.go, cmd_github.go, cmd_webhook.go, cmd_agent.go, cmd_server.go, cmd_attachment.go, cmd_db.go, cmd_library.go, cmd_bootstrap.go. main.go now holds only main(), newRootCmd(), and shared config/client wiring (265 lines). Zero behavior change — a pure move of command constructors + helpers. All 169 top-level declarations preserved verbatim; the recursive --help command/flag tree is byte-identical to main. cmdhelp and the MCP catalog read command schemas at runtime, so they are unaffected. Updates the CLAUDE.md "Add a new CLI command" recipe to point contributors at the appropriate cmd_<resource>.go file and groups.go, so the file stops being a merge-conflict magnet for parallel agents. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
15ad930d78 | feat(bootstrap): add convention_index for triggered-convention discovery (TASK-2004) (#848) | ||
|
xarmian
|
0aa431f132 |
fix(server,cli,mcp): default item list to per-collection non-terminal filter (BUG-2001) (#845)
The CLI's default `pad item list` (no --status/--all) sent a hardcoded ~20-status allowlist as the status filter. Collections with custom status vocabularies (blog: drafting/scheduled; human-tasks: todo) fell outside the list and had their open items hidden. MCP inherited the same bug via the CLI default and the HTTP route table's mirrored allowlist. Replace it with a server-side `non_terminal` filter: ItemListParams.NonTerminal resolves each collection's terminal set from its schema's terminal_options (falling back to DefaultTerminalStatuses) and keeps only items NOT in that set — reusing the existing doneFiltersForWorkspace + buildChildrenDoneExpr machinery, applied in both the normal and FTS query paths. The CLI default and both MCP dispatch paths (ExecDispatcher via the CLI, HTTPHandlerDispatcher via mapItemList) now send non_terminal=true. --status X and --all semantics are unchanged. |
||
|
xarmian
|
8c609be2e3 |
feat(store): guard against schema-ahead downgrade + pre-migration snapshot + upgrade docs (TASK-2006) (#843)
The migration runner only applied missing embedded migrations and never detected a DB that was AHEAD of the binary, so a brew/docker downgrade silently ran old code against a newer schema. It also took no backup before migrating, and there were zero upgrade docs. - guardSchemaAhead: refuse to start when schema_migrations contains a version that sorts after the highest embedded migration (a downgrade). Escape hatch: 'pad start --force' / PAD_ALLOW_SCHEMA_AHEAD=1. Applied to both the SQLite and Postgres migration paths. - snapshotBeforeMigrate (SQLite only): copy the DB file to <db>.pre-<VERSION> before applying pending migrations, but only when upgrading an existing DB (pending AND already-applied migrations). WAL-checkpointed, atomic temp+rename copy, and preserves an existing snapshot on retry so a failed multi-step upgrade can't clobber the original rollback point. Postgres is skipped (pg_dump/PITR is the DBA's). - Docs: 'Upgrading Pad' in README + an 'Upgrading' section in docs/deployment.md (forward-only rule, guard behavior, snapshot, flow). |
||
|
xarmian
|
cf5eb8dd3a |
feat(cli,mcp): summary-shaped item list with --full opt-in + limit clamp (TASK-2000) (#842)
`pad item list --format json` returned the full models.Item shape — including each item's rich markdown `content` body (~52% of the bytes) plus UUID plumbing and duplicate join fields — with no default limit, so a bare agent list dumped ~1.4MB (all collections) or 5.3MB (--all) into context. The single biggest agent-token lever. CLI: - JSON output now defaults to a token-light ItemSummary projection: `content` → short `content_preview`, UUIDs (id/workspace_id/collection_id/*_user_id/ parent_id/agent_role_id) and duplicate collection/parent join fields dropped, `fields`/`tags` emitted as nested JSON. ~71% smaller on a real workspace. - `--full` opt-in flag restores the complete models.Item shape. - Default limit (200) + hard-max clamp (1000) so --all/huge lists can't dump unboundedly; a stderr note fires when a table result is capped. MCP: - pad_item.list is now a custom action that injects a default limit (50) and clamps an oversized one (max 300), mirroring the backlinks default/max, so a bare agent list stays bounded on both dispatchers. - ToolSurfaceVersion 0.8 → 0.9 (list result shape + limit behavior change). Server: - Hard-max backstop clamp (1000) on an explicit `?limit=` at the item-list request boundary; no default (internal ListItems callers that fetch every row are untouched). rawJSONOrNil guards against a malformed stored Fields/Tags value breaking the whole list marshal (falls back to a JSON string). |
||
|
xarmian
|
9be8e96cfd |
fix(cli): safe SQLite db backup/restore (config path + VACUUM INTO) (#837)
* fix(cli): safe SQLite db backup/restore (config path + VACUUM INTO)
pad db backup/restore hardcoded ~/.pad/pad.db, so `docker exec pad db
backup` (container sets PAD_DATA_DIR=/data) and Windows layouts broke,
and the SQLite path did a torn io.Copy of pad.db + separate -wal/-shm
copy that could lose or tear in-flight WAL writes.
- Resolve the SQLite path via the server's config loader (PAD_DB_PATH >
PAD_DATA_DIR/pad.db > ~/.pad/pad.db) instead of os.Getenv("HOME").
Covers backup, restore, and migrate-to-pg's --from default.
- Replace the file copy with an online-safe `VACUUM INTO` through the
embedded modernc.org/sqlite driver: one self-contained file, no
-wal/-shm juggling, safe while the server is live.
- Restore refuses when a live server is detected (a running WAL
checkpoint could clobber the restored file); --force overrides.
- docs/backup.md: `pad db backup -o <file>` is the canonical SQLite
path (+ the `docker exec <container> pad db backup -o /data/backup.db`
form); dropped the "PostgreSQL-only" mislabel.
PostgreSQL pg_dump/psql paths are unchanged.
Fixes BUG-1996.
Claude-Session: https://claude.ai/code/session_01BoPkYhKqMiWPYmxQigeWsA
* fix(cli): fail restore on stale sidecar removal + drop unsafe backup doc
Address Codex review P2s:
- Restore: treat a failure to remove a stale -wal/-shm at the target as
fatal (was silently ignored). With single-file VACUUM INTO backups a
leftover sidecar would replay old WAL state over the restored DB.
- docs/backup.md: the SQLite strategy block still recommended a raw
`cp pad.db` daily; point it at `pad db backup --cron` instead.
Claude-Session: https://claude.ai/code/session_01BoPkYhKqMiWPYmxQigeWsA
|
||
|
xarmian
|
1b99c41fcf |
feat(cli): add 'pad workspace restore' + 'pad workspace deleted' (TASK-1972) (#833)
Wire two Cobra subcommands to the existing Client.RestoreWorkspace / ListDeletedWorkspaces methods: 'pad workspace restore <slug>' un-soft-deletes within the 30-day window, and 'pad workspace deleted' lists restorable workspaces with days-left. Both support --format json. Closes TASK-1972. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
b73ba63752 |
feat(store): hard-purge soft-deleted workspaces after 30 days (TASK-1966) (#825)
The /privacy policy promises owned workspaces are removed from live systems within 30 days, but DeleteAccountAtomic and DeleteWorkspace only SOFT-delete (workspaces.deleted_at) and nothing ever expunged them — a right-to-erasure gap. Add a scheduled sweeper that hard-purges workspaces soft-deleted longer than a named 30-day retention constant. - Store: ListPurgeableWorkspaces (soft-deleted + past cutoff; never touches live rows), WorkspaceAttachmentBlobs, CountAttachmentsForHash- OutsideWorkspace (content-addressed dedupe guard), and PurgeWorkspace- Data — a transactional cascade that deletes every workspace-scoped child row in FK-dependency order (items/comments/versions/links/ reactions/stars/yjs op-log/wiki-links/grants/transitions/moves/views/ collections/documents+versions/agent_roles/webhooks/invitations/ templates/share_links+views/oauth join rows/report layouts/members/ member access/api tokens/attachments/activities), de-identifies mcp_audit_log, and refuses to touch a non-soft-deleted workspace. - Server: a periodic sweeper modeled on the orphan GC — captures blob keys before the purge, cascades the DB rows, then reclaims blobs through the attachment store abstraction (FS + S3 safe) with the orphan GC's cross-workspace dedupe + in-flight-upload guards. Failure isolated per workspace; idempotent. - Dual-dialect (SQLite + Postgres); partial index on workspaces(deleted_at) — migrations/073 + pgmigrations/051. Both delete paths (account + manual workspace delete) purge on the same 30-day clock: identical deleted_at mechanism, both owner-initiated, and the orphan GC already reclaims their attachment blobs at 30 days. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
d36f27c29f |
fix(server): auth-perimeter hardening — B6–B9 from the IDEA-1927 audit (TASK-1932) (#811)
* fix(server): stop autoCreateWorkspace from swallowing member-add errors (B6, TASK-1932) A failed AddWorkspaceMember after workspace creation used to be silently discarded, leaving a workspace that's completely unreachable (owner_id alone grants no access) and invisible in the console forever. Retry once, then clean up the orphaned workspace and log loudly on continued failure so on-call can act on it. * fix(server): fail fast when cloud mode runs without secure cookies (B7, TASK-1932) SetCloudMode never forced secureCookies on, so PAD_CLOUD=true without PAD_SECURE_COOKIES was an unenforced ops contract: OAuth's __Host-prefixed session cookie is silently invisible to pad's own cookie reader without Secure set, producing a "logged in but appears logged out" failure mode. Add Config.ValidateCloudSecureCookies and check it at server startup, next to the existing PAD_CLOUD_SECRET requirement, so the misconfiguration is a startup error instead of a runtime mystery. * fix(server): align OAuth session TTL with web session TTL (B9, TASK-1932) handleOAuthLogin minted a 30-day session while every other web login used the 7-day webSessionTTL. createAuthSession derives the store session row, session cookie MaxAge, and CSRF cookie MaxAge all from one ttl argument, so the longer OAuth cookie outlived its own server-side session — the browser kept presenting a cookie whose session had already expired, producing silent 401s. Use webSessionTTL for OAuth logins too. * fix(server): narrow the /api/v1/auth/* CSRF exemption to anonymous endpoints (B8, TASK-1932) The CSRF middleware exempted the entire /api/v1/auth/ prefix, which also covered mutating cookie-authenticated endpoints: PATCH /me, oauth-unlink, 2FA setup/verify/disable, delete-account, token create/delete/rotate, CLI- session approve, and logout. Replace the prefix bypass with an exact-path allowlist of the endpoints that are genuinely pre-session (login, register, bootstrap, password reset, verify-email, resend-verification, 2FA login challenge, CLI session create) or authenticate purely via a cloud secret rather than a cookie (oauth-login, oauth-link — never touch the session, so CSRF isn't a meaningful threat model for them and the sidecar has no CSRF cookie to send). Everything else now requires the double-submit token like any other authenticated mutation; the web client already sends it on every non-GET/HEAD request, so no frontend change is needed. * docs(server): pin the deliberate CSRF-cookie legacy-fallback asymmetry (TASK-1932) Codex review (round 1) flagged that SessionAuth falls back from the __Host-pad_session cookie to the legacy unprefixed name, but the CSRF cookie lookup has no equivalent fallback — meaning a browser holding pre-secure-cookies-flip legacy cookies stays authenticated but gets 403'd on B8's newly CSRF-required endpoints until it re-logs-in. This asymmetry is deliberate, not a bug: the session cookie's value is an unguessable secret regardless of which name carries it, but the CSRF cookie's security property depends on the attacker being unable to set the cookie itself — an unprefixed name is settable from a sibling subdomain, which is exactly the hole __Host- exists to close. Restoring "symmetry" here would silently reopen it. Document the reasoning at the cookie lookup so a future maintainer doesn't "fix" it, and add a pinning test that exercises the exact scenario (secureCookies=true, legacy session + CSRF cookies, CSRF-required endpoint) end to end. * fix(server): require CSRF for session-authenticated requests to exempt auth paths (TASK-1932) Codex round 2 found a P1: handleRegister has an admin-session branch (an already-logged-in admin can create a verified account with no invitation code), but /api/v1/auth/register was unconditionally CSRF-exempt by path. A cross-site POST could ride the admin's cookie into that branch with no CSRF token — the same class of hole as the oauth-unlink case B8 already closed, just missed because register's other paths are genuinely anonymous. Fix generically rather than register-specifically: gate the authCSRFExemptPaths exemption on currentUser(r) == nil. SessionAuth runs before CSRFProtect, so a request that resolved to a real session falls through to the normal double-submit check instead of the early exemption, while a genuinely anonymous request keeps it. This also covers any future session-authenticated branch a handler on this list grows, with no handler changes. Bearer/PAT and cloud-secret (oauth-login/oauth-link) callers are unaffected — they have their own unconditional exemptions later in the same function. * fix(server): require validated Bearer/cloud-secret auth for CSRF exemption (TASK-1932) Codex round 3 found that CSRFProtect's Bearer and X-Cloud-Secret exemptions fired on header/marker PRESENCE, not validation. TokenAuth deliberately falls through (rejectInvalidBearer) instead of 401ing invalid Bearers on /api/v1/auth/* paths to support CLI-token recovery, so a cross-site request carrying a victim's real session cookie plus a garbage Bearer header could ride the cookie past CSRF on any newly-CSRF-required endpoint. The same presence-only pattern in the X-Cloud-Secret exemption is concretely exploitable too: handleSetPlan (and similarly-shaped handlers) accept an admin cookie session as an alternative to the secret, so a garbage X-Cloud-Secret plus a stolen admin cookie could set an arbitrary user's plan with no CSRF token at all. Add ctxValidatedSessionBearer (set by TokenAuth only on successful ValidateSession for CLI session-bearer tokens) alongside the existing ctxIsAPIToken, and a combined isValidatedBearerAuth() helper. CSRFProtect now exempts unconditionally only on validated Bearer auth; an unvalidated Bearer header or cloud-secret marker is exempt only when no session was also resolved for the request (currentUser(r) == nil), preserving the CLI-recovery contract (stale token, no cookie -> 401 from auth, not csrf_error) while closing the cookie-riding case. * fix(server): split CSRF auth-exempt allowlist by session sensitivity (TASK-1932) Codex round 2 gated the entire authCSRFExemptPaths allowlist on currentUser(r) == nil to close handleRegister's admin-session branch, but that gate applied to every anonymous endpoint on the list, not just register. CI's E2E suite caught the regression: the harness bootstraps an admin (minting a session cookie) then POSTs /login to re-authenticate, and the ambient cookie stripped /login of its exemption, producing a spurious 403 csrf_error. login/bootstrap/forgot-password/reset-password/local-reset/verify-email/ resend-verification/2fa-login-verify/oauth-login/oauth-link/cli-sessions- create derive their authority entirely from the request body (credentials, a token, a shared secret), never from the ambient cookie, and pad mints the CSRF cookie AT LOGIN — a pre-session endpoint categorically cannot require a token that doesn't exist yet. Split the allowlist: authCSRFUnconditionalExemptPaths (everything above, exempt regardless of cookie) and authCSRFSessionGatedExemptPaths (register only, exempt only when currentUser(r) == nil, since it alone has a session-privileged admin-account-creation branch). The round-2 security property (admin session + register + no CSRF -> still blocked) and round-3's Bearer/ cloud-secret validated-vs-present composite are unaffected. |
||
|
xarmian
|
31053086ee |
feat(server): RequireVerifiedEmail enforcement across all mutation perimeters (TASK-1937) (#807)
PLAN-1933 DR-4 (Wave 3a). Enforce, on Pad Cloud only, that an
authenticated-but-unverified user cannot mutate content or mint
credentials. Cloud-only + unauthenticated + verified are all no-ops, so
this is inert in production until Wave 3b starts creating unverified
users; the tests drive the state directly via the store's explicit
UserCreate.Unverified control.
Core rule: block only when cloudMode && currentUser != nil &&
!IsEmailVerified(), on mutating methods (POST/PATCH/PUT/DELETE). Returns
403 email_not_verified. The middleware does NOT inherit CSRFProtect's /
RequireAuth's blanket /api/v1/auth/* exemption — it decides for itself.
Perimeters gated (systematic DR-4 audit — one test each):
- /api/v1 core writes (session AND PAT) — RequireVerifiedEmail method
gate mounted after RequireAuth (server.go).
- Authenticated /auth/* mutations — token create/rotate/delete, PATCH
/me, 2FA setup/disable, OAuth link/unlink, and cli-session approve —
all fall through the method gate (no auth exemption); logout,
verify-email, resend-verification, delete-account allowlisted.
- Collab WS GET-upgrade — authorizeCollabAccess (a GET the method gate
can't catch; it persists Yjs edits).
- Remote MCP write path — dispatcher RequireVerifiedEmail hook fired in
buildAuthedRequest (the single chokepoint every synthesized write
passes through), wired in cmd/pad/main.go.
- OAuth-provider authorize + authorize/decide — emailUnverifiedBlocked
checks (mounted outside /api/v1; decide mints the auth code).
- POST /api/v1/import/url — SSRF/abuse surface, method gate.
Carve-outs: POST /api/v1/invitations/{code}/accept stays open for
unverified invitees (DR-1); legacy no-user workspace PATs are
intentionally ungated (currentUser==nil). Self-host (!cloudMode) is a
full no-op.
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
|
||
|
xarmian
|
b0eeef16ce |
feat(store): email_verification_tokens + SendEmailVerification + token reaper (TASK-1936) (#806)
Wave 2 of PLAN-1933 — verification-token infrastructure (pure infra; no endpoint consumes it until Wave 3). - Migration 071 (SQLite) / 049 (Postgres): email_verification_tokens table, cloning the password_resets shape (id/user_id FK/token_hash/expires_at/ used_at/created_at + token_hash + user_id indexes), per-dialect created_at default. - Store email_verification.go: 256-bit crypto/rand token, padver_ prefix, SHA-256-at-rest, non-destructive Lookup, atomic UPDATE...RETURNING Consume. Deltas from password_resets (DR-2): 24h TTL, keep invalidate-prior-on-mint (resend burns the old link), consume side-effect sets users.email_verified_at (RFC3339-with-Z, same format Wave 1's migration used) in one transaction — no password reset, no session mint. - Email SendEmailVerification: clones SendPasswordReset, "1 hour" -> "24 hours". - Token reaper (DR-5): lifecycle-safe background sweep (mirrors orphanGC/opLogGC — self-registers on Server.bg, context-cancellable via stop channel, started only from cmd/pad/main.go so unit tests don't leak goroutines) calling the four previously-unwired CleanExpired* methods (email verifications, password resets, sessions, CLI auth sessions) hourly. Adds CleanExpiredEmailVerifications. - Audit consts ActionEmailVerified + ActionEmailVerifiedByAdmin. Gates: make check + make test-pg green (store + migration on both dialects). Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
df3cf55d8f |
fix(mcp): stop cloud session-workspace bleed across users (BUG-1865) (#780)
The cloud /mcp transport constructed a single process-global WorkspaceState shared across every OAuth user and MCP session. pad_set_workspace mutated it, and env.Dispatch injected the shared value into any tool call without an explicit `workspace` — so one session's selection bled into another's (cross-user, and across a single user's concurrent sessions). Not a cross-tenant write: BUG-1616's bearer-auth membership gate already 403s a non-member. The real harm is workspace-slug leakage, confusing "not a member of <someone else's ws>" errors, and wrong-destination reads/writes among a user's OWN accessible workspaces. Fix: add NewSharedWorkspaceState() whose ResolveDefault() always returns "". The cloud mount uses it, so the shared value is never injected as a per-call default — resolution falls back to explicit workspace= (or the per-user maybeInjectWorkspace default), never cross-user shared memory. pad_set_workspace on a shared state no longer persists and returns status=not_persisted. Local `pad mcp serve` (single-user-per-process) is unchanged. Also make the agent-facing surfaces honest per-deployment: the pad_set_workspace tool description, the pad_item workspace param, pad_meta tool-surface, and the embedded instructions.md no longer promise session defaulting on multi-user/remote servers, and the two "workspace is required" hints drop the stale pad_set_workspace reference. Regression guards in internal/mcp/bug1865_test.go. Full suite green. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
e4caad2c64 |
feat(server): expose MCP tool-surface over authed REST endpoint (#764)
Add GET /api/v1/mcp/tool-surface, a session/token-authenticated same-origin endpoint that serves the MCP catalog descriptor JSON (the nine env.Catalog tools, their actions, and input schemas) with a new per-action read_only bool. Backs the Phase 3 browser-side WebMCP layer (PLAN-1888): the client fetches once and derives readOnlyHint from the read_only flags without re-deriving the read set in TS. Wired via the SetMCPTransport injection pattern to avoid the import cycle: internal/mcp already imports internal/server (dispatch_http.go), so internal/server cannot import internal/mcp. internal/mcp exports a cycle-free ToolSurfaceJSON() that builds from the package-global Catalog plus a co-located readOnlyActions allowlist; cmd/pad/main.go (which imports both) injects it via Server.SetToolSurfaceHandler before setupRouter. The route mounts in the authed API group so it inherits TokenAuth/SessionAuth/CSRFProtect/RequireAuth — NOT the bearer-gated /mcp infra path — and is available on both cloud and self-host. The existing actionMetaToolSurface (pad_meta action=tool-surface) now shares the same serializer, so MCP and REST can't drift; it gains the additive read_only flag too. No ToolSurfaceVersion bump (DR-7): adding read_only is additive metadata; names/actions/params are unchanged. Refs TASK-1891 / PLAN-1888 Claude-Session: https://claude.ai/code/session_01KmxkPxLksjf1pmrZDpsnTJ |
||
|
xarmian
|
7d07bfa6a2 |
fix(cli): share stdin reader in login so piped --interactive works (BUG-1886) (#762)
doInteractiveLogin read the email line with one bufio.Reader, then readPassword built a second independent bufio.Reader on os.Stdin. bufio reads in chunks, so the first reader could buffer past its newline and swallow the password line, breaking piped `pad auth login --interactive`. Extract readPasswordFrom(fallback *bufio.Reader): TTY path unchanged (term.ReadPassword, no echo); non-TTY fallback reads from the supplied reader. doInteractiveLogin now shares its reader across email/password/2FA. readPassword() stays as a fresh-reader wrapper for other callers. Pre-existing bug (predates BUG-988). Verified: go test ./... (SQLite), make test-pg (Postgres), make lint all green; new TestReadPasswordFromSharedReader reproduces the email-then-password sequence. Claude-Session: https://claude.ai/code/session_01WK9cUjxniBBAihGD5ygjDr |
||
|
xarmian
|
665f1918a7 |
feat(cli): headless admin bootstrap via --email/--name/--password (BUG-988) (#761)
* feat(cli): headless admin bootstrap via --email/--name/--password (BUG-988) Adds non-interactive flags to `pad auth setup` and `pad init` so agents running inside Claude Code or other non-TTY environments can bootstrap a fresh Pad instance without hitting interactive prompts that block forever. - New flags --email, --name, --password on both commands; all three must be supplied together when any one is present (clear error naming the missing flag otherwise). Checked before the remote-mode guard so the headless path works on any server host — the loopback gate is enforced server-side. - runHeadlessSetup() drives the existing POST /api/v1/auth/bootstrap endpoint directly, saves credentials, and respects --format json (emits a LoginResponse-shaped object with user + token). Already-initialized conflict produces a structured JSON error object under --format json. - `pad init` slots headless bootstrap into the bootstrap step only; the rest of init (config, workspace creation, skill install) continues. - Hardens readPassword() with an early non-TTY guard (generic message). - Hardens promptAndBootstrap() with a bootstrap-specific non-TTY guard (points at --email/--name/--password flags) so --cli-prompt on a pipe exits immediately rather than blocking. - Extends `pad init` non-TTY error message to mention the new flags. - Five new tests in cmd/pad/setup_headless_test.go covering success, missing-flag validation, non-TTY guard, already-initialized conflict, and the full init flow including workspace creation. NOTE: --password is visible in process listings (inherent to flag-based injection). Env-var bootstrap (PAD_ADMIN_*) is the tracked follow-up. Claude-Session: https://claude.ai/code/session_01WK9cUjxniBBAihGD5ygjDr * fix(cli): thread bootstrap token, factor shared core, restore readPassword fallback Round-1 codex findings: 1. Bootstrap token not sent on headless path (BLOCKER) Add BootstrapWithToken(email, name, password, token) to cli.Client that sets X-Bootstrap-Token when token is non-empty. Export ReadBootstrapToken from internal/cli/bootstrap.go (was readBootstrapToken) so cmd/pad can call it. Extract doHeadlessBootstrap(cfg, client, email, name, password) as the shared core for both setupCmd and padInitCmd: reads the on-disk token best-effort (absent → empty → loopback gate still covers that case), calls BootstrapWithToken, saves credentials, sets auth token on client. Both headless paths now go through this single function — no divergence. 2. readPassword bufio fallback removed by accident (REGRESSION) Restore the pre-round-1 bufio fallback in readPassword so piped-password flows (e.g. pad auth login --interactive in CI) keep working. The bootstrap wedge is already prevented by the top-of-promptAndBootstrap TTY guard; the generic readPassword fallback is only reached by non-bootstrap callers. 3. Shared core (CLEANUP) padInitCmd now calls doHeadlessBootstrap instead of duplicating Bootstrap + saveCredentials + SetAuthToken. The --format json asymmetry (init vs setup) is resolved by design: pad init is a multi-step flow; for machine-readable bootstrap output agents should use `pad auth setup --email … --format json`. Documented in the inline comment on the headless branch in padInitCmd. Tests added: TestHeadlessSetupSendsBootstrapToken, TestHeadlessSetupNoTokenFileOK, TestReadPasswordFallback. Update internal/cli/bootstrap_test.go for the rename. Claude-Session: https://claude.ai/code/session_01WK9cUjxniBBAihGD5ygjDr * BUG-988 round-2: surface token-read errors, wrap 403 with hint, rescope readPassword test doHeadlessBootstrap: distinguish os.ErrNotExist (absent token → best-effort empty, proceed without header) from other read errors (permissions, etc. → surface with the file path so operators can diagnose rather than silently hitting a confusing 403). Wrap 403/forbidden from BootstrapWithToken with an actionable multi-bullet hint covering loopback gate, token-file path, and PAD_BYPASS_SETUP_TOKEN. ReadBootstrapToken (internal/cli/bootstrap.go): add %w to the ErrNotExist branch so errors.Is(err, os.ErrNotExist) propagates to callers; existing tests and --cli-prompt hint text preserved. TestReadPasswordFallback → TestReadPasswordBufioFallback: rescoped to assert readPassword isolation only; added comment citing BUG-1886 (pre-existing doInteractiveLogin double-bufio.Reader bug). BUG-1886 filed in docapp. |
||
|
xarmian
|
616a6d2a0a |
feat(auth): localhost password recovery for locked-out self-host admins (#760)
Add a loopback-only account-recovery path so a self-hosted operator who
forgot their password (with no email provider configured) can recover
without editing the database by hand.
- POST /api/v1/auth/local-reset: loopback-gated, non-cloud, no auth
required (same trust model as bootstrap). Returns a single-use reset
link, or a temporary password with {"temp_password": true}.
- pad auth reset-password <email> [--temp-password]: talks to the local
server over loopback directly (not the configured public URL), so the
command works on the server host regardless of CLI config. Prints the
server's shareable reset_url when a public base URL is known.
- Web /forgot-password reads email_configured from the session and shows
host-recovery instructions instead of a dead "we emailed you a link"
when no provider is configured.
- forgot-password server log emits the reset path on non-cloud instances
so operators can also recover straight from the logs.
- Docs: CLAUDE.md + docs/deployment.md recovery sections.
Tests cover the loopback/cloud gates, the shareable reset_url, and both
output modes (reset link + temp password).
|
||
|
xarmian
|
285a58e40e |
feat(artifact): pad item export/import CLI commands (#756)
* feat(artifact): pad item export/import CLI commands Phase 3 of PLAN-1867. - pad item export <ref> [-o file] — writes a playbook/convention as a portable <slug>.pad.md artifact (or stdout via -o -). - pad item import <file> — POSTs the artifact (or stdin via -), prints the new draft ref + slug and any server warnings (coerced fields, renamed slug). Adds ExportItemArtifact/ImportArtifact client methods. Implements TASK-1876, TASK-1877. Claude-Session: https://claude.ai/code/session_01KmxkPxLksjf1pmrZDpsnTJ * fix(artifact): harden CLI export file write Addresses Codex Phase-3 review: - filenameFromContentDisposition reduces to filepath.Base with safe fallbacks — a hostile Content-Disposition can't traverse/abs-write. - export writes atomically (temp + Sync + Rename) like attachment download, so a failed write can't truncate/leave a partial artifact. Claude-Session: https://claude.ai/code/session_01KmxkPxLksjf1pmrZDpsnTJ |
||
|
xarmian
|
4f0984bb15 |
feat(artifact): server export + import endpoints for playbooks & conventions (#755)
* feat(artifact): server export + import endpoints for playbooks & conventions
Phase 2 of PLAN-1867. Adds:
- GET /workspaces/{ws}/items/{ref}/export — item-visibility-gated; encodes a
playbook/convention item to a Markdown+frontmatter artifact.
- POST /workspaces/{ws}/import-artifact — editor-gated; byte-capped +
YAML-bomb-guarded parse, forgiving preprocess (foreign selects blanked,
invocation_slug de-collided, status forced draft), creates via the shared
create path.
- Extracts createItemChecked from handleCreateItem so import inherits
validation / uniqueness / edit-perm / side-effects (no direct store.CreateItem).
- PAD_IMPORT_ARTIFACT_MAX_BYTES env override.
Server validation, coercion, and YAML input limits land at the HTTP boundary
per DR-4/DR-7/DR-8 and the Codex P2 notes (collSlug via shared helper,
item-visibility export auth, byte-cap→node-walk→decode ordering).
Implements TASK-1871, TASK-1872, TASK-1873, TASK-1874.
Claude-Session: https://claude.ai/code/session_01KmxkPxLksjf1pmrZDpsnTJ
* fix(artifact): enforce item quota + require title on artifact import
Addresses Codex Phase-2 review:
- P1: handleImportArtifact now calls enforcePlanLimit(items_per_workspace)
before create, matching handleCreateItem — imports can't exceed the plan cap.
- P2: reject empty/whitespace-only artifact titles with 400 (Title is required),
matching the normal create path.
Claude-Session: https://claude.ai/code/session_01KmxkPxLksjf1pmrZDpsnTJ
|
||
|
xarmian
|
22d901c823 |
fix(auth): unify first-run setup into one browser handoff (BUG-1843) (#739)
On a fresh instance, `pad init` / `pad auth setup` created the admin account in the browser and dropped the operator on the console, then printed a SECOND "authorize the CLI" URL back in the terminal that a user who'd moved to the browser never saw — forcing a ctrl-C + re-run. Collapse it into a single browser tab: the CLI mints the pending CLI auth session up front and hands /setup a validated `next=/auth/cli/<code>` target, so account creation flows straight into the approval page where the just-bootstrapped admin approves in one click and the CLI connects. - internal/cli/bootstrap.go: thread `next` into the /setup URL (query before the #token fragment); raise bootstrapPollTimeout to 20m to match the setup session TTL. - cmd/pad/main.go: extract pollAndSaveCLIAuth; runBrowserSetup pre-creates the session and polls it; `pad workspace init` drives local setup inline. - cmd/pad/init.go: `pad init` routes through the unified handoff. - internal/store + internal/server: grant a setup-specific 20m CLI auth session TTL when UserCount==0 so the combined create-account + approve window can't expire mid-flow; normal logins keep the 5m default. - web/src/routes/setup: honor a validated local `next` redirect (open- redirect guarded), preserved across the token-fragment scrub. Reviewed via Codex loop (3 rounds → clean). Claude-Session: https://claude.ai/code/session_01KmxkPxLksjf1pmrZDpsnTJ |
||
|
xarmian
|
248f7c5ede |
feat(items): expose item restore via CLI + MCP (TASK-1828) (#734)
Adds the agent-facing restore surface so an archived item discovered via
`pad item list --all` can be recovered without dropping to the web UI. The
server already had restore end-to-end (Store.RestoreItem + handleRestoreItem
at POST /items/{ref}/restore, used by the web UI and bulk ops); this wires
the two missing surfaces:
- CLI: `pad item restore <ref>` (cli.Client.RestoreItem → the existing
endpoint, which resolves the ref include-deleted server-side). Mirrors
`pad item delete`'s structured JSON envelope: {ref, title, restored: true}.
- MCP: pad_item action=restore via passThrough(["item","restore"]). Restore
is non-destructive, so it's safe to expose. The action auto-joins the
schema's action enum (derived from the Actions map) and is documented in
the tool description.
Conflict case (slug/invocation_slug reclaimed while archived) is already
handled by handleRestoreItem (409) and surfaced by the client's
handleResponse.
Tests: restore endpoint already covered (handlers_items_test.go); restore
added to the MCP catalog<->cmdhelp bijection + dispatch tests. Child of
BUG-1791 (TASK-1827 shipped in #733).
|
||
|
xarmian
|
b9ddd02ae7 |
feat(cli): add --tag filter to pad item list (TASK-1658) (#662)
The HTTP item-list endpoints already parse ?tag= (cross-collection on the workspace route), but the CLI had no flag to forward it. Add --tag, wired as a query param alongside --status/--role/--parent. The MCP pad_item list action inherits it via the cmdhelp passthrough (no catalog change). Parent: PLAN-1652. |
||
|
xarmian
|
1b1068537c |
feat(tags): workspace tag enumeration endpoint + cross-collection filter (TASK-1653) (#658)
* feat(tags): workspace tag enumeration endpoint + cross-collection filter (TASK-1653)
Foundation for the tags feature (PLAN-1652 / IDEA-1649). The write path and
per-collection ?tag= filter already existed; this adds tag enumeration and a
verified cross-collection read so a single tag can group items of any type.
- store: dialect.JSONArrayElements unnests a JSON text-array column
(json_each on SQLite, jsonb_array_elements_text on Postgres);
Store.ListWorkspaceTags returns distinct tags + item counts, ordered by
count desc then tag asc, with the same collection/item ACL filters as
ListItems so counts never leak hidden items.
- server: GET /workspaces/{ws}/tags (handleListTags), respecting collection
visibility + guest item grants.
- models: TagCount{tag,count}.
- cli: client.ListTags + `pad tag list`.
- web: api.tags.list + TagCount type (items.list already forwards `tag`).
- tests: store-level (cross-collection aggregation, collection scoping,
non-nil-empty = empty, archived excluded) and handler-level (a Task + an
Idea sharing one tag; GET /tags counts + ordering).
Parent: PLAN-1652.
* fix(tags): count distinct items per tag, not tag occurrences per Codex review (round 1)
COUNT(DISTINCT i.id) so an item with duplicate tags (e.g. ["ux","ux"]) is
counted once — the write path doesn't enforce per-item tag uniqueness.
Adds a regression test.
|
||
|
xarmian
|
d9fab3ea02 |
feat(report): cycle-time + WIP/aging metrics (TASK-1631) (#642)
* feat(report): cycle-time + WIP/aging metrics (TASK-1631)
Extend GET /workspaces/{ws}/report with two metric blocks:
- cycle_time: created→positive-terminal duration for completions in the
window — overall median + p90 + per-collection medians.
- wip: point-in-time open items (done field NOT a terminal value), open count,
median age, fixed aging bands (<1d/1-7d/7-30d/>30d), per-collection median age.
Medians/percentiles computed in Go from raw durations (dual-dialect: neither
SQLite nor Postgres has a portable percentile). Completed/WIP queries join live
items (deleted_at IS NULL), consistent with the rest of the report.
Wires through web ReportData TS types + `pad project report` rendering.
Tests: cycle-time median (backdated 48h), WIP open-count + aging bands,
percentile helper. Parent: PLAN-1628.
* fix(report): well-formed cycle_time/wip arrays on empty-scope path per Codex review (round 1)
The no-visible-collections early return left cycle_time.by_collection,
wip.aging_buckets, and wip.by_collection as nil → marshaled null, violating
the TS array contract for guests/restricted callers. Initialize those nested
slices in the ReportData literal so every path (including the early return) is
well-formed. Adds a JSON-shape regression test for the empty-scope case.
|
||
|
xarmian
|
949ae03c88 |
feat(cli,mcp): pad project report + pad_project report action (TASK-1635) (#641)
Expose the report aggregation (TASK-1630) to agents:
- CLI: `pad project report [--window day|week|2wk|month] [--collections a,b]`
fetches GET /workspaces/{ws}/report and renders a colored summary (totals,
per-bucket throughput, completed-by-collection, status distribution);
--format json prints the raw payload.
- client.GetReport HTTP method.
- MCP: pad_project gains action=report (passThrough to `project report`) with
window + collections params; catalog-readonly test stub + expected maps
updated.
Parent: PLAN-1628.
|
||
|
xarmian
|
5dfc2921b2 |
feat(store): structured status-transition log + backfill (TASK-1637) (#637)
* feat(store): structured status-transition log + backfill (TASK-1637)
Add a status_transitions table capturing every item status change as a
structured, queryable row — written in the same tx as the item update and
never debounced — so the Reports surface (PLAN-1628) can reliably compute
the completed-throughput and cycle-time series.
- migrations/063 + pgmigrations/042: status_transitions table (dual-dialect),
indexed on (workspace_id, created_at) and (item_id, created_at)
- write-path hook in UpdateItemWithPreCheck records from→to on status change
- BackfillStatusTransitions: one-time startup replay parsing the historical
activities.metadata.changes blob (mirrors BackfillWikiLinks), gated on an
empty table; wired into cmd/pad/main.go
- models.StatusTransition + tests (capture, multi-hop, no-op, parser, backfill)
Spike (TASK-1629) found the activity log records status changes only as a
human-readable, debounce-coalesced metadata string — unusable for aggregation.
This is the foundation TASK-1630 (report aggregation) builds on.
* fix(store): record status transitions on item move too per Codex review (round 1)
MoveItemWithPreCheck rewrites fields outside UpdateItemWithPreCheck, so a
status-changing move override (pad item move ... --field status=done) was
not recorded in status_transitions, making the table non-canonical. Insert
the from→to row in the move tx as well, stamped with the target collection.
Adds move-path capture tests (status override + status-preserving move).
* fix(store): make status-transition backfill idempotent per Codex review (round 2)
The empty-table gate isn't atomic, so concurrent replays (a future
multi-replica Postgres deploy; single-instance today) could double-insert
historical rows and overcount reports. Give backfilled rows a deterministic,
activity-derived primary key ("bf_" + activity id) and a dialect-aware
conflict clause (ON CONFLICT DO NOTHING / INSERT OR IGNORE) so a re-run
no-ops instead of duplicating. Count only rows that actually land.
Write-path rows keep using a random newID(), so live data never collides.
* fix(store): accurate from_status under lock + document backfill caveats per Codex review (round 3)
1. from_status was read from the pre-lock `existing` snapshot. When no
precheck ran, a concurrent update (serialized behind the locks we hold)
could make it stale. Capture the status from a fresh in-tx read BEFORE
the UPDATE (reading after would see the new value and drop the hop).
Applied to both UpdateItemWithPreCheck and MoveItemWithPreCheck.
2. Backfill stamps historical rows with the item's current collection_id;
reconstructing the collection at each past status change would require
replaying move history. Documented as a best-effort, historical-only
caveat (exact for the common never-moved case; live write/move paths
stamp the collection at transition time).
* feat(store): track collection done-field + seed create-time transitions per Codex review (round 4)
1. Generalize capture from hard-coded "status" to each collection's done
field (DoneFieldKey: status, or BoardGroupBy field like stage/result for
hiring/interviewing). Add a field_key column recording which field the
row tracks (robust to later BoardGroupBy changes). Applied to update,
move, and backfill paths.
2. Seed a create-time "entered initial status" transition on CreateItem and
in the backfill (Pass 2), so an item created directly in a terminal value
still counts as a completion. Initial value reconstructed from the item's
earliest recorded change, else its current value.
Also: item_id FK is ON DELETE CASCADE so hard-deletes clean up transitions.
Tests cover non-status done-field, create-in-terminal, create-seed, and
cascade-on-delete; full store suite green.
|
||
|
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)
|
||
|
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).
|
||
|
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.
|
||
|
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
|
||
|
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).
|
||
|
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. |
||
|
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. |
||
|
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. |
||
|
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. |
||
|
xarmian
|
0930743304 |
feat: retire IDEA-1 seed pattern + 'pad onboard' cobra + surface blank in init (TASK-1501/1502/1503) (#577)
* feat: retire IDEA-1 seed pattern + 'pad onboard' cobra + surface blank in init (TASK-1501,1502,1503)
PLAN-1496's legacy-onboarding teardown:
TASK-1501 (remove seed items + update banner):
- internal/collections/templates_onboarding.go (and the _product/_scrum
siblings) deleted — these generated the IDEA-1/PLAN-2/TASK-3/DOC-4 +
BACK-1/SPRINT-2/BUG-3/DOC-4 + FEAT-1/FB-2/ROAD-3/DOC-4 first-person
seeds. The /pad onboard playbook (TASK-1499 / TASK-1500) is the
replacement.
- startup/scrum/product templates: SeedItems lines removed.
- post-init banner in printOnboardingHints: now points at "/pad onboard"
in one line, then web UI link, then dashboard hint. The "use pad to
get IDEA-1 / BACK-1 / FEAT-1" branch is gone.
TASK-1502 (retire cobra + OnboardingPrimaryRef plumbing):
- OnboardingPrimaryRef struct field on WorkspaceTemplate removed. The
dashboard's banner auto-discovers seeds via item_number=1 +
source="template" + created_by="system", so the field was redundant
even before retirement.
- onboardingPrimaryRef() helper in cmd/pad/main.go removed.
- 'pad onboard' Cobra subcommand removed (~160 lines). It scanned the
project directory for build/test/CI markers and seeded library
conventions — useful behavior but CLI-only, unreachable from
MCP-only agents. The /pad onboard PLAYBOOK now covers it.
- internal/cli/detect.go and workspace_context_detect.go stay; still
used by the web-side workspace-context save path.
TASK-1503 (Blank in interactive picker):
- The picker already surfaces Blank because templates_picker.go iterates
GroupTemplatesByCategory, and the IDEA-1479 Blank template entry lives
in CategoryCustom. Verified the output renders correctly with the
TASK-1498 description + icon update.
- 'pad workspace init --help' Long now mentions Blank explicitly +
points users at /pad onboard. Helps discoverability without restructuring
the picker.
Test changes (delete or rewrite tests that exercised the retired pattern):
- internal/collections/templates_test.go: six tests deleted (StartupOnboardingItemsOrderAndShape,
ScrumOnboardingItemsOrderAndShape, ProductOnboardingItemsOrderAndShape,
Startup/ScrumProduct/TemplatesDeclareOnboardingPrimaryRef). New
TestSoftwareTemplatesShipNoSeedItems replaces them with the inverse
invariant: software templates ship zero seed items.
- internal/server/handlers_dashboard_test.go: three IDEA-1/BACK-1/FEAT-1
expectation tests collapsed into TestDashboardOnboardingSeed_NilForAllTemplates,
which asserts the auto-discovery finds no seed because seeds no longer
ship. (Hiring + EmptyWorkspace tests untouched — they already expect
nil for unrelated reasons.)
- internal/store/items_test.go: TestSeedCollectionsFromTemplate{Startup,Scrum,Product}RefSequence
and TestOnboardingFlow_FullWalkthrough_{Startup,Scrum,Product} deleted;
these locked the IDEA-1 ref-sequence + walkthrough behavior. Unused
helpers (findItemByTitle, extractStatus, safeFields, setItemStatus,
countItemsInCollection) deleted alongside them.
- internal/mcp/resources_test.go: TestReadItem_PreservesIDEAOneOnboardingBodyVerbatim
→ TestReadItem_PreservesBodyVerbatim. Property is the same (resource
pipeline doesn't mangle markdown), but the fixture is now synthetic
markdown instead of the IDEA-1 seed.
Note: handlers_dashboard.go still has the auto-discovery code path
(onboardingPrimaryCollectionSlugs map + the loop that probes for
item_number=1 + source="template"). It's now dead code — no item
will ever match the criteria after this PR. Left in place for a
follow-up cleanup pass to keep this PR focused.
Parent: PLAN-1496.
* docs: replace 'pad workspace onboard' references with /pad onboard (Codex round 1)
P2 finding on PR #577: README + CLAUDE.md still advertise the
'pad workspace onboard' subcommand in four places (README §Onboard
agents to a new codebase, README §3 Teach your agents the rules,
README CLI Reference, CLAUDE.md CLI). After this branch lands, those
instructions return "unknown command."
Replaced each with guidance pointing at /pad onboard (the playbook,
auto-seeded into every workspace). The library-list commands still
work and stay where they are.
Parent: PLAN-1496.
* docs: replace 'use pad to get IDEA-1' guidance with /pad onboard (Codex round 2)
P1 finding on PR #577: README.md:33-39 and CLAUDE.md:111-117 still
told users to 'use pad to get IDEA-1' after the post-init banner.
Since this branch deletes templates_onboarding.go and stops seeding
IDEA-1/PLAN-2/TASK-3/DOC-4, the quickstart instructions in both
top-level docs pointed at items that no longer exist.
Replaced each with /pad onboard guidance (the playbook is auto-seeded
into every new workspace by TASK-1500). CLAUDE.md's CLI reference
gets a one-line historical note explaining the pre-PLAN-1496 IDEA-1
pattern so readers reviewing older code/blame have context.
Parent: PLAN-1496.
* docs(skill): retire 'use pad to get IDEA-1' guidance in agent skill (Codex round 3)
P1 finding on PR #577: skills/pad/SKILL.md:175 still taught agents
that '"use pad to get IDEA-1"' should dispatch to 'pad item show IDEA-1'.
This branch deletes the seed items, so any agent following the
shipped skill in a fresh workspace would try to fetch a missing ref
instead of running /pad onboard.
Updated the routing entry to dispatch the legacy phrasing (kept as a
recognized intent so older docs/conversations still work) to the
/pad onboard playbook. Explicit "do NOT try to fetch IDEA-1
directly" to short-circuit the previously-trained behavior.
A broader skill cleanup — removing the standalone Onboarding
workflow section and adding the bootstrap nudge rendering — is
TASK-1505's scope. This PR's update is the minimal change needed to
unbreak the agent-facing routing.
Parent: PLAN-1496.
* docs(skill): add library-activation caveat to onboard routing entry (round 4)
P2 finding on PR #577: the routing entry said /pad onboard is
'always invokable because every workspace auto-seeds it.' True for
newly-created workspaces, but pre-existing workspaces (created before
PLAN-1496 lands) won't have it. Auto-upgrade is intentionally not
wired into SeedCollectionsFromTemplate for empty-template-name paths.
Mirrored the same activation-fallback caveat /pad plan and
/pad decompose carry: 'activate via library if the bootstrap's
playbooks array lacks invocation_slug=onboard, status=active.'
Parent: PLAN-1496.
|
||
|
xarmian
|
8c9974f6fb |
feat(cli,mcp): expose 'role update' via CLI and MCP catalog (TASK-1512) (#574)
* feat(cli,mcp): expose 'role update' via CLI and MCP catalog (TASK-1512) Third of three TASK-1497 capability-spike follow-ups (after #572 and #573). The handlers_agent_roles.go::handleUpdateAgentRole PATCH handler and the internal/cli/client.go::UpdateAgentRole HTTP client method already existed. Only the agent-facing surfaces were missing. - cmd/pad: new 'pad role update <slug-or-uuid>' Cobra subcommand with --name / --slug / --description / --icon / --tools / --sort-order flags. Uses cmd.Flags().Changed for omit-if-unset. Positional arg = lookup ref; --slug = new slug value (rename). Empty-string clears for description and icon (the store treats *string("") as "clear", matching collection update semantics). - internal/mcp/catalog_role: new 'update' action + supporting params (new_slug, sort_order). The catalog disambiguates lookup-slug (in path) from rename-target (in body) with the new_slug input, avoiding the conflated-semantics footgun. - internal/mcp/dispatch_http_routes: new mapRoleUpdate mapper. Path uses input.slug for the lookup; body's "slug" key is sourced from input.new_slug. String fields use key-presence semantics so empty-string clears round-trip to the store. - Tests cover canonical body (with AgentRoleUpdate round-trip), new_slug-to-body-slug mapping, empty-string clearing, and required-arg validation. - README.md + internal/mcp/instructions.md pad_role action lists updated to include "update". Pairs with TASK-1510 + TASK-1511 to complete the workspace-mutation trio the /pad onboard playbook (TASK-1499) needs to adapt seeded roles, collections, and schemas to each project's actual shape. Parent: PLAN-1496. * fix(cli,mcp): rename role-update flag --slug → --new-slug (Codex round 1) P1 finding on PR #574: pad_role.update via local stdio MCP was silently broken. BuildCLIArgs translates MCP property "slug" to the CLI's positional <slug> AND to the --slug flag (same key reused), so: pad_role.update slug=<uuid> → pad role update <uuid> --slug <uuid> → tries to rename the role's slug to the literal UUID. BAD. pad_role.update slug=implementer new_slug=engineer → pad role update implementer --slug implementer → new_slug ignored entirely, no rename. The HTTP dispatcher had the disambiguation right (mapRoleUpdate already mapped MCP new_slug → body slug). The CLI flag name was the problem. Renamed --slug to --new-slug. Now MCP "slug" maps to the positional only (lookup), and MCP "new_slug" maps to --new-slug (rename target). Both transports symmetric. Updated example in --help, the liveCmdhelpDoc fake, and the change-detect block. Parent: PLAN-1496, Codex round 1 on PR #574 / TASK-1512. |
||
|
xarmian
|
f76520f6e7 |
feat(cli,mcp): expose 'collection delete' via CLI and MCP catalog (TASK-1511) (#573)
* feat(cli,mcp): expose 'collection delete' via CLI and MCP catalog (TASK-1511)
Mirrors TASK-1510 (collection update). The HTTP handler at
handlers_collections.go::handleDeleteCollection already supported
DELETE on a collection (owner-only, soft-deletes the collection and
every item in it). Wires both agent-facing surfaces:
- internal/cli/client.go: new DeleteCollection client method.
- cmd/pad: new 'pad collection delete <slug>' Cobra subcommand
(no --force; the help text is the confirmation contract).
- internal/mcp/catalog_collection: 'delete' action passes through
to the CLI; tool description updated.
- internal/mcp/dispatch_http_routes: simple routeSpec entry for
DELETE /api/v1/workspaces/{workspace}/collections/{slug}. No
custom mapper needed — no body, no field coercion.
Pairs with TASK-1510 as the second adaptation primitive for the
/pad onboard playbook (TASK-1499): when the onboard interview
discovers a seeded collection that doesn't fit the project, the
agent now has a way to remove it before creating the right one.
Tests:
- TestRouteTable_CollectionDelete (route substitutes correctly)
- catalog_readonly bijection + liveCmdhelpDoc fake updated.
Parent: PLAN-1496.
* docs: correct collection delete contract per Codex review (round 1)
Two findings on PR #573 — both documentation, no code behavior change:
1. CLI Long help / Short blurb / MCP description claimed delete
"removes seeded collections" and the onboard use case targets
template-seeded collections. But store.DeleteCollection refuses
any collection where is_default=true, and every template seed is
is_default=true. The advertised use case wouldn't actually work.
Updated docs to clarify: delete is for USER-CREATED collections;
template seeds must be adapted via 'pad collection update'.
2. Both CLI help and MCP description claimed "AND every item in it"
gets archived. The store delete path only sets collections.deleted_at
and never touches items. The web UI hides them via the join, but
raw API queries still surface them. Updated docs to be honest:
items are NOT cascaded.
Captured the underlying behavior limitation as a follow-up: IDEA-1513
("Lift is_default restriction on collection delete or add a
cascade-items option") — surfaces options 1-4 for lifting the guard
plus the items-orphan issue.
Parent: PLAN-1496, addressing Codex round 1 on PR #573 / TASK-1511.
* docs: tighten collection delete contract per Codex review (round 2)
Three P3 documentation-drift findings:
1. internal/cli/client.go::DeleteCollection Go doc still said "and
all items in it" — missed it in round 1. Updated to describe the
actual behavior (collections.deleted_at only; items orphaned with
soft-deleted collection_id; is_default rejected).
2. CLI Long help and MCP description claimed restore is available
"via the API," but there is no restore endpoint and no
RestoreCollection client method. Recovery is database-backup only.
Both docs updated.
3. catalog_collection.go:33 slug ParamDef only mentioned action=update;
action=delete needs it too. And the headline description still
said "list, create, and update" — three actions when there are
now four. Both fixed.
Parent: PLAN-1496, addressing Codex round 2 on PR #573 / TASK-1511.
* docs: update pad_collection action lists in instructions.md + README (round 3)
Codex round 3 finding: two top-level reference docs still advertised
pad_collection as list/create only. internal/mcp/instructions.md is
embedded into the MCP initialize() handshake instructions — stale
guidance there means MCP clients miss update/delete entirely. README's
catalog table had the same drift.
Parent: PLAN-1496, Codex round 3 on PR #573 / TASK-1511.
|
||
|
xarmian
|
f5579300fb |
feat(cli,mcp): expose 'collection update' via CLI and MCP catalog (TASK-1510) (#572)
* feat(cli,mcp): expose 'collection update' via CLI and MCP catalog (TASK-1510) The HTTP handler at handlers_collections.go::handleUpdateCollection already supported PATCHing a collection's name, icon, description, prefix, schema, settings, and sort_order (plus field-value migrations). The CLI and MCP surfaces never exposed it, so agents couldn't rename collections, swap icons, or reshape schemas — a hard blocker for the adaptive /pad onboard playbook (TASK-1499) which needs to rewrite seeded collections to match each project's actual vocabulary. This wires both agent-facing surfaces to the existing handler: - cmd/pad: new 'pad collection update <slug>' Cobra subcommand with --name / --icon / --description / --prefix / --schema / --fields / --sort-order flags. Only flags explicitly set are sent (uses cmd.Flags().Changed); --schema and --fields reuse the existing collectionSchemaJSONFromFlags helper so DSL parity stays. - internal/mcp/catalog_collection: add 'update' action plus the slug, prefix, and sort_order params on padCollectionTool. - internal/mcp/dispatch_http_routes: new mapCollectionUpdate handles the schema-object-vs-string coercion. The catalog declares schema as a JSON object for MCP ergonomics, but models.CollectionUpdate.Schema is *string — and its UnmarshalJSON only flexes settings, not schema. The mapper re-marshals object input to its JSON-string form before sending, symmetric to what the CLI does via collectionSchemaJSONFromFlags. Tests cover canonical body, schema-object-to-string coercion (round-trip through CollectionUpdate.UnmarshalJSON), schema-string pass-through, empty-field omission, and required-arg validation. catalog_readonly_test bijection + liveCmdhelpDoc fake updated. Parent: PLAN-1496. * fix(mcp): collection update — clear-on-empty + fields DSL parity per Codex review (round 1) Addresses two P2 findings on PR #572: 1. The catalog advertises `icon=""` / `description=""` / `prefix=""` as clear-the-field, and the CLI flag help says the same, but the HTTP mapper filtered empty strings via `v != ""` — leaving MCP HTTP callers unable to clear fields the CLI can. Switched to key-presence semantics for the four string fields so explicit empty strings round-trip to the store (which honors *string("") as "clear"). 2. The catalog advertises `fields OR schema` as mutually exclusive (mirroring `pad collection create`), but the mapper only consumed `schema`. An MCP HTTP request with `fields=...` produced a `{}` PATCH body silently. Extracted the DSL parser to a shared package (internal/collections/dsl.go::ParseFieldsDSL + FieldsDSLToSchemaJSON) so the CLI and the mapper share one parser; mapper now resolves fields-or-schema with the same mutual-exclusion guard the CLI has. Tests added in dispatch_http_routes_extras_test.go: - TestMapCollectionUpdate_EmptyStringClearsField - TestMapCollectionUpdate_AcceptsFieldsDSL (round-trips through models.CollectionSchema to confirm the parsed shape) - TestMapCollectionUpdate_RejectsFieldsAndSchemaTogether cmd/pad/main.go's parseFieldsDSL becomes a one-line alias for collections.ParseFieldsDSL so the CLI's behavior stays identical. Parent: PLAN-1496, fixing PR #572 / TASK-1510. * fix(mcp): collection update — use encodeSchemaForBody + normalize empty schema (round 2) Addresses two more findings from Codex round 2 on PR #572: 1. P2: mapCollectionUpdate bypassed encodeSchemaForBody, so structured schemas didn't get label backfill and string schemas weren't validated before PATCH — diverged from collection create + CLI. Now reuses encodeSchemaForBody (the same encoder collection create uses at dispatch_http_routes.go:418), getting label-backfill via the Title-Case-of-key heuristic and shape validation for free. 2. P3: schema=null or schema="" plus a real fields=... update tripped the mutual-exclusion check. Now normalizes empty inputs as absent BEFORE checking exclusivity, matching the relaxed handling collection create has for optional empty params. Tests: - Renamed TestMapCollectionUpdate_PassesSchemaStringVerbatim to TestMapCollectionUpdate_AcceptsSchemaString — the new property is round-trip parity + label backfill, not verbatim pass-through. - New TestMapCollectionUpdate_EmptySchemaDoesNotBlockFields covers both nil and empty-string schema combined with a real fields value. Parent: PLAN-1496, addressing Codex round 2 on PR #572 / TASK-1510. |
||
|
xarmian
|
e59d3904c9 |
feat(server): refuse to mark item terminal while it has open children (IDEA-1494) (#571)
* feat(server): refuse to mark item terminal while it has open children (IDEA-1494)
Server-side guard inside handleUpdateItem that rejects a non-terminal →
terminal done-field transition when the item still has at least one
non-terminal child. Returns HTTP 409 with code=open_children plus a
structured details payload listing each blocking child's
{ref, title, status, collection_slug} so MCP-driven agents can
self-recover (ship the listed children, then retry) and the CLI can
render the same list verbatim.
Escape hatch: `--force` on `pad item update` / `pad item bulk-update`
and `force: true` on the MCP pad_item.action: update / bulk-update
inputs both forward into the same ItemUpdate.Force transport field
the handler consumes before any store mutation.
Trigger conditions are tight: the PATCH must change the done-field key
(resolved via TerminalValuesForDoneField against the parent's schema +
settings) AND the new value must be terminal AND the current value
must NOT already be terminal. Terminal → terminal and no-op terminal
transitions bypass the guard; only entering the terminal set is gated.
Per-child evaluation uses the child's own collection schema so
hierarchical workspaces with custom typed collections work without
extra plumbing.
Tests cover: rejection with one open child (with mutation-safety
assertion on the parent), no children, all-terminal children, --force
override, no-op terminal → terminal, terminal → terminal,
non-terminal → non-terminal, custom collection terminal_options
honored, and a parent task (not a plan) — IDEA-1494 optional extra #3.
MCP coverage asserts --force round-trips through both ExecDispatcher
and HTTPHandlerDispatcher and is omitted when force=false.
* fix(server): open-children guard round 2 — visibility, MCP pass-through, TOCTOU (IDEA-1494)
Three Codex round-1 issues, each fixed with the recommended shape:
P1 — visibility leak. The 409 response previously listed every blocking
child by ref/title/status, including children in collections the caller
couldn't see. The INVARIANT still evaluates against ALL children (it's a
data-integrity gate — a restricted user must not be able to close a
parent whose blockers they can't see), but the response payload now
filters to caller-visible children only. Hidden blockers surface as a
new `details.hidden_blocker_count` field plus an alternate human message
when every blocker is hidden ("blocked by N open children you don't
have access to"). Mirrors the visibility helpers (`visibleCollectionIDs`
+ `isItemVisibleToGuest`) used by the per-parent progress endpoint so
the two paths can't drift.
P2 — MCP code/details pass-through. The HTTP classifier was collapsing
409 into the generic `conflict` code and dropping `details`; the stdio
classifier was matching the human "cannot " message against the
validation regex and surfacing `validation_failed`. Both now surface
`open_children` with the structured details intact:
- HTTP: classifyHTTPStatusKind's 409 branch extracts the upstream
code; any non-empty, non-"conflict" code is passed through with
its `details` RawMessage. Generalizes beyond open_children — any
future structured 409 from a handler gets the same treatment.
- Stdio: the CLI writes a `pad-error: {json}\n` marker line on
stderr before the human-readable block (single source of truth for
both views), and classifyExecError detects the marker and lifts
the envelope verbatim. Marker is duplicated as a const between
internal/cli and internal/mcp to avoid pulling the cli package
into the classifier just for one string.
A new ErrOpenChildren error code constant + `Details json.RawMessage`
field on ErrorPayload back the wire shape.
P2 — TOCTOU. The guard previously ran in the handler before the store
transaction began; a concurrent child insert / child status flip could
slip between the children-list read and the parent's UPDATE. Fix:
- New `Store.UpdateItemWithPreCheck(id, input, precheck)` runs the
caller's invariant check inside the same tx, after acquiring the
workspace seq lock AND a new parent-children advisory lock keyed
on the parent ID. UpdateItem is now a thin wrapper passing nil.
- Every UpdateItem unconditionally acquires the parent-children
advisory lock for its own parent (if any) AND for itself-as-parent,
in a fixed order (parent first) so two updaters touching the same
parent always grab that key before the more-specific one — no
AB/BA deadlock.
- New `GetChildItemsTx` reads via the caller's tx; on Postgres the
advisory lock provides the snapshot guarantee (DISTINCT precludes
`FOR UPDATE`), on SQLite the global BEGIN IMMEDIATE write lock
serializes all writers.
- Handler now passes a precheck closure into UpdateItemWithPreCheck
at all three call sites (collab-snapshot path, applier-direct-write
path, main path). The guard's openChildrenGuardError sentinel is
unwrapped after each call so the 409 surfaces cleanly.
Tests:
- TestOpenChildrenGuard_VisibilitySanitizesPayload — restricted
editor sees parent + visible child, hidden child contributes to
hidden_blocker_count, no leak of ref/title/slug.
- TestOpenChildrenGuard_AllBlockersHiddenSurfaceGenericMessage —
open_children=[], hidden_blocker_count>0, message mentions "you
don't have access to."
- TestOpenChildrenGuard_TOCTOURace — 8 iterations of a child-flip
racing a parent-terminal update; asserts the forbidden outcome
(parent=completed AND child=open) never occurs.
- TestClassifyHTTPStatus_OpenChildrenPreservesCodeAndDetails +
inverse generic-409 test.
- TestClassifyExecError_OpenChildrenMarkerLiftsStructuredPayload +
no-marker-falls-through inverse.
* fix(server): open-children guard round 3 — 7 Codex findings closed (IDEA-1494)
P1 — visibility fail-closed. The handler was swallowing
visibleCollectionIDs errors, leaving visIDs==nil which the guard
treats as unrestricted, leaking hidden-child metadata. Now surfaces
the error as 500 BEFORE installing the precheck. Test:
TestOpenChildrenGuard_VisibilityLookupErrorFailsClosed closes the
store DB and asserts no 409+children leak.
P1 — link mutations acquire the advisory lock. SetParentLink,
ClearParentLink, CreateItemLink (when link_type ∈ childLinkTypes via
new isChildLinkType helper), DeleteItemLink (same condition), and
RestoreItem now take `pad:parent-children:<id>` in canonical sorted
order via new AcquireParentChildrenLocks helper. SetParentLink locks
BOTH old and new parents (re-parenting case). Race test
TestOpenChildrenGuard_LinkMutationRace asserts the forbidden
"link-committed-before-parent-flip AND parent flip succeeded" never
occurs by comparing link.created_at to parent.updated_at. Documented
semantics: status-wins + link-after-commit is legal under the
invariant "no open children EXIST AT THE MOMENT of transition" —
the post-condition variant ("no open child may EVER attach to a
terminal parent") is intentionally deferred.
P1 — MoveItem bypass closed. New MoveItemWithPreCheck mirrors
UpdateItemWithPreCheck — acquires workspace seq lock + parent-children
locks, re-reads in tx, runs caller precheck. handleMoveItem builds
the same guard closure using the DESTINATION schema for done-field
resolution (conservative — honors the schema the item moves INTO).
CLI gains `pad item move --force`, client gains MoveItemWithForce
that appends `?force=true` to the move endpoint. MCP catalog +
mapItemMove forward `force` through the route mapper. Tests:
TestOpenChildrenGuard_MoveItem_RejectsTerminalWithOpenChildren and
…_ForceOverrides.
P2 — pre-tx field-read TOCTOU. UpdateItemWithPreCheck and
MoveItemWithPreCheck now re-read the item via new getItemTx INSIDE
the tx (after locks) and pass that fresh snapshot to the precheck
closure; the precheck classifies the transition against the in-tx
view, not the handler-side pre-tx capture. Handler precheck closure
swaps `currentFieldsJS` from the in-tx snapshot. Test:
TestOpenChildrenGuard_PrecheckReadsInTxSnapshot stages a between-load
status mutation and asserts the precheck observes the post-mutation
fields.
P2 — bulk-update carries structured errors. cmd/pad/main.go's
updateFailure struct extended with Code + Details
(json.RawMessage). When client.UpdateItem returns *cli.APIError, the
row preserves the structured envelope. Human-text output also
renders the open-children list inline. Chose JSON-envelope route
over per-row stderr markers because bulk-update already produces a
structured envelope and ExecDispatcher returns stdout verbatim on
exit-0 — no classifier change needed. Test:
TestBulkUpdateStructuredFailuresCarryOpenChildrenDetails confirms
the wire shape the CLI lifts.
P3 — marker hardening. Marker bumped to versioned form
`pad-structured-error/v1:` (was `pad-error:`). cli.StructuredErrorMarker
+ mcp.structuredErrorMarker kept in lockstep with cross-references.
mcp.allowedStructuredErrorCodes whitelists known codes (currently
just open_children); unknown codes fall back to regex classification.
Marker must start the line after whitespace trim (embedded markers
ignored). Last-marker-wins to defeat pre-emption attacks. Tests:
TestClassifyExecError_{UnknownStructuredCode,OldMarkerVersion,
MarkerEmbeddedMidLine,LastMarker}.
P3 — soft-deleted collection schemas honored. New GetCollectionAnyState
mirrors childrenDoneFiltersForParent's inclusion rule; guard uses it
so a child still attached to a soft-deleted collection is evaluated
against ITS schema (custom terminal_options) instead of the default-
status fallback (which would mis-classify and false-block). Test:
TestOpenChildrenGuard_SoftDeletedCollectionSchemaHonored seeds a
custom collection, soft-deletes it while a child remains, and
asserts the terminal status is correctly recognized.
Comprehensive store-mutation audit results recorded in the PR
description (every method touching items.fields / items.collection_id
or item_links).
* fix(server): open-children guard round 4 — multi-parent locks, enum parity, PATCH atomicity (IDEA-1494)
Four Codex round-3 (blast-radius lens) findings, each fixed with the
recommended shape.
P1 — multi-parent lock set. acquireParentChildrenLocksForUpdate and
RestoreItem previously used `LIMIT 1` against item_links, so a child
with BOTH a `parent` link to P1 AND an `implements` link to P2 only
locked one of them. The other parent's open-children precheck could
race against the child's status flip and miss it.
Fix: new listParentChildLockKeys helper runs the same query
GetChildItems' inclusion rule uses (childLinkTypes), returns ALL
distinct parent target_ids, and feeds them into the canonical
multi-lock helper. Both UpdateItemWithPreCheck and RestoreItem now
acquire locks on {self} ∪ {all-parents-via-childLinkTypes}. Test:
TestOpenChildrenGuard_MultiParentChildLocksAll races a child status
flip against terminal-updates on both parents simultaneously.
P2 — lock-order asymmetry. The pre-fix codebase had multiple lock-
acquisition shapes: parent-then-self in acquireParentChildrenLocksForUpdate,
single-key in RestoreItem / CreateItemLink / DeleteItemLink /
ClearParentLink, and a sorted multi-key in SetParentLink. Two
concurrent callers using different ad-hoc orderings could AB/BA
deadlock.
Fix: removed the per-call-site AcquireParentChildrenLock helper
entirely. Every site now goes through AcquireParentChildrenLocks
(the canonical sorted multi-lock helper) — including ones that need
only one ID (the variadic call still sorts a one-element slice).
The helper's doc comment explicitly states the contract: "Ad-hoc
single-key acquisition outside this helper is FORBIDDEN — two call
sites taking distinct keys in different orders WILL deadlock."
Test: TestOpenChildrenGuard_NoDeadlockUnderReverseOrderConcurrency
runs reverse-order re-parents with a 5-second timeout; assertion
fails on hang.
P2 — HTTP/stdio code-surface parity. Round 2's HTTP pass-through
("any non-conflict upstream code") silently widened the ErrorCode
enum beyond stdio's allow-list (`open_children` only). Agents saw
different code surfaces depending on which dispatcher delivered
the response.
Fix: HTTP 409 branch in classifyHTTPStatusKind now consults the
same allowedStructuredErrorCodes whitelist stdio does. Codes
outside the set collapse to ErrConflict (no details), matching
what stdio does for an unknown-code structured marker. Doc on
allowedStructuredErrorCodes updated to make the dual-consumer
contract explicit: "Adding a new structured code is a TWO-WAY
change." Tests:
TestClassifyHTTPStatus_UnknownConflictCodeFallsBackToErrConflict
and TestStructuredErrorCodeParityAcrossTransports.
P3 — PATCH atomicity. A combined PATCH with `parent` + `status=terminal`
on an item with open children used to commit the parent-link change
INLINE (before the guard ran) and then reject the field write.
Caller saw 409 but the parent had already moved.
Fix: parent-link mutation is now DEFERRED — captured into outer-
scope vars during fields validation, executed AFTER
UpdateItemWithPreCheck succeeds. A guard rejection returns before
the link write block, so on rejection the link is untouched.
Documented choice: "reorder, don't tx-wrap" — wrapping SetParentLink
into the same store tx would require threading a *sql.Tx through
the SetParentLink API (which is also called from the
handler_item_links path); reordering is the smaller surgery and
gives the correct outcome on the failure direction. A residual
window remains in the OTHER direction (field write commits, link
write fails) — not made worse by the reorder, and called out
inline for a future tx-wrap pass.
Test: TestOpenChildrenGuard_PatchAtomicRejectionPreservesParentLink
sets up target → oldParent → openChild, sends PATCH {parent=newParent,
status=completed}, asserts 409 AND target.parent_link still points
at oldParent.
* fix(server): open-children guard — emit details.open_children as [] not null on hidden-only rejection (IDEA-1494)
|
||
|
xarmian
|
9ebdfb503e |
Revert "feat(cli): pad session shape — Claude Code context-window telemetry (IDEA-1491) (#569)" (#570)
This reverts commit
|