Commit graph

20 commits

Author SHA1 Message Date
minasarustamyan
c6c72b9c00
feat(flea): marketplace refactor — data model, attribution, UI unification (#342)
* feat(flea): phase-1 — title, tagline, synthetic_name columns + upload UX

Schema v49 adds three user-facing metadata columns to store_entities:

- title (NOT NULL) — humanized display name shown on marketplace
  surfaces in later phases. Acronym-aware humanizer in
  src/store_naming.py (27 entries: MCP, API, OAuth, S3, …) shared
  with the frontend via Jinja-injected dict so JS pre-fill and
  Python backfill produce identical output.
- tagline (NULL, ≤200 chars) — optional short description for card
  listings. Long-form `description` stays.
- synthetic_name (NOT NULL) — deterministic `<name>-by-<owner_username>`
  stored as a column for indexing and as the single source of truth
  for attribution lookups in later phases. Today's bundle bake still
  uses suffixed_name() at the same call sites.

Migration (_v48_to_v49_migrate, Python function — humanize has no
SQL equivalent) backfills existing rows: title from
humanize_name(strip_archive_suffix(name)), synthetic from the concat
formula; tagline stays NULL. Idempotent (ADD COLUMN IF NOT EXISTS +
SET NOT NULL no-op on re-run).

Upload form (store_upload.html step 2) reorders fields: Title
(pre-filled from server-side humanize, JS keeps it in sync until
the user edits manually) → Name + dark synthetic preview on one
row (matches marketplace_item_detail.html dark code styling, no
copy button — preview only) → Short description with character
counter → Description (unchanged). Edit form (store_edit.html)
mirrors the layout with pre-filled values from the entity row.

API:

- POST /api/store/entities/preview returns `title` (humanized
  fallback) for upload form pre-fill.
- POST + PUT /api/store/entities accept `title` and `tagline` form
  fields with 100/200-char validation; PUT recomputes
  synthetic_name when `name` changes (caller responsibility per
  repo contract).
- StoreEntityResponse exposes all three new fields.

Repository:

- create() takes title + tagline + synthetic_name as optional
  kwargs with derived defaults (humanize_name(name) / concat) so
  existing test fixtures don't need to thread them.
- update() supports partial updates on all three; tagline empty
  string clears via NULL sentinel.
- archive() recomputes synthetic_name on rename to the archived
  slug so the column stays consistent with name.

Tests:

- New test_schema_v48_to_v49_migration.py: fresh install,
  populated-row backfill (incl. archived row strip), idempotence,
  NOT NULL constraint verification.
- test_store_naming.py: 14 humanize parametrize cases + acronym
  dict invariants.
- test_store_api.py::TestStoreV49Metadata: preview humanize, POST
  with explicit + fallback title, 100/200-char rejects, PUT
  partial update + synthetic recompute on rename.
- Schema version assertion bumps (48 → 49) in test_db_schema_version,
  test_home_stats, test_schema_v42_migration, test_schema_v46_migration.

Phase 1 only — surface rendering on cards / detail pages and
Claude Code bundle propagation come in later phases.

* feat(flea): phase-2 — wire title/tagline/owner through marketplace cards + detail pages

Phase 1 (7f4cfcbb) populated the three new columns on store_entities;
phase 2 surfaces them across the web presentation layer so the kebab-
case slug + bare username no longer leak into user-facing copy.

API:

- `_flea_to_item` now takes `conn` (both callsites updated) and sets
  `display_name=entity.title`, `tagline=entity.tagline`, `owner=
  _resolve_owner_display(conn, owner_user_id, owner_username)` —
  matches the chain the curated path already uses (users.name →
  users.email → fallback). The card JS chain `it.display_name ||
  it.name` then renders the friendly form; `name` stays at the
  suffixed slug as the technical identifier JS uses for fallbacks.
- `flea_detail` adds `display_name` + `tagline` to PluginDetailResponse
  so the standalone skill/agent + plugin detail heroes pick them up
  through the existing `d.display_name` / `d.tagline` chains.
- `_flea_inner_parent_fields` swaps `parent_display_name` from
  `strip_archive_suffix(name)` to `entity.title or strip_archive_suffix(
  name)`. Drives parent-plugin label in four surfaces at once:
  breadcrumb 3rd segment, hero "part of <plugin>" meta-row,
  helper "This skill is part of <plugin>" panel, and the Details
  sidebar's "Parent plugin" row.

Templates — `marketplace_item_detail.html`:

- Pre-render: browser title, hero h1, and hero-window-label read
  `(entity.title if entity else None) or inner_name or item_name or
  plugin_name` so the SSR shell shows the friendly title before the
  JS fetch lands (no flash of kebab-case).
- Breadcrumb last segment for flea standalone drops the `d.manifest_name
  || heroTitle` fallback in favour of just `heroTitle` — manifest_name
  is the suffixed slug and users explicitly didn't want it in the path.
- Hero meta-row for flea standalone is now hidden. The prior "by
  <author> · N installed · <size>" line duplicated install count
  (hero telemetry chip below), owner + bundle size (Details sidebar).

Templates — `marketplace_plugin_detail.html`:

- Same SSR pre-render swap (title, h1, window-label, crumb-name).
- Hero tagline element starts hidden; JS shows it only when
  `d.tagline` is truthy. Pre-fix it fell back to `d.description`
  (long-form text), which read awkwardly under the h1 and pulled the
  hero too tall. Description still renders in the "What it does"
  panel below the hero.
- Initial "Loading…" placeholder removed so entities without a
  tagline don't flash that text mid-fetch.

Tests:

- New `TestFleaPhase2Presentation` class in test_marketplace_api.py
  (6 cases): card title + tagline + full-name owner, owner fallback
  chain when users.name is NULL, flea_detail exposes title + tagline,
  tagline null when omitted, inner skill parent_display_name uses
  entity.title (explicit + humanize-fallback variants).
- Updated `TestListItems.test_flea_lists_uploads` to assert both
  `display_name == "Alpha"` (humanized) and `name ==
  "alpha-by-alice"` (suffixed slug compat).
- Updated `TestWebPages.test_marketplace_flea_detail_page_renders`
  to look for the humanized title ("Page Skill") in the SSR shell
  instead of the kebab-case `page-skill`.

* feat(flea): phase-3 — read synthetic_name from DB, suffixed_name() only on write

Phase 1 added the column + backfill, repo write paths keep it in sync.
Phase 3 routes every READ callsite through `store_entities.synthetic_name`
directly instead of recomputing `<name>-by-<owner_username>` on the fly,
and switches the collision query off the inline string concat. The
`suffixed_name()` primitive now lives exclusively in write flows.

Read callsites updated (all read `entity["synthetic_name"]` directly,
no fallback — the column is NOT NULL and a missing value would be a
real bug worth surfacing as KeyError):

- app/api/marketplace.py:_flea_to_item — card MarketplaceItem.name.
- app/api/marketplace.py:flea_detail — PluginDetailResponse.manifest_name.
- app/api/store.py:_entity_to_response — StoreEntityResponse.invocation_name.
- app/api/store.py PUT bundle re-bake — `suffixed` passed to
  `_bake_plugin_tree`; entity is loaded pre-rename, so its
  synthetic_name is the OLD value `_bake_plugin_tree` expects.
- app/api/store.py PUT rename — `old_suffix` for `_rename_baked_tree`.
- app/api/my_stack.py — StoreInstallEntry.invocation_name.
- src/marketplace_filter.py — manifest_name in served plugin entry.

`suffixed_name` imports removed from marketplace.py, my_stack.py, and
marketplace_filter.py (no remaining callsites). store.py keeps the
import for its write paths:

- POST create (`suffixed = suffixed_name(final_name, username)` →
  passed to `_bake_plugin_tree` and `repo.create(synthetic_name=...)`).
- PUT rename collision check (`new_suffixed`).
- PUT rename `new_suffix` for `_rename_baked_tree` (proposed value).
- PUT rename `new_synthetic` for `repo.update(synthetic_name=...)`.
- Archive `old_suffix` + `new_suffix` for `_rename_baked_tree`
  (retro-compute pre-archive value after `repo.archive` already
  overwrote the DB row with the post-archive synthetic).

Collision SQL — `_suffixed_already_taken`:

  WHERE name || '-by-' || owner_username = ?   (before)
  WHERE synthetic_name = ?                     (after)

Same matches today (phase 1 backfill + NOT NULL invariant + write
paths in sync); indexable + single source of truth going forward.

Repository:

- UserStoreInstallsRepository.list_for_user explicit SELECT extended
  with `se.title`, `se.tagline`, `se.synthetic_name` so my_stack and
  marketplace_filter callers can read them off the joined row.

Tests:

- test_store_api.py::test_invocation_name_reads_from_synthetic_column —
  upload entity, manually override the column with a non-canonical
  value, verify GET response returns the override (proves read path
  consumes the column, not recomputes).
- test_marketplace_api.py::test_flea_card_and_detail_read_synthetic_name_from_db —
  same proof for `MarketplaceItem.name` (card) and
  `PluginDetailResponse.manifest_name` (detail).

* feat(flea): phase-4 — rename agnes-store-bundle → flea (synthetic plugin)

The synthetic plugin that wraps loose flea-market skills + agents into
one Claude Code plugin is renamed from `agnes-store-bundle` to `flea`.
Plugin-type flea uploads (their own standalone plugin entry) are
unaffected.

Constants:
- src/marketplace_filter.py:
  - BUNDLE_PLUGIN_NAME: "agnes-store-bundle" → "flea"  (Claude Code
    plugin manifest name + .claude-plugin/plugin.json name)
  - BUNDLE_PREFIXED_NAME: "store-bundle" → "flea"      (on-disk ZIP /
    git tree path, now plugins/flea/...)

Attribution layer (services/session_processors/usage_lib.py):
- FLEA_BUNDLE_PREFIX: "agnes-store-bundle" → "flea". The JSONL
  invocation identifier going forward is `flea:<skill-name>`.
- New `_LEGACY_FLEA_BUNDLE_PREFIXES = ("agnes-store-bundle",)`.
  `MarketplaceItemLookup.resolve()` + `_attribute_event()` accept BOTH
  the new and the legacy prefix so historic usage_events (~90-day
  retention) continue attributing to source='flea'. The tuple becomes
  a no-op once the rename has been live past the retention window —
  a follow-up commit can drop it then.
- USAGE_PROCESSOR_VERSION bumped 6 → 7 so the session-pipeline reprocess
  loop re-runs attribution with the new + legacy prefix branches.

User-facing copy:
- /api/store/bundle.zip Content-Disposition filename: agnes-store-bundle.zip → flea.zip
- `agnes admin store pull` default --out: agnes-store-bundle.zip → flea.zip
- Docstrings + JS comment + welcome template comment updated.

Tests:
- skill_flea.jsonl fixture identifier updated to flea:flea-skill.
- New skill_flea_legacy.jsonl with the legacy prefix for backward-compat
  coverage.
- New test `test_legacy_agnes_store_bundle_prefix_resolves` replays the
  legacy fixture and asserts source='flea' attribution still lands.
- All other test assertions / mocks substituted mechanically:
  test_session_processor_usage.py, test_usage_rollups.py,
  test_marketplace_filter_store.py, test_store_api.py,
  test_cli_refresh_marketplace.py.
- `_seed_flea_entity` (test_usage_rollups.py) + `_seed_attribution`
  (test_session_processor_usage.py) helpers now supply the NOT NULL
  `title` + `synthetic_name` columns from phase 1, since they INSERT
  directly bypassing the repo's create() fallback.

Client rollover note (CHANGELOG): `agnes refresh-marketplace` will
install the new `flea@agnes` plugin and the local marketplace clone's
`plugins/store-bundle/` source folder is removed via `git reset --hard`.
Whether Claude Code itself auto-prunes the orphan `agnes-store-bundle
@agnes` registry entry is undocumented — to verify empirically on the
dev VM. If the orphan entry lingers, a follow-up will add targeted
cleanup; until then users can manually run
`claude plugin uninstall agnes-store-bundle@agnes`.

Verified locally: 98 passed (session_processor_usage + usage_rollups +
marketplace_filter_store + cli_refresh_marketplace) + 228 passed/2
skipped (store_api + marketplace_api + admin_store_submissions +
store_entity_versions + store_repositories).

* fix(flea): phase-5 — attribution keyspace mismatch (closes #335)

Pre-fix every flea skill/agent invocation silently fell through to
`usage_events.source = 'builtin'`. Root cause: lookup tables in
`services/session_processors/usage_lib.py` keyed `_flea_entities` (and
the derived `_flea_plugins` set) by `store_entities.name` — the
un-suffixed display name. Claude Code writes invocations as
`flea:<synthetic_name>` (e.g. `flea:xlsx-by-c-marustamyan`), so
`dict.get(local)` always missed and the resolver fell through to
builtin. Result: marketplace cards, detail telemetry chips, admin
group-by-source all showed 0 flea invocations even when the raw
JSONL stream was correct.

Phase 1 added the `synthetic_name` column + backfill; phase 4 renamed
the bundle prefix to `flea`; phase 5 finally flips the lookup
keyspace to match what JSONL writes.

usage_lib.py:
- `MarketplaceItemLookup.__init__` preload: `SELECT synthetic_name,
  type FROM store_entities` (was `SELECT name, type`). `_flea_plugins`
  set derived from those keys, so it now carries synthetic_names
  too — matches what Claude Code writes when invoking a skill nested
  inside a flea plugin (`<synthetic>:<inner>`).
- `rebuild_rollups` preload: same SELECT change; also derives
  `flea_plugins` and threads it through `_aggregate_events` /
  `_rebuild_window`.
- `_attribute_event`: signature extended with `flea_plugins`; new
  branch `if prefix in flea_plugins: return ("flea", default_type,
  prefix, local)` for flea-plugin-nested skills/agents. This branch
  was added to `MarketplaceItemLookup.resolve()` in v6 (commit
  e076ebbe) but the rollup builder's helper was never updated to
  match, so nested skills inside flea plugins silently dropped out
  of the daily/window fact tables.
- `USAGE_PROCESSOR_VERSION`: 7 → 8. Forces the session-pipeline
  reprocess loop to re-attribute existing usage_events rows with
  the corrected lookup so rollup tables fill correctly on the next
  tick.

marketplace.py — 4 API stats lookup callsites switched from
`entity["name"]` to `entity["synthetic_name"]`:
- `_flea_to_item` (card stats lookup)
- `flea_detail` (`_build_telemetry` + `_load_inner_items_stats_by_parent`)
- `flea_skill_detail` (inner detail `parent_plugin` key)
- `flea_agent_detail` (inner detail `parent_plugin` key)

Tests:
- `skill_flea.jsonl` invocation: `flea:flea-skill` →
  `flea:flea-skill-by-alice` (mirrors what Claude Code writes after
  phase 1/4 — the suffixed synthetic_name).
- `test_flea_skill_attributed_with_empty_parent` assertion: rollup
  `name` column now carries the synthetic_name.

No legacy `agnes-store-bundle` prefix backward compat — clean cut per
user direction (dev phase, no production data worth preserving).

Verified locally: 53 passed targeted (session_processor_usage +
usage_rollups + marketplace_filter_store) + 215 passed/2 skipped
broader (store_api + marketplace_api + admin_store_submissions +
store_entity_versions).

* fix(flea): phase-6 — plugin-level rollup aggregation parity for flea

Flea plugin entity cards + detail pages showed 0 invocations even
though nested skills had correct rollup rows. Root cause: the
plugin-level aggregation pass in `_aggregate_events` was hardcoded
to `source='curated'` only:

    if source != "curated" or not parent:
        continue
    if group_by_day:
        pkey = (day, "curated", "plugin", "", parent)
    else:
        pkey = ("curated", "plugin", "", parent)

So flea plugin entities never got a synthetic
`(source='flea', type='plugin', parent_plugin='', name=<synth>)`
row aggregating nested invocations. `_load_invocation_stats('flea')`
filters `parent_plugin = ''` and returned no row for flea plugin
entity cards, so `stats.get(entity["synthetic_name"])` missed and
the API exposed 0/0.

Triggered by empirical observation on the dev VM —
`codex-second-opinion-by-c-marustamyan` plugin showed 0 calls in
the listing card while its three inner skills (codex-setup ×3,
codex-review ×1, codex-second-opinion ×1) had the expected child
rollup rows.

Fix:

- Extend the guard to `source in ("curated", "flea")`.
- Replace the hardcoded `"curated"` in the `pkey` tuple with the
  loop's `source` variable, so flea aggregation lands as `source=
  'flea'` and curated aggregation continues landing as
  `source='curated'`.

API path unchanged — `_load_invocation_stats('flea')` filters
`parent_plugin = ''` already picks up the new aggregated row
alongside standalone skill/agent rows. Rollup `name` field carries
the synthetic_name keyspace; no collision between standalone entity
synthetic and plugin entity synthetic (global suffix uniqueness
enforced by `_suffixed_already_taken`).

`USAGE_PROCESSOR_VERSION` bumped 8 → 9 to force a reprocess pass so
historic nested-invocation data fills the new plugin-level rows on
the next tick (instead of waiting for the next live invocation).

Tests:

- New `test_flea_plugin_row_aggregates_children` mirrors the existing
  `test_curated_plugin_row_aggregates_children`: seeds a flea plugin
  entity, three nested events (one user invoking two skills, a
  second user invoking one) → asserts the aggregated plugin row
  carries count=3, distinct_users=2 (union, not sum), plus the child
  rows survive alongside.

Verified locally: 43 passed (session_processor_usage + usage_rollups)
+ 82 passed/2 skipped broader (+ marketplace_filter_store +
marketplace_api).

* refactor(marketplace): phase-7 — unify Details sidebar across detail surfaces

Five marketplace detail surfaces (curated plugin, flea plugin, curated
inner skill/agent, flea inner skill/agent, flea standalone skill/agent)
had drifted on which Details rows they show and what order — the same
field landed in different positions, some fields duplicated hero info,
and the flea plugin Owner row leaked the kebab-case `owner_username`
slug instead of the user's real name. This commit aligns all five
surfaces on a single scan order driven by UX priority:

  identity → life-stage → telemetry → debug-tier

Concretely:

  1. Curator / Owner          (first scan signal — trust)
  2. Parent plugin            (inner skill/agent only)
  3. Released                 (top-level only — plugins + flea standalone)
  4. Last used                (recency)
  5. Active days              (engagement consistency)
  6. Version                  (flea standalone only — content hash)
  7. Bundle size              (debug-tier)

Dropped:

  - Slug field on plugin detail surfaces (`marketplace_id` for curated,
    `entity_id` for flea). Pure debug info, never user-relevant; URL
    already carries it.
  - Category + Installs on flea standalone skill/agent detail.
    Category is already shown as a hero badge; install count is in
    the hero telemetry chip — sidebar duplication added noise.

Owner display:

  - Flea plugin Owner row now reads `d.owner_display` (resolved through
    `users.name → users.email → owner_username` by `_resolve_owner_display`
    in `app/api/marketplace.py:1491`) instead of the raw `d.author_name`
    (which is `owner_username`, the kebab-case slug). API field already
    populated from phase 2; templates just consume it.
  - Curated Curator row continues to read `d.author_name` from
    marketplace-metadata.json; `owner_todo` placeholder behavior
    preserved.

Files:

  - app/web/templates/marketplace_plugin_detail.html — rewrote the
    Details render loop (lines 1364-1427 area). Slug row removed,
    rows reordered, Owner branch reads `d.owner_display`.
  - app/web/templates/marketplace_item_detail.html — both branches of
    the Details sidebar (inner skill/agent + flea standalone) re-laid
    around the same scan order. Telemetry helper unchanged, just
    repositioned. Category + Installs rows removed from the
    standalone branch.

No new tests — no existing test asserts the precise order of Details
rows or references the dropped fields in a sidebar context (grep
confirmed). API surface unchanged.

Verified locally: 84 passed / 2 skipped on `test_marketplace_api.py`
+ `test_store_api.py`.

* fix(flea): post-review hardening — N+1, v50 UNIQUE, docs, test cleanup

Addresses 5 critical findings from PR #342 code review:

1. N+1 query in `_flea_to_item` — owner-display resolution previously
   ran one `SELECT … FROM users WHERE id = ?` per item in the listing
   comprehension. Now batched via `_load_users_display` IN-query
   prefetch; 50 items drops 51 user queries to 2. Regression-guarded
   by `TestFleaOwnerDisplayBatched` (spies `_resolve_owner_display`
   and asserts it's not called inside the list path).

2. Misleading comment in `src/marketplace_filter.py` claimed the
   attribution layer accepts both `agnes-store-bundle` and `flea`
   prefixes — it doesn't (clean cut per CHANGELOG). Rewrote to match
   reality.

3. CHANGELOG `[Unreleased]` had two `### Changed` blocks. Merged into
   one (BREAKING bullet first).

4. New v49→v50 migration adds `UNIQUE INDEX
   idx_store_entities_synthetic_name`. v49 made `synthetic_name` the
   canonical attribution key but uniqueness was only app-enforced;
   v50 promotes the invariant to the DB layer. Migration pre-checks
   for existing duplicates and raises `RuntimeError` listing them
   rather than letting `CREATE UNIQUE INDEX` fail mid-way. v48→v49
   migration gained an `is_nullable='YES'` guard on its `SET NOT NULL`
   ALTERs so re-runs on a fully-migrated DB don't trip DuckDB's
   "cannot alter entry … entries depend on it" block (the new index
   counts as such an entry). Index is created by the migration only —
   keeping it out of `_SYSTEM_SCHEMA` preserves fresh-install ordering
   (CREATE TABLE → v49 ALTERs → v50 CREATE INDEX).

5. Deleted three redundant version-pinned schema asserts whose names
   lied about their bodies (`test_schema_version_is_42` asserting
   `== 49`, etc.). Canonical assert lives in
   `test_db_schema_version.py`, renamed to
   `test_schema_version_matches_constant`.

* fix(db): gate v34→v38 store_entities ALTER COLUMN steps on column state

CI on Linux failed `test_v17_to_v18_drops_*` after the v50 UNIQUE INDEX
landed. Root cause: those tests open a DB at the full target version,
seed fixtures, then reset `schema_version` to 17 and reopen — forcing
the ladder to re-run from 17 → current. With the v50 index now in place,
DuckDB blocks intermediate `ALTER COLUMN` steps on `store_entities`
("Cannot drop this column: an index depends on a column after it!" /
"Cannot alter entry because there are entries that depend on it"),
because `synthetic_name` (the indexed column) sits positionally after
the columns those steps touch.

Fix: convert the three SQL-list migrations that hit store_entities into
defensive Python functions:

- `_v34_to_v35_migrate` short-circuits when `synthetic_name` already
  exists (post-v49 shape — the visibility_status rebuild is moot and
  the DROP COLUMN would be blocked by the index).
- `_v35_to_v36_migrate` gates the `visibility_status SET NOT NULL` +
  `SET DEFAULT` on `is_nullable='YES'` so it's a true no-op when the
  column is already constrained.
- `_v37_to_v38_migrate` gates the `version_no SET NOT NULL` step the
  same way.

Forward-roll path (real installs that never reset schema_version) is
unchanged: the gates fire `YES` → ALTERs run. The fix only changes
behavior for the "DB is already at v50 shape but version row says 17"
scenario the tests construct.

---------

Co-authored-by: Minas Arustamyan <arustamyan.minas@gmail.com>
2026-05-19 02:32:41 +02:00
Vojtech
c552bf8243
feat(api): enforce API design rules via pytest + fix DELETE/status-code violations (#338)
* feat(api): enforce API design rules via pytest + fix DELETE/status-code violations

Adds tests/test_api_design_rules.py with four forward-only design guardrails
that prevent new endpoints from accumulating REST debt:

  Rule 1 — No new verbs in URL paths (existing 28 grandfathered via allowlist)
  Rule 2 — DELETE must declare 204 No Content (zero allowlist entries)
  Rule 3 — Creator POSTs (path has GET counterpart) must declare 201/202
  Rule 4 — All protected /api/* routes must declare 401 and 403

Fixes found by running the rules:

- DELETE /api/admin/metrics/{metric_id}: return 204, drop redundant body
- DELETE /api/memory/{item_id}/dismiss (undismiss): return 204, drop body
- POST /api/memory/admin/contradictions: add status_code=201 (creates a resource)
- app/main.py: _add_auth_error_responses() injected into app.openapi() at startup;
  declares 401/403 on all protected /api/* operations centrally, fixing the 120
  routes that previously omitted these response codes from the spec.

Closes #337

* fix(api): resolve CI failures — extend 204 fixes + complete allowlists

- Fix remaining 6 DELETE endpoints to return 204: store entities,
  store entity install, marketplace curated install, marketplace plugin
  system flag, admin store submission, and observability view
- Update all affected tests to expect 204 (removed body assertions)
- Add 4 missing verb paths to _VERB_PATH_ALLOWLIST in test_api_design_rules.py
- Add 2 upsert endpoints to _CREATOR_POST_ALLOWLIST
- Update admin_marketplaces.html to not call r.json() on 204 DELETE

* fix(tests): align 2 DELETE-asserting tests with 204 contract (post-#339 rebase)

CI's test-shard (1) and (4) failures on this PR were caused by
Vojta's second commit (`fix(api): resolve CI failures — extend 204
fixes`) flipping more DELETE endpoints to status_code=204 than just
the two mentioned in the PR body. Two tests assert status_code==200
on the DELETE response and broke:

- tests/test_admin_store_submissions.py::TestQuarantineGates::test_admin_can_delete_quarantined
  (DELETE /api/store/entities/{entity_id})
- tests/test_store_api.py::TestInstallCycle::test_admin_hard_delete_cascades_installs
  (DELETE /api/store/entities/{entity_id}?hard=true)

Updated both to assert 204 with a comment pointing at
tests/test_api_design_rules.py rule 2 so future reviewers can
trace the contract. Verified via broader scan that no other test
asserts == 200 on a .delete() response directly (4 other sites do
.delete() then check 200 on a subsequent GET — those are fine).

* release: 0.54.26 — API design rules (test_api_design_rules.py) + 8 DELETE endpoints flip to 204

---------

Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
2026-05-18 15:25:07 +02:00
Vojtech
cd03028776
fix(store): restore reuses prior approved verdict + admin detail surfaces content_quality (#332)
* fix(store): restore reuses prior approved verdict; admin detail surfaces content_quality

Live bug on agnes-development: entity 6ba2ee1d…'s v5 submission (third
restore of v1, byte-identical to v1/v2/v4/v6) landed `blocked_llm`
while the other identical-hash siblings landed `approved`. Anthropic
structured output is non-deterministic — same bytes flipped
`content_quality.verdict` pass↔fail across calls. Admin detail page
made the failure look mysterious: only security-findings table
rendered, so a content-quality-only block showed up as
"No findings — model verdict was clean".

Two fixes:

1. Restore endpoint reuses a prior `approved` submission's verdict
   when the restored bundle hash matches an existing history entry
   AND `reviewed_by_model` matches. Skips the LLM call, stamps the
   new submission with the prior verdict + `reused_from_submission_id`
   marker. Deterministic + saves Anthropic tokens. Gated on
   schedule_async_llm so guardrails-off keeps its existing path.

2. Admin detail template now renders `content_quality.issues` in its
   own table + adds an explicit "Blocked but no findings recorded"
   notice for the transient-non-determinism case + surfaces the
   reuse marker when present.

Reuse falls back to a real LLM call when:
- prior submission's reviewed_by_model doesn't match current (admin
  upgraded tier Haiku → Sonnet → Opus)
- prior submission was guardrails-off (no reviewed_by_model)
- no history entry has matching hash

Tests:
- TestRestoreReusesApprovedVerdict::test_restore_of_approved_version_skips_llm_and_reuses_verdict
- TestRestoreReusesApprovedVerdict::test_restore_legacy_v1_falls_back_to_llm

* fix(store): admin detail v# by submission_id + version switcher

Three related fixes surfaced live by a user inspecting submission
47bbc1f5… on localhost where v# rendered as v1 even though current
was v10.

1. Admin queue + admin detail derive submission v# by submission_id
   instead of hash. Pre-fix the loop matched first hash-equal entry
   in version_history — always v1 when bundles were byte-identical
   (which is the common case after the restore-reuse path). Two
   call sites updated:
   - `src/repositories/store_submissions.py:list_for_admin` (queue
     v# column)
   - `app/web/router.py:admin_store_submission_detail_page` (detail
     page v# chip on each section header)

   Same fix pattern as PR #330 for runner / override.

2. New version-switcher card on admin detail page lists every
   submission linked to the entity with status + reviewed_by_model +
   click-to-jump. Solves the user's secondary ask ("should be a way
   to switch different versions on the submission detail").

3. Initial POST now backfills the v1 seed entry's submission_id
   right after creating the v1 submission. The helper
   `update_history_submission_id` existed but no production code
   path called it — so v1 always had submission_id=None and every
   "find v# for submission" lookup silently failed for v1.

171 tests green on touched surface.

* release: 0.54.24 — restore reuses prior approved verdict + admin detail content_quality + v# by submission_id (Codex/Live follow-up to #330/#331)

---------

Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
2026-05-16 07:12:29 +02:00
minasarustamyan
302cf58ccd
feat(marketplace): telemetry v46 + flea inner parity + listing polish (#329)
* feat(telemetry): marketplace item rollup refactor (schema v46)

Replace the v42 attribution layer with prefix-split + live lookup against
marketplace_plugins / store_entities. The v42 design had a latent bug —
AttributionLookup keyed on bare skill names while Claude Code writes
`<plugin>:<local>` in JSONL, so lookups never matched and
usage_plugin_daily stayed empty in every deployment.

Schema (v46 migration):
- Drop usage_attribution_skills / _agents / _commands (mapping tables,
  derivable from marketplace_plugins + plugin tree).
- Drop usage_plugin_daily (always empty in production due to the bug above).
- Create usage_marketplace_item_daily — per-day fact (count, distinct_users,
  error_count), composite PK on (day, source, type, parent_plugin, name).
- Create usage_marketplace_item_window — sliding-window snapshot with
  true cross-window distinct user counts; period_label='last_7d' refreshes
  every tick, 'last_30d' refreshes hourly (tracked via session_processor_state).
- Mark usage_tool_daily as candidate for removal (no product-UI consumer).

Attribution flow:
- MarketplaceItemLookup replaces AttributionLookup. Preloads
  marketplace_plugins.name + store_entities.name into memory once per
  UsageProcessor tick, then per-event splits identifier on ':',
  matches prefix, writes resolved source / parent_plugin into
  usage_events. agnes-store-bundle prefix routes to flea entities.
  Slash commands with `plugin:` prefix count as type='skill' in rollup.

API:
- BREAKING: MarketplaceItem.unique_users_30d renamed to distinct_users_30d
  (now a true distinct count from the window snapshot, not sum-of-daily).
- InnerDetailResponse gains a telemetry field — invocations_30d +
  distinct_users_30d surfaced on curated inner skill / agent detail pages.
- Card chip hidden pending UX finalisation; data stays in the response.

Backfill: scripts/backfill_marketplace_rollup.py — one-shot rebuild over
historic usage_events after deploy, idempotent.

USAGE_PROCESSOR_VERSION bumped 4 → 5 so the reprocess loop re-attributes
existing events to the new source/ref_id semantics on the next tick.

Tests rewritten: test_session_processor_usage, test_usage_rollups,
test_marketplace_telemetry, test_api_admin_usage_reprocess,
test_db_schema_version, test_home_stats, test_schema_v42_migration.
New: test_backfill_marketplace_rollup.

* fix(marketplace): refresh Most Popular on search + category changes

`loadMostPopular()` early-exits when `state.q` or `state.category` is
set, but the search + category handlers only called `loadItems()` —
so once the section was visible, typing a query or filtering by
category didn't re-run the hide check and the cards stayed on screen
out of scope. Tab + sort handlers already chained the call.

Add the call to runSearch + category pill click handlers (All +
per-category) so the visibility contract holds for every state
mutation that can flip the early-exit condition.

* feat(marketplace): All-plugins section + 7-day Most Popular

Listing layout:
- Always-visible "All plugins" / "All items" / "Your stack" section
  header (label swaps per tab) wrapped in `#mp-all-section` so its
  margin-collapse mirrors the sibling `#mp-popular-section` and the
  spacing from the filter row stays consistent in both layouts.
- Sort dropdown moved from the filter row into the All-* header,
  pinned right via `margin-left: auto`. Anchored to its section so
  the relationship between sort + grid is obvious.
- `.mp-section-header` gets `min-height: 32px` + `align-items: center`
  so the bare-text Most Popular row matches the dropdown-bearing
  All-* row.
- `.mp-section-header` margin tightened 24px → 20px on top.

Most Popular:
- Capacity reduced 8 → 4 cards.
- Now reflects a 7-day window (was 30-day). Backend surfaces
  `invocations_7d` + `distinct_users_7d` on `MarketplaceItem`
  alongside the existing 30d fields; the loader pulls a wider page
  (server still sorts by 30d) and re-sorts + filters client-side
  on `invocations_7d > 0` so the strip stays "hot right now".
- Section label updated to "Last 7 days".
- Section now renders on both `curated` and `flea` tabs (was
  curated-only). Hidden on `my` and whenever search / category
  filter is active. Refresh hooks wired into search + category
  click handlers so visibility flips immediately on state change.

Backend (`_load_invocation_stats`):
- Single SELECT pulls both `last_30d` and `last_7d` rows from
  `usage_marketplace_item_window`; the result dict carries
  invocations + distinct_users for both windows.
- Trend (recent_7 vs prior_7) kept on the daily fact table so it
  stays independent of the window snapshot's freshness.

* feat(marketplace): Most adopted sort + hide Trending when no trend data

Add a fourth sort option to the All-items dropdown — "Most adopted
(30d)", keyed on `MarketplaceItem.distinct_users_30d` (true 30d
distinct user count from `usage_marketplace_item_window`). Protects
the listing from power-user skew that `most_used` is susceptible to:
one user × 100 invokes can't beat 10 different users × 1 invoke
under adoption sort.

Hide Trending option when the response has no trend data. User
reported `sort=trending` returning an empty grid because every
plugin's `trend_pct` was None (prior-week threshold of >= 3
invocations didn't clear anywhere). Empty grids on a user-selected
sort are worse UX than just not offering the sort — surface what
works, hide what doesn't.

Backend (`app/api/marketplace.py`):
- `_apply_sort` gains a `most_adopted` branch (DESC distinct_users_30d,
  ties by name ASC).
- `sort` Literal extended.
- `ItemListResponse.available_sorts` lists the sort keys the UI
  should expose for this response. recent/most_used/most_adopted
  always; trending only when at least one item in the tab's stats
  carries a non-null trend_pct.
- `_available_sorts(stats_dicts)` helper centralises the rule —
  curated and flea branches pass one stats dict, my-tab passes both
  (option is available when either source has trend data).

Frontend (`app/web/templates/marketplace.html`):
- New `<option value="most_adopted">Most adopted (30d)</option>`
  between Most used and Trending.
- URL state allowlist extended so `?sort=most_adopted` round-trips.
- `applyAvailableSorts(available)` runs after each list fetch:
  hides options not in the response's available_sorts; if the user
  is on a now-unavailable sort, resets to 'recent' and re-fetches.
  Search-mode fan-out unions availability across the curated + flea
  responses so a hit on either side keeps the option visible.

* feat(marketplace): funnel chip on cards + deterministic Most Popular sort

Card chip — funnel telemetry between description and footer:

  [stack-icon] N installed · [user-icon] N active · [bolt-icon] N calls · ↑/↓ N%

- stack_count (new MarketplaceItem field): for curated it's COUNT(*)
  on user_plugin_optouts (post-v28 row PRESENCE = subscribed; system
  plugins are fanned out to every user via fanout_system_for_user so
  the count includes them naturally). For flea it reuses the existing
  store_entities.install_count (bumped on install/uninstall).
- distinct_users_30d (existing) — active users in the 30d window.
- invocations_30d (existing) — call volume.
- trend_pct (existing) — week-over-week, both directions: green ↑ /
  red ↓, magnitude only (sign in the arrow). Hidden when null.

Backend additions in app/api/marketplace.py:
- MarketplaceItem.stack_count field.
- _load_curated_stack_counts() — one SELECT per render, GROUP BY
  (marketplace_id, plugin_name). Wired into the curated + my-tab
  branches; flea reads install_count off the entity row directly.

Frontend (app/web/templates/marketplace.html):
- Heroicons solid 24×24 inlined (one helper per icon, all
  fill="currentColor" so per-segment colour tokens apply): rectangle-
  stack (mirrors the My Stack tab icon), user, bolt, arrow-trending-
  up/down.
- Per-segment colour: installed=amber #F59F0A (My Stack accent),
  active=green #0e9b6a, calls=orange #f97316. Text stays neutral so
  the chip still reads as metadata, the leading glyph carries the
  visual cue. Trend pill keeps the full-segment green/red colour.
- Zero state: chip hidden when stack_count == 0 AND invocations_30d
  == 0 — brand-new cards aren't visually penalised by a "0·0·0" row.
- Tooltips on every segment via title="…" so hover explains the
  number's meaning to anyone uncertain about the icon.

Most Popular section — deterministic ordering:

Previously sorted by invocations_7d DESC with no tie-breakers, so
several cards with identical 7d call counts would swap places on
refresh (JS stable sort fell back on backend order, and the backend's
own tie-breaker for `most_used` was just name ASC — six `grpn`
plugins from six test marketplaces collapse to the same name and
became indeterminate via list_with_filters' created_at order).

New cascading hierarchy (chosen primary now matches what "most
popular" really means — wide adoption, not power-user volume):

  1. distinct_users_7d DESC  ← adoption / social proof
  2. invocations_7d   DESC  ← volume at equal adoption
  3. distinct_users_30d DESC ← broader adoption fallback
  4. invocations_30d  DESC  ← broader volume fallback
  5. name              ASC  ← deterministic textual order
  6. marketplace_slug  ASC  ← splits duplicate plugin names across
                              marketplaces

Six levels guarantee any two items end at a different sort key, so
the strip is stable across refreshes.

* fix(marketplace): unify Most Popular on 30d + right-align installed chip

Most Popular section was sorting on the 7d window while its cards
rendered 30d numbers — header label promised one thing, cards showed
another. Unified everything on 30d so a card means the same data
everywhere on the page.

- Dropped the "Last 7 days" meta from the Most Popular header.
- Sort cascade now starts on distinct_users_30d, then invocations_30d,
  with 7d adoption/volume as recency-aware fallbacks before the name +
  marketplace_slug deterministic tail. Six levels guarantee identical
  sort keys never produce indeterminate order across refreshes.
- Filter switched from invocations_7d > 0 to invocations_30d > 0 to
  match the new horizon.
- Most Popular now only renders on page 1 of the listing. Past initial
  discovery, a top-of-list popularity strip on page 2+ would shadow the
  results the user paged into. Pager click handler refreshes the
  section so navigating back to page 1 re-mounts it.

Chip layout — split engagement vs adoption visually:

  [user] N active · [bolt] N calls · [↑/↓] N%        [stack] N installed
  └────────── LEFT (time-bounded engagement) ────┘   └── RIGHT (all-time) ──┘

- Installed (stack_count) is all-time, decremented on uninstall. Alone
  it says little ("12 people installed it") without the engagement
  context next to it ("…but did anyone actually use it?"). Visually
  separating the two groups makes that distinction obvious — left
  group answers "is it used", right answers "does anyone have it".
- Implemented via flex with margin-left:auto on .seg-installed so
  installed drifts to the trailing edge.
- Installed tooltip now reads "Currently installed by N users" — the
  count is a real-time net (uninstall drops it), and saying "currently"
  makes that explicit. Helps when a card shows 0: signals "nobody has
  this in their stack right now", not "data missing".

* feat(plugin-detail): telemetry chip in hero, derived rows in sidebar

Surface the same telemetry funnel the listing card carries on the
curated plugin detail page, so clicking through from /marketplace
keeps a single mental model — figures match, semantics match. The
detail sidebar drops the two raw numbers that used to live there
(Invocations 30d / Users 30d — duplicated by the chip now) and
replaces them with two *derived* signals only the daily series can
provide: Active days + Last used.

Backend (app/api/marketplace.py):
- PluginDetailResponse.stack_count — curated reads via
  _load_curated_stack_counts(), flea reuses install_count. Frontend
  treats both sources uniformly.
- _build_telemetry() always returns a dict (never None). Frontend
  decides chip visibility from stack_count + invocations_30d the
  same way the listing card does. daily_series is always 30 entries
  (zero-padded) so "Active days" and "Last used" derivations on the
  sidebar are trivial array filters.

Frontend (app/web/templates/marketplace_plugin_detail.html):
- New .hero-telemetry slot at the bottom of the hero meta column,
  between the pills row and the action buttons. Renders the four
  funnel segments — active · calls · trend · installed — joined by
  ` · `. No left/right split: the hero has space, so a single
  coherent metadata strip reads cleaner than the card's split layout.
- Heroicons solid inlined (user / bolt / arrow-trending-up,-down /
  rectangle-stack) recoloured against the dark hero — icons in
  lighter tokens (mint #6ee7b7, peach #fdba74, cream #fde68a), trend
  pill keeps the saturated green/red because direction-coding earns
  its own colour.
- Tooltip on installed reads "Currently installed by N users" — the
  count is a real-time net (drops on uninstall), and "currently"
  makes that explicit when a card shows 0.
- fmtNum helper added so 1.2k / 14M renderings match the card's
  format exactly.
- Sidebar swap: Invocations + Users rows removed, replaced by
    Active days  →  "N of 30"
    Last used    →  fmtRelative of the latest non-zero day
  Both derived from telemetry.daily_series — engagement consistency
  + recency, neither of which the hero chip exposes on its own.

* feat(item-detail): telemetry chip in hero for curated skill/agent

Bring the funnel chip the plugin detail page got in 4cf38d40 to the
curated inner skill/agent detail page — clicking through from the
listing card now keeps the same metadata strip from grid to plugin
page to inner item page.

Backend (app/api/marketplace.py):
- _load_inner_item_stats() rewritten:
    * always returns a dict (never None) so the frontend can decide
      chip visibility client-side, same contract as _build_telemetry
    * adds trend_pct, computed the same way as plugin level
      (recent_7 vs prior_7 from usage_marketplace_item_daily, ≥3
      prior-week threshold)
    * adds daily_series (30 entries, zero-padded) so the sidebar can
      derive Active days + Last used
- InnerDetailResponse.parent_stack_count — new field. Skills/agents
  don't have a per-item subscription model, so the hero shows the
  *parent plugin's* stack count under a "Plugin:" prefix. The
  funnel: "12 installed plugin → 2 actually use this skill".
- curated_skill_detail + curated_agent_detail handlers load
  _load_curated_stack_counts() once and pass the parent's value.

Frontend (app/web/templates/marketplace_item_detail.html):
- New .item-detail .hero .hero-telemetry slot beneath the badges
  row. CSS mirrors plugin-detail's colour tokens (mint/peach/cream
  Heroicons solid + saturated trend pill) so the two surfaces read
  as one visual family.
- Installed segment uses a "Plugin:" label rendered with reduced
  opacity to signal the metric describes the parent, not the item
  itself. Tooltip: "Parent plugin (<plugin_name>) currently
  installed by N users".
- Sidebar Invocations + Users rows removed (chip carries them).
  Active days + Last used derived from telemetry.daily_series replace
  them; only rendered when activeDays > 0 so a brand-new skill
  doesn't show "0 of 30" / "Last used —".
- "Type" row dropped from the sidebar — duplicates the hero badge.
- fmtNum helper added (matches listing card + plugin detail).

Plugin detail (app/web/templates/marketplace_plugin_detail.html):
- Hero "Curator: …" line removed. The Details sidebar already
  carries that info; duplicating it under the h1 was visual noise.
- Sidebar "Owner" row renamed to "Curator" — for curated plugins
  it's a person who curates inclusion in this Agnes instance, not
  the upstream code owner. "Owner" was a hold-over label.

* feat(item-detail): unify hero with plugin detail — pills + breadcrumb + cleaner sidebar

- Inner skill/agent hero now uses the same `.pills` / `.pill.cat / .curated /
  .flea / .muted` class names + CSS as the plugin detail page; the only
  item-only addition is `.pill.type` (Skill / Agent uppercase, plugin detail
  has no kind axis).
- Hero `Updated` moved out of the meta-row into a muted pill (mirrors the
  plugin detail hero), removed from the Details sidebar to avoid duplication.
- Details sidebar slimmed: dropped Marketplace, Path, Updated rows; Parent
  plugin now shows the curator-friendly display name
  (`parent_display_name || manifest_name || slug`) instead of the slug.
- Breadcrumb extended to full path: Marketplace > <marketplace_name> >
  <plugin display name> > <self>, mirroring the plugin detail breadcrumb.
- Backend: new `InnerDetailResponse.parent_display_name` field, populated via
  `_curated_plugin_enrichment` from marketplace-metadata.json — same source
  plugin detail hero already uses.

* feat(marketplace): flea inner skill/agent detail + breadcrumb polish

- Flea inner skill/agent detail page parity with curated:
  * GET /api/marketplace/flea/{id}/skill/{name} + /agent/{name}
    returning InnerDetailResponse (mirror of curated_skill_detail).
  * /marketplace/flea/{id}/skill|agent/{name} web routes that render
    marketplace_item_detail.html with source='flea' + innerName context.
  * Frontend apiURL grows a third branch for flea-inner; breadcrumb
    grows to 4 segments (Marketplace > Flea Market > <plugin display
    name> > <self>) when innerName is set.
  * Telemetry attribution: MarketplaceItemLookup resolves
    <flea_plugin>:<inner> prefixes to (source='flea',
    parent_plugin=<plugin name>) so nested invocations land in the
    same rollups curated nested skills use. USAGE_PROCESSOR_VERSION
    bumped 5 -> 6 so the reprocess loop re-attributes historic events.
- Breadcrumb 2nd segment is now a generic clickable "Curated
  Marketplace" / "Flea Market" link to /marketplace?tab=... instead
  of the opaque per-instance marketplace_name. Applied on both plugin
  detail and inner item detail.
- Inner item hero telemetry chip works for both sources: installedCount
  branches on parent_stack_count (curated) vs install_count (flea),
  installed segment drops the "Plugin:" prefix for flea standalone /
  inner items.
- Updated row dropped from Details sidebar on item detail — the hero
  pill already carries the value, sidebar row was duplicate.

* feat(item-detail): block stack-install on flea inner items (mirror curated)

Inner skills/agents nested inside a flea plugin can no longer be added
to a user's stack on their own — adoption only happens at the plugin
level, same rule curated nested items have followed since launch.

- Hero action: when innerName is set (curated nested OR flea nested),
  render "Open parent plugin →" link + helper text instead of the
  install/remove buttons. Flea standalone entities (no innerName) keep
  the normal install UX.
- Meta-row: same branch now serves curated + flea inner — "part of
  <parent plugin display name> · by <author>" with the parent link
  pointing at the right detail page per source.

No API gate change needed: POST /api/store/entities/{id}/install only
accepts existing entity ids (plugin-level), inner items have no entity
id of their own so the endpoint cannot target them directly.

* feat(marketplace): telemetry chip on inner cards + fix flea hero chip visibility

Inner skill/agent cards on the plugin detail page now carry the same
four-segment funnel chip the marketplace listing cards show (N active
. N calls . trend . N installed), for both curated nested skills and
flea nested skills. Plus two fixes that were keeping the hero chip
hidden on flea plugin / flea inner detail pages.

- Backend `_load_inner_items_stats_by_parent(conn, source, parent_plugin)`
  bulk loader: one query per plugin against usage_marketplace_item_window
  + one against _daily, returning {(name, type): stats}. Avoids N+1
  per-card lookups.
- `InnerItemSummary` gains invocations_30d / distinct_users_30d /
  trend_pct / parent_stack_count fields. `curated_detail` and
  `flea_detail` (in the entity.type=='plugin' branch) enrich the
  skills / agents lists after the existing cover-photo enrichment loop.
- `marketplace_plugin_detail.html`: new `.plugin-detail .inner-card
  .inv-chip*` CSS lifted from marketplace.html with the listing-card
  rules, new buildInnerCardChip() helper, buildCardSection appends
  the chip to each card body. Same gate as the listing card (hidden
  on parent_stack==0 && calls==0).

- fix(flea): flea_detail forgot to populate PluginDetailResponse.stack_count
  from entity.install_count (listing card does this on line 851; detail
  endpoint didn't). Hero chip gate `stackCount===0 && calls===0` then
  always hid the chip even when the entity had installs. Now mirrors
  listing card semantics: stack_count == install_count for flea.
- fix(flea inner): renderInnerHeroTelemetry was reading `d.install_count`
  for any non-curated source. InnerDetailResponse has no install_count
  field — it has parent_stack_count (populated server-side from the
  parent flea plugin's install_count). Gate + label now read
  parent_stack_count for both curated nested AND flea nested scenarios;
  install_count remains the flea standalone path.

* fix(marketplace): Owner label on flea + parent-centric sidebar for flea inner

- Plugin detail Details sidebar — authorship row label now tracks the
  source: curated bundles get `Curator` (existing behaviour), flea
  bundles get `Owner`. The `owner_todo` reminder placeholder stays on
  the curated branch only; flea falls through silently.
- Inner item detail Details sidebar — flea-inner (skill/agent nested
  inside a flea plugin) now shares the curated nested layout: Parent
  plugin / Bundle size / Active days / Last used / Owner. Drops the
  flea-standalone shape's `Category`, `Version`, `Installs`, `Released`
  rows that didn't apply to a nested item. Active days + Last used were
  already wired (telemetryRows) — they just weren't on the flea-inner
  branch.

* fix(tests): bump SCHEMA_VERSION assertions 47 -> 48 post-rebase

The marketplace telemetry migration was renamed _v46_to_v47 -> _v47_to_v48
during the rebase onto main (collision with #326 FTS BM25 migration that
took the v47 slot). Two test files still asserted the pre-rebase value:

- tests/test_home_stats.py::test_schema_version_constant_is_46 (CI red)
- tests/test_schema_v46_migration.py::test_schema_version_is_46

Renames the helper fn name + bumps the assertion. The other two test
files (test_db_schema_version.py, test_schema_v42_migration.py) were
already updated in the rebase resolution.

* fix(telemetry): _build_telemetry returns None when invocations_30d == 0

The follow-up commit that introduced the always-return-dict shape broke
the test contract from the original v46 PR (commit b603e998):

  tests/test_marketplace_telemetry.py::TestDetailTelemetry::
    test_detail_endpoint_telemetry_absent_when_no_data
    AssertionError: assert {'daily_series': [...], ...} is None

Both `PluginDetailResponse.telemetry` and `InnerDetailResponse.telemetry`
are declared `Optional[Dict] = None`, the frontend renders are None-safe
(`d.telemetry || {}` guard + `if (!d.telemetry || ...)` on daily_series),
so dropping the dict on zero activity is the cleaner default.

* release: 0.54.21 — marketplace telemetry refactor (schema v48) + flea inner detail parity + listing UX polish

---------

Co-authored-by: Minas Arustamyan <arustamyan.minas@gmail.com>
Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
2026-05-15 20:58:03 +02:00
Vojtech
bb703517c9
fix(store): close 2 medium + 1 low adversarial-review findings (#322)
Three remaining findings from Codex's adversarial review of PR #316
(issue #318), plus a pre-existing version-numbering bug surfaced while
fixing the atomic-promote ordering.

M1 — Prompt sentinel escape now covers file PATHS, not just file
BODIES. Pre-fix the per-file `--- FILE: {rel} ---` header inlined the
untrusted relative path unescaped. A ZIP whose relative path
concatenated to `</bundle>` (a `<` directory plus a `bundle>` child)
could forge the trust-boundary close tag from inside the path slot
and inject apparent system instructions after the boundary. Same
`_escape_sentinels` helper now runs on both rel and body.

M2 — Live-bundle swap + DB promote is now atomic-ish. The runner /
override / inline-promote paths previously called
`repo.promote_version(...)` then `_swap_live_to_version(...)`. A
missing `versions/v<N>/plugin/` made the swap silently return False
— leaving the DB ahead of live. New `promote_to_version` helper in
`app/api/store.py` swaps FIRST (with the existing
staging → backup → live rename chain) and only advances the DB row
after the on-disk swap succeeds; rolls live back to prior on DB
write failure.

While wiring up M2, the strict source check exposed a pre-existing
bug: `update_entity` and `restore_version` derived
`new_version_no = entity.version_no + 1`. Under deferred promotion
that's wrong — entity.version_no stays at the last approved version
while version_history grows with blocked / pending entries.
Subsequent PUTs would overwrite an in-flight blocked v2 dir's bytes,
then the runner's hash-match promotion in `runner.run_llm_review`
would load bytes that didn't match the recorded submission hash.
Fixed by deriving from `max(version_history.n) + 1`.

L1 — Admin forensic download now serves STAGED bundle bytes per
submission, not live. Pre-fix downloading a blocked v2 streamed
live's prior approved v1 bytes — admins reviewing whether to
override saw the wrong bytes. Resolves staged `versions/v<N>/plugin/`
via `_version_no_for_submission`; falls back to live for legacy rows
without history linkage.

Tests:
- test_filename_with_bundle_sentinel_is_escaped
- TestAtomicPromote::test_missing_source_dir_does_not_advance_db
- TestAdminBundleDownload::test_download_v2_blocked_returns_staged_bundle_not_live
2026-05-15 17:56:09 +02:00
Vojtech
6fb11a137b
fix(store): close 1 critical + 2 high adversarial-review findings (C2/H2/H3 from #318) (#320)
* fix(store): close 1 critical + 2 high adversarial-review findings

Three findings from Codex's adversarial review of PR #316 (issue #318).

C2 — `/api/store/bundle.zip` leaked quarantined entities. The export
endpoint called `repo.list(...)` with no `visibility_status` filter,
so any authenticated non-admin could download pending / blocked v1
bytes — bypassing the publish gate. Mirrored the browse-listing gate:
non-admin sees only `approved` (plus their own non-approved entries
via `include_owner_id`); admins skip the filter.

H2 — concurrent PUTs on the same entity could both pass the
`latest_for_entity` pending gate. The `update_entity` and
`restore_version` handlers now wrap their critical section in a
per-entity asyncio.Lock (`_hold_entity_write_lock`). Single-process
deployments are now serialized; multi-worker deployments still have
a residual window (tracked in issue #318).

H3 — `StoreSubmissionsRepository.update_status` blindly overwrote any
current status. A late BG-task LLM verdict could clobber an
`overridden` row back to `approved` / `blocked_llm` after the admin
had already force-published. Added compare-and-swap on terminal
statuses (`approved`, `overridden`, `blocked_inline`); callers that
legitimately need to overwrite (admin rescan etc.) pass
`allow_terminal_overwrite=True`. Returns bool indicating whether the
write landed; BG callers no-op on terminal rows.

Tests:
- TestStoreBundle::test_bundle_zip_filters_quarantined_for_non_owner
- TestStoreBundle::test_bundle_zip_owner_sees_own_pending
- TestStoreBundle::test_bundle_zip_admin_sees_all
- TestConcurrentPutSerialization::test_per_entity_lock_serializes
- TestConcurrentPutSerialization::test_per_entity_lock_does_not_serialize_across_entities
- TestBgTaskIdempotency::test_late_verdict_does_not_clobber_overridden
- TestBgTaskIdempotency::test_explicit_allow_terminal_overwrite_works

* review fix: runner.run_llm_review honors update_status CAS bool

Codex's CAS in update_status closes the DB-level race correctly, but
runner.run_llm_review was still discarding the new bool return on both
its `approved` and `blocked_llm` branches. When the CAS no-op'd
(submission already at terminal status — most commonly an admin
override fired mid-review), the runner kept running the downstream
cascade:
  - set_visibility_if_pending (no-op on approved, but still ran)
  - promote_version + _swap_live_to_version (forward-only check
    mitigated worst case)
  - update_flea_attribution
  - audit.log(action="store.submission.approved" / "blocked_llm")
    — this is the operator-visible damage: the audit trail would
    show a verdict that contradicts the row's actual `overridden`
    status.

Fix: capture the bool, skip the cascade on no-op, log a single
`store.submission.bg_verdict_skipped` audit row instead. Mirrors the
existing `superseded_reason` path the runner already has for the
archive-during-review case (TestPRReviewFixes::
test_bg_verdict_skipped_when_admin_archives_during_review).

Test: TestBgTaskIdempotency::test_runner_late_verdict_logs_skipped_not_approved
sets up the v1-approved + v2-pending + admin-override sequence, fires
run_llm_review directly with a mocked "approved" verdict, asserts row
stays overridden AND audit has bg_verdict_skipped AND audit does NOT
have a contradictory approved entry.

CHANGELOG H3 bullet expanded to acknowledge the bg_verdict_skipped
audit-row behavior — operator reviewing the queue now sees dropped
verdicts explicitly rather than via row-vs-audit contradiction.

---------

Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
2026-05-15 17:45:43 +02:00
Vojtech
a694a30a5e
fix(store): surface review failures + harden publish gate (#316)
* fix(store): surface review failures + harden publish gate

Four independent fixes to the flea-market submission pipeline, all surfaced
by an admin upload that landed at status='approved' without an LLM review.

1. LLM truncation no longer pins submissions in review_error.
   - Raised MAX_RESPONSE_TOKENS 2500 → 6000 in llm_review.py
   - Added one-shot retry-with-doubled-budget in anthropic_provider.py
     (capped at 4× initial)

2. Flea detail page surfaces the latest submission's failure verdict even
   when a previously-approved version is still serving (deferred-promotion
   path). The _quarantine_banner gate widened from `visibility != approved`
   to also fire on `blocked_inline / blocked_llm / review_error`, with copy
   that distinguishes the v2+ edit case ("Latest edit failed review —
   previously approved version (vN) keeps serving") from the initial-upload
   quarantine wording.

3. Restore button + endpoint no longer allow restoring a version that was
   never approved. Added StoreEntitiesRepository.get_with_version_approvals
   joining store_submissions, gated the UI button on submission_status in
   ('approved', None), rendered status pills for non-restorable rows, and
   added a 400 version_not_approved guard in POST /restore.

4. **BREAKING (operator-facing)**: publish gate is now fail-CLOSED on
   misconfig. The previous get_guardrails_enabled() silently fell back to
   "disabled, auto-approve everything" when guardrails.enabled=true in YAML
   but no ANTHROPIC_API_KEY was in env. Split into:
     - get_guardrails_enabled()              (intent — YAML)
     - get_guardrails_llm_provider_ready()   (readiness — env)
   Three-state matrix:
     enabled=false                       → auto-approve (unchanged)
     enabled=true + ready=true           → normal pipeline (unchanged)
     enabled=true + ready=false (NEW)    → submissions hold at pending_llm
                                           awaiting admin retry or override
                                           (was: silent auto-approve)
   Admin "Retry review" eligibility broadened to include pending_llm.
   Boot-time WARNING banner surfaces the misconfig in app/main.py.
   docs/STORE_GUARDRAILS.md updated with the three-state matrix.
   Operators relying on the auto-fallback for local-dev no-LLM setups must
   now explicitly set `guardrails.enabled: false` in instance.yaml.

Tests: 4623 passed. Added TestPublishGateFailClosed (4 tests) and
TestRestoreVersion::test_restore_rejects_* (3 tests). conftest.py adds an
autouse fixture defaulting guardrails OFF so legacy tests don't need to
know about the new toggle.

* fix(store): admin override promotes v2+ edits to current

The override handler at app/api/admin.py:3708 only flipped submission
status → 'overridden' and entity visibility → 'approved'. Under the v37+
deferred-promotion model that's insufficient for v2+ edits / restores:
the new bundle sits in versions/v<N>/plugin/ and the entity row stays at
the prior approved version_no + hash + on-disk live bundle. Installers
kept getting the OLD bytes the admin had just intended to replace.

Mirror the runner.run_llm_review auto-approval branch: look up the
submission's version_hash in entity.version_history, and if its `n`
differs from entity.version_no, promote_version + _swap_live_to_version.
Initial v1 overrides are unaffected — the loop finds n=1 == version_no
and skips promotion.

Tests:
- test_override_v2_edit_promotes_to_current: stage v1 approved + v2
  blocked_llm; override the v2 sub; assert entity.version_no=2,
  entity.version flips off the v1 hash, and the live plugin/ dir
  mirrors versions/v2/plugin/.
- test_override_v1_initial_upload_no_promote: regression guard so the
  promote loop doesn't accidentally bump a v1 override.

Audit log gains a promoted_to_version_no field on the override action.

* fix(store): retry/rescan review staged bundle; override forward-only

Two adversarial-review findings from a Codex pass on the publish-gate
work.

C1. Admin retry + rescan were passing live `plugin/` to the LLM. For a
v2+ submission held at `pending_llm` / `blocked_llm` / `review_error`,
live still holds the prior approved version's bytes — so the LLM
reviewed the WRONG bytes, and the runner's hash-match promotion in
`run_llm_review` would then advance the entity to staged bytes that
were never actually reviewed. Resolve the staged
`<entity>/versions/v<N>/plugin/` from the submission's
`version_history` entry, with a fall-back to live for legacy pre-v37
rows that never seeded a versions/ dir. Helpers
`_submission_plugin_dir` and `_version_no_for_submission` added to
`app/api/store.py` so override / retry / rescan share one path.

H1. Override's promote loop used `target != current`, which would
silently demote the live bundle when admin overrode a stale v2
submission while v3 was already approved + live. Changed to
`target > current` so override flips status + visibility on the row
regardless, but on-disk promotion only fires forward. Same `>`
defensive guard applied in `runner.run_llm_review` so a late LLM
verdict racing with a newer approval can't demote either.

Tests:
- TestAdminRetryReviewsStagedBundle::test_retry_v2_blocked_passes_staged_dir_not_live
- TestAdminRetryReviewsStagedBundle::test_rescan_v2_blocked_passes_staged_dir_not_live
- TestOverrideForwardOnly::test_override_stale_v2_does_not_demote_when_v3_current

* review polish: CHANGELOG drift, override eligibility, defensive copy

Three small additions on top of the retry/rescan staged-bundle fix:

1. CHANGELOG: the PR's bullets had drifted into the released
   [0.54.17] section during rebase (context-match landed them next
   to already-released content). Moved them up to [Unreleased] where
   they belong; [0.54.17] now holds only what was actually released
   (refresh-marketplace ls-remote, /me/activity hero, CI sharding +
   workflow polish).

2. app/api/admin.py: admin override eligibility now accepts
   pending_llm alongside blocked_inline + blocked_llm + review_error.
   Closes a UX gap from the new fail-CLOSED behavior: under
   enabled-but-not-ready, a known-good submission would otherwise
   sit indefinitely until the admin set credentials AND clicked
   Retry. Override already routes through version_history (and is
   now forward-only on promote), so it stays safe for v2+ deferred-
   promotion submissions.

3. src/repositories/store_entities.py: get_with_version_approvals
   defensively copies each version_history entry before annotating
   with submission_status. self.get() re-parses JSON each call today
   so this is belt-and-suspenders against any future caching layer
   leaking the annotated key into a subsequent plain get() call.

Tests: 112 passed (focused on test_store_entity_versions +
test_admin_store_submissions, covering the retry/rescan staged-
bundle fix the author shipped + this polish).

---------

Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
2026-05-15 15:52:07 +02:00
minasarustamyan
63ae676b27
perf(marketplace): cache cover photos + restore Curated filter spacing (#294)
* perf(marketplace): browser-cache cover photos + restore Curated tab filter spacing

Cover photos on /marketplace grid now serve with `Cache-Control: public,
max-age=2592000, immutable` plus URL fingerprinting (`?v=<commit-sha8>`
for curated, `?v=<version_no>` for flea) so browser refresh stops hitting
the server entirely for unchanged assets. Per-plugin RBAC dropped from
the three image endpoints (curated_asset, curated_mirrored, get_entity_photo)
in favor of login-only auth — eliminates _system_db_lock contention on
parallel image requests. Per-request magic-bytes revalidation also dropped
from curated_asset (it was re-reading the file just to discard the bytes,
then FileResponse read it again).

Spacing bug: sort-dropdown commit (6be1cee) wrapped .mp-filter-row in a
new flex container with inline margin-bottom:4px, masking the original
12px CSS rule. Curated tab (where .mp-type-row is hidden) ended up with
4px between filters and the card grid. Wrapper margin restored to 12px.

See CHANGELOG entry under [Unreleased] — the RBAC relaxation is called
out under ### Security with explicit threat-model rationale for AI/human
reviewers.

* test(marketplace): update renamed-html-as-png test for dropped magic-bytes check

Magic-bytes body validation was dropped from `curated_asset` in the previous
commit — the request path now relies on extension allowlist + pinned
Content-Type + nosniff + strict CSP to neuter mismatched payloads at the
browser layer. Update the test to assert the new defense-in-depth posture
(200 served, but Content-Type=image/png + nosniff + CSP=default-src 'none')
rather than the gone 415.

---------

Co-authored-by: Minas Arustamyan <arustamyan.minas@gmail.com>
2026-05-14 10:09:32 +02:00
Vojtech
4501c9c3dd
fix(store-guardrails): post-#290 review follow-up — purge tuple, filter chip, stale docs, lazy bundle_meta, logger.exception (#295)
Addresses post-merge review findings on #290:

- Admin Rescan is the only post-v30 producer of status='blocked_inline'.
  Re-add it to admin queue 'Needs review' filter chip and to
  TERMINAL_BLOCKED_STATUSES in the bundle-purge job so rescan-produced
  rows surface in the default operator view and bundles get TTL-swept
  instead of lingering indefinitely.

- Update three doc-drift sites still referring to the pre-#290 spam
  counter scope (counted blocked_inline). The counter now narrows to
  blocked_llm + review_error; fix the comment in app/api/store.py,
  the docstring in get_guardrails_blocked_quota_per_day(), and the
  operator-facing hint rendered on /admin/server-config.

- Add positive test for _reject_inline_or_continue validation branch
  (code='validation_failed', checks payload shape, no-DB-write
  contract). Locks the frontend wizard's detail.checks contract.

- Tighten test_quota_disabled_with_zero — assert (200, 201) explicitly
  instead of !=429 so a 500 regression no longer passes.

- _reject_inline_or_continue takes plugin_dir and lazy-computes
  bundle_meta only on the security branch. Validation rejects no
  longer pay for a SHA256 walk on the bundle.

- Surface store.upload.security_blocked audit-log write failures via
  logger.exception instead of swallowing — that audit row is the only
  forensic trace by design.
2026-05-14 08:02:44 +02:00
Vojtech
513711ed37
feat(store): hard-reject inline guardrail failures, trace security only (#290)
* feat(store): hard-reject inline guardrail failures, trace security only

Inline failures (manifest + content validation, static-security
deny-list hits) now hard-reject upstream of any DB write or bundle
persistence. The v30 contract that landed every inline failure as a
hidden+blocked_inline entity + admin-rescannable bundle is replaced
with two response shapes:

  - 422 code=validation_failed — manifest/content issues. Banner-only,
    no submission row, no audit_log entry. Submitter fixes and retries.
  - 422 code=security_blocked — static_scan finding. Banner-only on
    the wire, plus one audit_log row (store.upload.security_blocked)
    carrying findings + sha256 + size for admin forensics.

Quarantine + admin rescan/override apply only to the async LLM path
(blocked_llm / review_error) — the cases that genuinely benefit from
admin judgment.

Spam-quota counter narrows to blocked_llm + review_error. Admin queue
filter chip drops blocked_inline. Bundle TTL purge stops sweeping
blocked_inline. Legacy blocked_inline rows from instances that ran
the v30 contract remain reachable via the "All" tab.

New _reject_inline_or_continue helper in app/api/store.py centralises
the two-tier rejection across create_entity, update_entity, and
restore_version. Frontend templates render the new payloads as inline
banners (no redirect on failure) and keep submission_blocked as a
one-release back-compat branch.

Tests: new _seed_quarantined_entity helper replaces the older
_make_eval_skill_zip-driven setup wherever a test needs a
hidden+blocked_llm entity. 199 store tests pass under -n auto.

* release: 0.54.8 — store inline hard-reject (BREAKING)

Last commit on the PR per CLAUDE.md hard rule. Patch bump (0.54.7 →
0.54.8) wrapping Vojta's hard-reject refactor.

**BREAKING for store-upload clients**: validation failures now return
422 with `code='validation_failed'` (no entity row, no submission row,
no audit_log entry) instead of the v30 `submission_blocked` 200
response that landed a hidden `blocked_inline` row. Frontend wizard +
edit + restore still understand the legacy code for one release as a
fallback for stale clients hitting an older deploy. Operators with
custom integrations against `POST /api/store/entities` should update
to handle the new `code='validation_failed'` / `code='security_blocked'`
422 responses.

No DB migration required (legacy `blocked_inline` rows from instances
that ran the v30 contract remain reachable via the admin queue's
"All" tab; bundle-purge job no longer covers them but they linger
harmlessly).

---------

Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
2026-05-13 19:59:12 +00:00
ZdenekSrotyr
b4d3c576af
Activity Center: audit log + telemetry + sessions + agnes_* tables (#278)
* docs(spec): admin observability spec + Activity Center MVP plan

Parent spec (480 lines) + executable plan (2295 lines, 14 TDD tasks).
Covers Activity Center rebuild (/admin/activity), with /admin/sessions
and /admin/feedback deferred to follow-up plans.

Already incorporates reviewer-pass revisions across three angles
(security, production resilience, code architecture):
- _get_db import path corrected to app.auth.dependencies
- Test fixtures aligned with seeded_app / admin_user / get_system_db
- All new audit writes wrapped in try/except + logger.exception
- Filename sanitization on session uploads
- DuckDB DESC index behavior documented; upgrade window flagged
- Migration idempotency + evolved-DB test cases
- reveal_raw + shared-cache multi-worker explicitly deferred

Targets schema v40 (audit_log gains params_before, client_ip,
client_kind, correlation_id + 3 indices).

* feat(db): schema v40 — audit_log gains params_before, client_ip, client_kind, correlation_id + 3 indices

* chore(test): clean up Task 1 — drop unused import, rename stale test

* feat(audit): AuditRepository.log() accepts params_before/client_ip/client_kind/correlation_id

* test(audit): strengthen params_before assertion to round-trip JSON content

* feat(audit): AuditRepository.query() rich filters + keyset cursor pagination

* feat(sync): SyncStateRepository.list_recent() cross-table feed

* feat(audit): POST /api/sync/trigger writes audit_log row

* feat(audit): POST /api/scripts/run-due writes audit_log row

* feat(audit): POST /api/upload/sessions writes audit_log row + sanitizes filename

* feat(audit): GET /api/data/{table_id}/download writes audit_log row

* feat(activity): /api/admin/activity timeline + /health + /sync endpoints

* feat(ui): /admin/activity rebuilt — health pulse, timeline, sync grid; /activity-center → 308 redirect

BREAKING: removed demo executive-pulse / maturity-roadmap content from activity_center.html.
The page now reflects real audit_log + sync_history data.

* feat(ui): admin nav + dashboard widget point at /admin/activity

* feat(activity): recursive-audit suppression for AC read endpoints (60s window per actor+filter)

* feat(activity): emit PostHog events when integration enabled (no-op default)

* fix(audit): move v40 indices out of _SYSTEM_SCHEMA + update test_repositories to unpack query() tuple

_SYSTEM_SCHEMA CREATE INDEX on audit_log(timestamp) failed when migration
tests hand-roll a bare audit_log (id, action) without the timestamp column.
Fix: remove indices from _SYSTEM_SCHEMA; add ADD COLUMN IF NOT EXISTS guards
for timestamp and other pre-v40 columns in _v39_to_v40() so the upgrade path
is safe on any hand-rolled schema; call _v39_to_v40 explicitly in the
fresh-install (current==0) path to restore index creation there.

Also unpack the (rows, next_cursor) tuple from AuditRepository.query() in
the three TestAuditRepository tests that still treated it as a list.

* docs: CHANGELOG entry for Activity Center MVP

* chore: refresh stale module docstring in app/api/activity.py

* feat(cli): agnes admin activity — terminal access to Activity Center (timeline + health + sync)

* fix(db): _v39_to_v40 — add IF NOT EXISTS guard for 'action' column

The v39→v40 ladder step adds defensive ADD COLUMN IF NOT EXISTS for
every audit_log column so a hand-rolled bare audit_log (id only) is
safe through the ladder. 'action' was missing from the guard list,
causing CREATE INDEX idx_audit_action_time to fail on tests that
stub audit_log with only an id column (tests/test_e2e_extract.py::
TestSchemaMigration::test_migration_preserves_and_extends).

Local 6/6 schema tests + the previously-failing CI test pass.

* docs(spec): platform telemetry epic — Boss directive + Activity Monitoring plan rebased onto v40 (stacked on zs/spec-activity-center)

* feat(db): schema v41 — 7 usage_* tables for telemetry (events, summary, rollups, attribution)

* chore(db): tighten v41 — usage_session_summary.session_id NOT NULL + upgrade test asserts all 7 tables

* feat(usage): UsageAttributionRepository — replace/delete/lookup over usage_attribution_* tables

* refactor(marketplace): extract list_inner_skills/agents/commands to src/marketplace_listing.py for reuse

* feat(usage): explode plugin attribution on marketplace sync + store entity write; backfill script

* refactor(marketplace): finish src/marketplace_listing.py extraction — drop duplicate _list_inner_* + _parse_frontmatter from app/api/marketplace.py

* feat(usage): promote attribution helpers to src/usage_attribution_helpers.py; hook update_entity rename + bundle-swap; clarify best-effort semantics

* feat(usage): UsageProcessor real extraction + rollup rebuild + 10 fixture-driven tests

* fix(usage): include tool_id in event hash + executemany + rollup transaction (critical multi-tool-turn drop fix)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(marketplace): popularity stats — invocations_30d + trend + sort=most_used|trending + Most Popular section

* feat(admin): /admin/users/<id> Sessions section — list + single-file + bulk-zip downloads (audit-logged)

* feat(usage): admin export endpoint + CLI — csv/json/parquet streaming, filters, audit-logged

* feat(usage): agnes admin ask — LLM Text-to-SQL over usage_events with SELECT-only validator (audit-logged)

* feat(usage): reprocess + prune endpoints + scheduler daily prune job + CLI

* docs: PLATFORM_SETUP.md operator playbook + HOWTO/ cookbook (5 guides + index)

Adds docs/PLATFORM_SETUP.md as a consolidated operator playbook covering
bootstrap, TLS, marketplaces (curated + flea), scheduler env vars, telemetry
extraction/export/ask/prune, privacy posture, and daily routine.

Adds docs/HOWTO/ with 5 analyst cookbook guides: first query, snapshots for
remote tables, private sessions, feedback + admin ask, and customizing skills.

Existing setup docs (QUICKSTART, DEPLOYMENT, ONBOARDING, HEADLESS_USAGE)
get a one-line cross-reference at the top pointing to PLATFORM_SETUP.md.

* docs(changelog): platform telemetry epic — usage_* foundation + surfaces + admin access + docs

Comprehensive [Unreleased] entry covering: usage_events/session_summary/
tool_daily/plugin_daily tables (v41), attribution lookup tables, backfill
script, marketplace Most Popular + invocation chips + sort, admin Sessions
section, export/ask/reprocess/prune endpoints + CLI mirrors, Activity Center
(v40), PLATFORM_SETUP.md + HOWTO/ docs, and operations notes for v41 upgrade.

* fix(security): block DuckDB read_*/http_*/glob functions in usage_ask validator + symlink escape guard in session zip + clarify mark-private semantics

* fix(admin): parquet export tempfile cleanup on COPY failure + correct processed-first sort on /admin/users/<id>/sessions

* feat(audit): close 8 production audit gaps — query (local/remote/hybrid), catalog/schema/sample, snapshot estimate/create, check-access

* feat(ui): /admin/usage summary dashboard + per-user activity tab on /admin/users/<id>

* fix(audit): cap error messages at 200 chars + audit user_activity reads + recursion guard on usage.summary

* fix(audit): catalog.list audits on error path + clean up deferred json import

* fix(ux): client_kind=cli for PAT auth + timeline empty state + email-instead-of-uuid + nav reorder + help text + loading indicators + ask doc

* feat(observability): unify /admin/activity into single page with saved views

- KPI cards (events, users, error rate, p95) clickable as quick-filters
- Faceted filter dropdowns populated from audit_log in the current window
- Sortable audit table, cursor pagination, per-row JSON side panel
- Saved views (schema v43: user_observability_views) — per-user state
- Top bar: window selector + 30s Live toggle + saved views dropdown
- /admin/scheduler-runs → 308 redirect (source=scheduler filter)
- New endpoints: /api/admin/observability/{facets,kpis,views}

* test: update activity + scheduler-runs tests for unified page

- test_admin_activity_page_renders asserts new structural anchors
- test_admin_scheduler_runs_page_admin_only asserts 308 redirect

* fix(observability): respect [hidden] on modal + side panel

CSS `display: flex` on .obs-modal beat the [hidden] attribute's UA
display:none, so the save-view modal rendered on page load and Cancel
clicks couldn't dismiss it. Gate the modal's flex layout on
:not([hidden]); add the same display:none guard prophylactically to
.obs-panel and .obs-views-panel.

* feat(observability): user enrichment in audit + interactive /admin/usage

Activity:
- /api/admin/activity now joins users for user_email + user_name per row
- User column renders "name (id-prefix)" or "email (id-prefix)" instead
  of an opaque truncated UUID; falls back to id when the user record is
  missing

Usage:
- /admin/usage rewritten as the same filter/group-by/search pattern as
  /admin/activity. Faceted dropdowns (User / Tool / Source / Event type)
  populated from usage_events; debounced free-text search across
  tool_name / skill_name / subagent_type / command_name
- New endpoints /api/admin/usage/{facets,kpis,query}; the query endpoint
  supports group_by in {day, username, tool_name, source, ref_id} with
  sort + offset pagination, plus an ungrouped raw-events mode
- 4 KPI cards (events, distinct users, distinct tools, error rate) are
  clickable quick-filters; clicking a grouped row applies the bucket as
  a filter
- Old static `?window=7d|30d|all` server preload removed; all state is
  client-side via since_minutes + group_by + filters in the URL

* fix(observability): clearer labels, all-column sort, drop saved views UI

- Rename page titles: "Activity" → "Server activity", "Usage" → "Tool usage"
  with a one-line subtitle on each explaining what the page covers and
  linking the other one. The two pages source different data (audit_log
  vs usage_events) and the previous labels conflated them.
- Drop the saved-views dropdown + save modal from /admin/activity. The
  modal pop-open bug was the trigger; the value wasn't there yet. The
  /api/admin/observability/views CRUD + DuckDB table stay in place.
- Rename "Live (30s)" to "Auto-refresh (30s)" with a tooltip clarifying
  that it's the re-fetch rate, not the time range. Time range now
  labeled "Time range" instead of "Window".
- All audit-table columns are sortable (User, Source, Action, Resource,
  Result added); sort is page-local with a Jinja comment explaining the
  trade-off. Same for raw usage rows.
- Fix duplicate sort-arrow bug — the literal "▼" in the Time th HTML was
  rendering alongside the CSS ::before arrow. Removed the literal; CSS
  is the single source of truth.

* feat(observability): global Sessions browser + transcript viewer + CLI

Web:
- /admin/sessions — list every collected session JSONL across all users
  with time-range, user, model, errors-only and free-text filters. Default
  sort surfaces error-heavy sessions first. KPI cards (sessions, distinct
  users, sessions w/ errors, tool error rate) clickable as quick-filters.
- /admin/sessions/<username>/<file> — transcript viewer rendering the
  JSONL chronologically: user prompts, assistant text, tool calls (with
  JSON input) and tool results (with flattened output). Errors get a red
  border + chip and a "Next error" navigation button at the top.
- Admin dropdown gains a "Sessions" link.

API:
- GET /api/admin/sessions/{list,kpis,facets} — filtered cross-user reads
  off usage_session_summary
- GET /api/admin/sessions/{username}/{file}/transcript — parses JSONL via
  the existing services.session_pipeline.lib, returns chronological events
- GET /api/admin/sessions/{username}/{file}/download — JSONL stream, same
  path-safety guards as the per-user endpoint, audit-logged

CLI:
- `agnes admin sessions list [--user X] [--errors] [--since 7d]` — table
  output with `!` prefix on rows that hit a tool error
- `agnes admin sessions show <username> <file>` — transcript dump, with
  `--errors` to print only the failed tool_result blocks
- `agnes admin sessions download <username> <file> [-o path]`
- `agnes admin sessions kpis` — top-level numbers

* feat(internal): expose telemetry tables to agnes query with row-level RBAC

Three new registered tables backed by system.duckdb, queryable through
the same /api/query plumbing analysts use for Keboola / BigQuery /
local sources:

  agnes_sessions  → usage_session_summary  (filter: username)
  agnes_usage     → usage_events           (filter: username)
  agnes_audit     → audit_log              (filter: user_id)

RBAC is per-row, not per-table: admins see every user's rows; non-admins
see only their own. The filter is built server-side from the auth user
dict; non-admin filter values are regex-validated before SQL interpolation.

Implementation:
- new connector connectors/internal/ with access (filter+exec) + registry
  (idempotent table_registry seed at startup)
- /api/query detects internal table refs and short-circuits to a CTE
  wrapper that prepends "WITH agnes_x AS (SELECT * FROM <src> WHERE …),
  …" then "SELECT * FROM (<user_sql>) AS _q". DuckDB cursor on the
  shared system.duckdb handle — opening parallel handles / ATTACH on the
  same file is blocked process-wide.
- mixing internal + BQ / registered local tables in one SELECT is
  rejected (v1 limitation)
- src.rbac.can_access_table waves internal tables through for all
  authenticated users; row scoping is the actual security control
- /api/v2/schema and /api/v2/sample gained internal branches; sample
  intentionally skips its cache because rows are RBAC-scoped per caller
- audit row written as action='query.internal' with is_admin flag

Tests: connectors/internal/access — RBAC, filter clause, schema, CTE
wrapper coexistence with user-supplied aggregations, unsafe-username
rejection. 16/16 passing.

Motivating queries this enables:
  SELECT tool_name, COUNT(*) FROM agnes_usage
   WHERE is_error GROUP BY 1 ORDER BY 2 DESC
   -- analyst self-introspection: which tools fail for me?

  SELECT user_id, COUNT(*) FROM agnes_audit
   WHERE action = 'session.transcript_view' GROUP BY 1
   -- admin: who's been looking at whose session transcripts?

* feat(admin): group dropdown into 5 named sections + internal tables in /catalog

Admin dropdown gains section headers so admins can land on the right
page without re-reading the full menu:

  Activity Center      Server activity / Tool usage / Sessions
  Users & Access       Users / Groups / Resource access / Tokens
  Data                 Tables
  Agent Experience     Curated Marketplaces / Flea Submissions /
                       Agent Setup Prompt / Agent Workspace Prompt
  Server               Server config

"Agent Experience" frames the curated content + prompts as one cluster
— it's all admin-controlled material that shapes what an analyst's AI
agent encounters. "Configuration" → "Server" since only one item lives
there now.

Renamed the section's first two items:
  "Activity" → "Server activity" (matches page H1)
  "Usage"    → "Tool usage"

Also fixes /catalog visibility of the internal tables (agnes_sessions /
_usage / _audit) for non-admin users: ``app.auth.access.can_access``
short-circuits to True for resource_type='table' + an internal-table id.
Without this, non-admins saw the tables in /api/v2/catalog (which uses
the same RBAC bypass) but not on the /catalog HTML page (which calls
can_access directly, requiring a resource_grants row internal tables
don't have).

CSS for `.app-nav-menu-section`: small caps, muted, non-clickable; first
section trims top padding so the panel doesn't open with an awkward gap.

* refactor(admin): move corporate memory into Admin > Agent Experience

Memory link was the only admin-only entry in the primary nav (gated by
session.user.is_admin). Moves it into the Admin dropdown under Agent
Experience, alongside Curated Marketplaces / Flea Submissions / Prompts
— all admin-curated content that shapes what an analyst's AI agent
encounters.

Renamed the nav label to "Shared Knowledge" to match what the page
actually is (admin-curated organisational knowledge from session
verification, surfaced to agents). URL stays at /corporate-memory; the
route still gates on require_admin per the existing comment.

Side effect: primary nav (Home / Marketplace / Data Packages) is now
uniform for every authenticated user — no conditional admin-only entry.

* ui: rename admin entries to Curated Knowledge / Init Prompt / Workspace Prompt

- "Shared Knowledge" → "Curated Knowledge" (parallel with "Curated
  Marketplaces" in the same Agent Experience section; "curated" tells
  the admin what they do there — review + approve)
- "Agent Setup Prompt" → "Init Prompt" (matches the `agnes init` flow
  it actually drives)
- "Agent Workspace Prompt" → "Workspace Prompt" (the "Agent" prefix
  was redundant — every item in the section is agent-facing)

Renames page titles + H1s on /admin/agent-prompt and
/admin/workspace-prompt to match.

* refactor: rename Usage → Telemetry across user-facing surfaces

External surfaces all switch; internal Python module / file names and the
physical DB tables (usage_events, usage_session_summary, usage_tool_daily,
usage_plugin_daily) stay — renaming them would force a schema migration
+ a redo of the LLM Text-to-SQL prompt for no analyst-visible win.

Changes:
- Admin dropdown: "Tool usage" → "Telemetry"
- Page H1 / <title>: same
- URL: /admin/usage → /admin/telemetry; old URL 308-redirects
- API prefix: /api/admin/usage/* → /api/admin/telemetry/*
- CLI: primary command `agnes admin telemetry …`; `agnes admin usage` kept
  as a deprecated alias so existing operator scripts keep working
- Internal data-source table id: agnes_usage → agnes_telemetry. The
  registry seed now evicts any stale internal-source row whose id no
  longer matches INTERNAL_TABLES, so the old `agnes_usage` row is
  removed from table_registry on next app boot
- All tests + JS endpoint paths updated

* test(rbac): include auto-appended internal tables in expectations

get_accessible_tables now appends agnes_sessions / agnes_telemetry /
agnes_audit to every authenticated user's accessible-tables list so the
internal data source shows up in /catalog. The two existing rbac tests
asserted hardcoded list shapes that pre-dated the change.

Rewritten to assert "granted tables + the canonical internal-table set"
instead of literal lists, so the test stays correct if the internal
table roster changes again later.

* ui: visual dividers between admin-dropdown sections

Adds a 1px top border + 6px top margin to every section header except
the first, so the five named groups (Activity Center, Users & Access,
Data, Agent Experience, Server) read as visually separated clusters.
The header itself stays small-caps + muted as before — the border is
additive.

* ui(memory): match obs-topbar visual on /corporate-memory

The Curated Knowledge page (linked from the admin dropdown's Agent
Experience section) opened straight into the stats bar — no title,
no subtitle, no shared chrome with the other admin pages. Adds an
obs-topbar-style header at the top of .container-memory:

  - H1 "Curated Knowledge"
  - subtitle explaining what the page is + how AI agents pull from it

The `.ck-*` class set duplicates the inline obs-* styles from
/admin/activity etc. for this one page; promoting the obs-* class set
to style-custom.css for shared reuse is the obvious next step (4 pages
already inline the same CSS), tracked as a follow-up.

Page <title> also renamed from "Corporate Memory" → "Curated Knowledge".

* ui(tables): list Agnes internal tables in /admin/tables + group in /catalog

/admin/tables previously rendered three per-source-type listings
(BQ / Keboola / Jira) and dropped any row whose source_type didn't
match — so the agnes_sessions / agnes_telemetry / agnes_audit rows
seeded into table_registry were invisible. Adds a fourth read-only
section "Agnes internal tables" that filters source_type === 'internal'
and renders the same registry-table layout the other sections use,
with two changes:

  - no Register button (these rows are seeded on every app boot from
    connectors/internal/registry.py)
  - Edit + Delete actions hidden (any change would be reverted on the
    next start). Manage access stays so admins can still inspect.

Mode badge picks up a new mode-internal CSS class (teal accent) so the
display doesn't lie and call it "local".

In /catalog, internal tables now group under an "agnes" accordion
section (bucket="agnes" on seed) instead of falling into the catch-all
"default". Single source of truth for which tables exist; admins find
them where they expect.

* ui(tables): Agnes internal as a 4th tab next to BQ/Keboola/Jira

Previous iteration mounted the internal-table listing as a separate
standalone card under the tab strip. Reshapes it to a proper
tab-content section so admins switch between data sources via one
consistent nav (BigQuery / Keboola / Jira / Agnes internal).

- New tab button "Agnes internal" in the tab-nav.
- The listing card becomes <section id="tab-content-internal"
  class="tab-content">; switchTab() already routes by id so no JS
  change beyond extending the hash allowlist for direct #internal
  links.
- Tab content keeps the read-only treatment from the previous commit
  (no Register button, no Edit / Delete in renderRegistryListing).

* ui: rename Curated Knowledge → Curated Memory

Settles the naming back on "Curated Memory" — parallel structure with
"Curated Marketplaces" in the same Agent Experience section, and zero
rename ripple: URL (/corporate-memory), API (/api/memory/*), CLI
(agnes admin memory), and Python modules all stay on "memory" so the
admin label finally lines up with the underlying surfaces.

The "Curated" prefix still tells admins what they do on the page
(review pending → approve / mandate / reject) and reads as a sibling
of "Curated Marketplaces" right next to it in the dropdown.

Touches: admin dropdown label, page <title>, page H1. DB tables stay
on knowledge_* (already the canonical naming for the data shape).

* ui: rename "Server activity" → "Audit log"

"Audit log" is what the page actually is — server-side audit_log table
rendered with KPI cards + filter bar + sortable table. The "Server
activity" label confused the term with Claude Code session telemetry
(Telemetry page) and didn't make the source/concept clear.

Touches:
- Admin dropdown nav label
- /admin/activity page H1 + subtitle
- /admin/telemetry subtitle cross-link
- test_activity_api page-renders assertion

URL (/admin/activity) and API (/api/admin/activity/*) stay — the
"activity" name has stuck at the route layer for a year; rerouting
those would churn dashboards/bookmarks for zero analyst-visible win.

* ui(admin-nav): gray band on each section header for clearer separation

Previous iteration used a 1px top border between section labels — the
labels still blended into the items above/below at a glance. Switches
to a light gray background band per section header, extended edge-to-
edge inside the panel via negative horizontal margins. Bolder
font-weight (700) reinforces the separation; bumping the font color
isn't needed because the band itself does the work.

First section's header tucks into the panel's top border-radius so the
band reaches the corners without a gap.

* ui(catalog): rename internal-table category to "Agnes Internal"

`bucket` is what /catalog renders as the accordion category header
verbatim — "agnes" lowercase didn't read as a real category name and
got confused with a system identifier. Bumps to "Agnes Internal".
Seed re-applies on every app boot so existing rows pick up the new
bucket value via `ON CONFLICT (id) DO UPDATE`.

* ui(catalog): split Agnes Internal into its own card on /catalog

Previously the three internal tables landed inside the "Core Business
Data" card under an "Agnes Internal" accordion alongside Keboola / BQ
buckets — readers conflated system telemetry with business datasets,
and the data_stats header counter ("3 tables · ~X rows total") only
ever counted synced rows so internal tables looked invisible.

Split the catalog page into two cards:
- Core Business Data: only non-internal source_types (Keboola, BQ,
  Jira). Accordions group by bucket as before. Stats counter reflects
  this card's tables.
- Agnes Internal: a dedicated card with its own visual treatment
  (teal accent matching the mode-internal badge in /admin/tables).
  Flat list (no accordion — only 3 rows, never grows here), each
  row carries the canonical `agnes query` snippet. Read-only — no
  profiler click, no In-stack toggle, no sync metadata.

Route adds `internal_card` context object; template renders the new
card only when it's non-None.

* fix(rbac): hide internal tables from /admin/access + drop "my" framing

Two related cleanups for the Agnes-internal tables:

1. /admin/access (resource grants) no longer lists them. The
   `can_access` check has a hardcoded internal-table bypass — security
   is row-level (per-request view filter), so a table-grain
   `resource_grants` row would do nothing. Surfacing them in the UI
   let admins set up grants that silently no-op. Filter at the
   `_table_blocks` projection so the UI tree never sees them.

2. Display names drop the analyst-perspective "my" framing:
     "Agnes — my sessions"          → "Agnes sessions"
     "Agnes — my telemetry events"  → "Agnes telemetry events"
     "Agnes — my audit log"         → "Agnes audit log"
   The "my" only makes sense from the querying analyst's seat
   (`SELECT … FROM agnes_sessions` returns *their* rows); on /admin/*
   pages where admin sees / configures them across users, the
   pronoun was misleading. Description text now spells out the
   row-level RBAC contract explicitly.

Display names update via TableRegistryRepository.register's ON CONFLICT
UPDATE on next app boot; no manual cleanup needed.

* ui: subtitle notes about agnes_* tables on each Activity Center page

The recursive observability story — Agnes serves its own audit /
telemetry / session data through the same `agnes query` plumbing
analysts use for business data — wasn't surfaced anywhere on the
admin pages that show that data. Three pages get a one-liner with
the canonical `agnes query` snippet + the RBAC contract (analysts
see their own rows, admin sees all):

- /admin/activity (Audit log)   → agnes_audit
- /admin/telemetry (Tool usage) → agnes_telemetry
- /admin/sessions               → agnes_sessions

Sets up the discovery moment for admins: they're reading the page,
they see "you can query this from Claude Code", they remember it
when an analyst asks "how do I find my own failed tool calls?".

* ui(tables): explain "Show log" empty-state on /admin/tables

Cache warmup log <pre> renders with a dark background and is only
populated by the SSE stream during a Re-warm all run. Opening the
page cold + clicking Show log just revealed a black bar with no
context — admins couldn't tell what they were looking at.

Adds an inline paragraph above the <pre> explaining what the log is,
the row format, when it fills in, and where to find the historical
audit trail (/admin/activity). The actual <pre> stays empty until
SSE events arrive, but the surrounding copy carries the meaning.

* ui(tables): auto-open cache-warmup log on Re-warm all click

A Re-warm all run takes ~24s per remote BQ row. With the <details>
collapsed by default, operators saw the button disable, watched a
quiet ~24s pass, and assumed nothing had happened — the streaming
log was hidden behind a closed disclosure.

Two small JS tweaks:
- cacheWarmupRun() opens the details on click, so streamed lines
  appear without an extra interaction
- cacheWarmupOnStart() hides the inline hint paragraph the moment
  real log content lands, so the dark log block isn't competing
  with redundant context

Hint paragraph also clarifies that only `query_mode='remote'` BQ
rows are warmed — operators with only materialized/internal tables
would see total=0 and the page would "do nothing" by spec.

* ui: trim Agnes internal copy across surfaces

Descriptions had grown to explain the extraction pipeline ("parsed
out of session JSONLs"), the underlying table ("Backed by
usage_session_summary"), the RBAC mechanic ("row-level RBAC at query
time — analysts see their own; admin sees all"), and the SQL snippet.
Every implementation detail meant another rewrite on the next iter.

Strips to one stable line per surface: what the data is, plus
"Also available locally for analysis". Mechanics live in code +
docs; the page copy says what the user needs to know.

Touched:
- connectors/internal/access.py: INTERNAL_TABLES descriptions
- activity_center.html / admin_usage.html / admin_sessions.html
  subtitles
- catalog.html Agnes Internal card description + row strip
- admin_tables.html "Agnes internal" tab hint

* fix(internal): is_user_admin arity bugs + + saved-view payload cap

Round-1 code review (PR #278) caught two blocking bugs and three nits.

Blocking — both `is_user_admin(user)` (single dict arg) calls raised
TypeError. is_user_admin signature is `(user_id, conn)`. Affected:

- app/api/query.py:_run_internal_query — every POST /api/query that
  references agnes_sessions / agnes_telemetry / agnes_audit blew up
  with a 500. The headline analyst-facing feature of this PR was
  unusable through the API.
- app/api/v2_sample.py — same shape; `GET /api/v2/sample/agnes_*`
  returned 500.

Both fixed to call `is_user_admin(user.get("id"), conn)`. Added two
FastAPI-level tests in test_internal_data_source.py that go through
the TestClient — the existing unit tests on `execute_internal_query`
and `build_filter_clause` skipped the request-handler layer where the
bugs lived, which is why this landed.

Nits also closed:
- connectors/internal/access.py: `+` allowed in _USERNAME_RE /
  _USER_ID_RE so RFC 5321 email local-parts (alice+test@x) resolve
  correctly without hitting InternalAccessError.
- app/api/observability.py: saved-view payload capped at 64 KiB to
  prevent an admin from bloating system.duckdb with a malformed save.

* fix(security): close non-admin data-leak via underlying-table refs

PR #278 R2 review surfaced a non-admin-exploitable bypass: SQL whose
string literal contains 'agnes_sessions' routed into the privileged
internal-query path, then queried the underlying physical table
(usage_session_summary / usage_events / audit_log) directly, escaping
the CTE wrapper's row filter. Two reinforcing defenses:

1. find_internal_refs() now strips single-quoted string literals
   before scanning for alias names — a literal alone no longer
   routes the request into the privileged code path.

2. execute_internal_query() rejects non-admin SQL that references
   the underlying physical tables (usage_*, audit_log). The CTE
   wrapper only scopes the agnes_* aliases; a direct FROM on the
   base table — or a shadowing inner WITH that still has to read
   the base table — bypasses RBAC. Block before execution with an
   actionable error pointing to the agnes_* alias. Admins are
   unaffected (god-mode short-circuit on the filter clause).

3. tests/test_internal_data_source.py — three new negative tests
   covering literal-only matches, direct-table refs, and CTE
   shadow attempts.

Also tightens usage_ask.py's SELECT-only validator: pragma_table_info,
pragma_storage_info, pragma_database_*, and duckdb_tables / columns /
views / indexes / schemas are reflection functions that leak metadata
the analyst question shouldn't reach. \bPRAGMA\b in _FORBIDDEN never
matched the function-call form (word-boundary between `A` and `_`).

* fix(security): dynamic denylist for non-admin internal queries

R3 review (PR #278) caught a wider data-leak than R2: the underlying-
physical-table guard listed only the 7 usage_* + audit_log tables,
but system.duckdb has 30+ other sensitive tables — users (emails +
ids), personal_access_tokens, resource_grants, user_groups,
user_observability_views, store_*, marketplace_*, knowledge_*, etc.
A non-admin SQL like

    SELECT * FROM agnes_sessions
    UNION ALL SELECT email, id, … FROM users LIMIT 1

would leak every user's row.

Replaces the hardcoded denylist with a **dynamic allowlist** —
non-admin SQL may reference ONLY the registered agnes_* aliases.
Every other table in `information_schema.tables` (main schema) is
rejected. Future migrations that add a new sensitive table are
automatically covered without re-editing this module.

Also strips SQL comments (`/* */` and `--`) before the identifier
scan so a comment-wrapped table name (`/**/users/**/`) can't slip
past the regex.

Four new negative tests pin: `users`, `personal_access_tokens`,
block-comment wrap, line-comment wrap.

Plus: per-user view-count cap (100) on /api/admin/observability/views
so an admin can't fill system.duckdb with thousands of saved views.

* release: 0.54.0 — Activity Center + Telemetry + Sessions + internal datasource

Cuts the work shipped across this PR (Activity Center build, recursive
internal data source) into a versioned release. Bumps pyproject.toml
to 0.54.0; renames the top of CHANGELOG.md from [Unreleased] to
[0.54.0] — 2026-05-12 with a header summary; opens a fresh
[Unreleased] section for the next round.

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 22:41:19 +02:00
Vojtech
fb6e930bc9
feat(store-guardrails): per-component description quality + plain-language UX (#276)
* feat(store-guardrails): enforce per-component description quality

Two-tier hard guardrail on flea-market submissions. Empty / placeholder /
single-word descriptions now block before any LLM call; vague-but-passes-
floor descriptions block on the substantive LLM review layer.

Tier 1 — inline mechanical check (src/store_guardrails/content_check.py).
Walks the baked plugin tree, evaluates each component (plugin manifest,
agents, skills, commands) plus the submission-level form description
against a 60-char / 25-char (commands) / 5-distinct-word / 200-char-body
floor with a placeholder denylist (TODO, TBD, {{var}}, etc.). Floors
calibrated against real ecosystem norms: Claude / superpowers /
compound-engineering skill packs cluster 150–220 chars, npm / Docker /
VS Code at 100–120. InlineResult.passed now ANDs in content.status.

Tier 2 — LLM review extension (prompts.py + llm_review.py). System
prompt gains a content-quality criterion; REVIEW_JSON_SCHEMA carries a
content_quality {verdict, issues[]} object alongside the existing
security findings. is_safe() requires content_quality.verdict == 'pass'.
Single LLM call covers both dimensions. MAX_RESPONSE_TOKENS bumped
2000 → 2500 for the extra payload. Verdicts missing content_quality
treated as pass (backwards compat with already-recorded rows).

Submitter UX:
- /store/new wizard now carries a "Before you upload — what passes
  review" collapsible disclosure on both step 1 and step 2 with the
  bar + patterns that work. Live char counter on the description
  field. Per-component preview table (green/red dots from the new
  summarize_for_preview helper) renders after the ZIP /preview round
  trip, scoping each finding to its file.
- New /store/examples page with rejected/passes pairs for skill /
  agent / plugin / command plus a "Why these limits" research table.
  Anchored sections (#skill / #agent / #plugin / #command) so the
  rejection banner can deep-link by component_type.
- Quarantine banner _content_findings.html groups findings by file
  (one "See <type> example ↗" per component, not per field) and
  translates field codes (frontmatter.description / body / etc.) to
  plain-English labels. _content_howto_fix.html surfaces a static
  "Re-upload as new version" + "See examples" action row beneath any
  content failure on the entity detail page.
- _parse_frontmatter moved to src/store_guardrails/_frontmatter.py so
  the new check module shares the parser without inverting the
  app → src dependency direction.

Tests:
- New tests/test_store_guardrails_content.py (29 cases) covering
  every failure code per component type plus submission-level checks
  and the summarize_components / summarize_for_preview helpers.
- Extended test_store_guardrails_inline.py for the new
  InlineResult.content field + aggregate behaviour.
- Extended test_store_guardrails_llm.py for the new
  content_quality verdict pathways (fail blocks, missing field passes).
- Backfilled fixture descriptions across test_store_api.py,
  test_store_entity_versions.py, test_store_put_atomic.py,
  test_admin_store_submissions.py, test_marketplace_api.py,
  test_marketplace_v32_endpoints.py so existing happy-path tests
  clear the new 60-char floor.

* fix(content-guardrail): align agents walker with preview + drop import-time .format()

Two cleanups from the takeover review on #276 (vr/guardrails-content).

1) `_iter_components` for agents now skips files lacking frontmatter
   (no `name` AND no `description`). Pre-fix the walker greedily
   evaluated every `*.md` under `agents/` — `agents/README.md` and
   helper docs got flagged as "frontmatter.description empty"
   rejections. Worse: `summarize_for_preview` for `type=agent` ALREADY
   filters the same shape, so the upload preview gave a green dot
   while the post-bake check gave a red rejection on submit. Two new
   regression tests in TestAgentsWalkerSkipsNonAgentFiles pin both
   shapes (README + _NOTES.md) so the preview/check parity stays
   aligned.

2) `body_too_short` hints now use the same runtime-kwarg substitution
   pattern as every other hint in the table. Pre-fix the skill +
   agent body_too_short hints called `.format(min_chars=_MIN_BODY_CHARS)`
   at module-load time, but the call site `_hint_for(type_,
   "body_too_short")` didn't pass `min_chars=`, so the format() was
   just baking the constant at import. Cosmetic inconsistency; pass
   `min_chars=_MIN_BODY_CHARS` at the call site instead and let
   `_hint_for` do the substitution like it does for `too_short`.

Verified end-to-end:
- New TestAgentsWalkerSkipsNonAgentFiles cases fail on the unfixed
  walker (verified by reverting to the pre-fix file and re-running);
  pass cleanly after the fix.
- Full content-guardrail suite: 25/25 (23 existing + 2 new).
- Full pytest: 4189 passed, 25 skipped.

* release: 0.53.5 — content guardrail (flea-market submitter UX) + catalog ENTITY column + BQ hint dispatch

Bundles three threads landed in [Unreleased]:
- Vojta's flea-market content guardrail (two-tier mechanical + LLM)
- Zdeněk's `agnes catalog` ENTITY column replacement for FLAVOR
- Zdeněk's `/api/query` remote_estimate_failed hint dispatch fix

Plus the takeover hygiene from #276 review (agents walker preview/check
parity + body_too_short hint runtime kwarg consistency) and the
backslash-escape fix follow-up to v0.53.4 #275.

No DB migration; no API change. Patch upgrade lands transparently.
Upload form's new "Before you upload" disclosure + per-component preview
table appear on the next dev-VM auto-pull. Quarantine banner now groups
findings by file with "See <type> example ↗" deep-links to the new
/store/examples reference page.

---------

Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
2026-05-12 21:48:27 +02:00
Vojtech
929520f5e1
Flea-market edit feature with version history (schema v37) (#239)
* feat(store): flea-market entity edit feature with version history (schema v38)

Owner + admin can now edit a store entity from a real Edit page at
/marketplace/flea/{id}/edit, replacing the prior "coming soon"
placeholder. Editable: display name, description, category, video
URL, cover photo, and an optional new bundle. Type is locked (400
type_locked). Display-name change renames the on-disk slug for both
live plugin/ and version dirs (reuses rename-on-archive helper).

Schema v38 (originally drafted as v37; renumbered after rebase onto
main where v37 was taken by the curated marketplace enrichment).

Versioning model:
* Each bundle update bakes into ${DATA_DIR}/store/<id>/versions/v<N+1>/plugin/
  and runs the standard guardrails pipeline.
* DEFERRED PROMOTION: live plugin/ + entity.version_no stay at the
  prior approved version through the LLM review window so existing
  installers keep receiving the previously approved bundle. Live swap
  + version_no/version/file_size bump happen only on LLM approval.
  Blocked verdicts leave the prior version serving forever.
* store_entities gains version_no INTEGER + version_history JSON.
  Each version_history entry carries hash, sha256, size, submission_id,
  created_at, created_by.
* Existing entities backfill to v1 with a single-entry history seeded
  from the row's current `version` hash. Initial create also seeds
  versions/v1/plugin/ so future restore can copy v1 bytes forward.

Concurrency:
* Block-while-pending: an in-flight LLM review blocks any further edit
  with 409 prior_version_pending. Owner waits 5-30s; Edit button on
  detail page renders disabled in the same window via the new
  edit_in_flight flag (decoupled from quarantine_sub since the
  deferred-promotion flow keeps visibility='approved').

Rollback:
* New endpoint POST /api/store/entities/{id}/versions/{n}/restore
  (owner + admin). Copies vN bundle forward as v<max+1> and re-runs
  guardrails (rules tighten over time; pre-approved bundles re-validate).
  Forward-only history. Same deferred-promotion semantics — live stays
  at prior version until LLM approves the restored copy.

UI:
* New /marketplace/flea/{id}/edit page (owner + admin gated).
* Versions card on plugin + item detail templates (owner/admin only)
  via shared _flea_versions.html partial.
* Admin queue gains v# column with current badge + separate Hash
  column. Submission detail surfaces Version + Bundle hash rows.
* Activity timeline split into per-submission + entity-wide cards;
  entity-wide rows render vN chips when audit row params reference
  a specific version.
* Section headers (Manifest / Static / Quality / LLM review) tag
  with vN chip via shared macro.
* Reviewed-by-model field surfaces explanatory text per status.
* Banner upload-failure now redirects to detail page on
  submission_blocked instead of staying stuck.

Tests: 24 in tests/test_store_entity_versions.py covering metadata-
only edit, bundle-edit version bump, type lock, block-while-pending,
name change disk rename, restore flow + 404/400/403 paths, edit page
404 for non-owner, versions card visibility gating, admin queue v#
column, admin detail Version/Hash rows, deferred-promotion installer
contract (pending review doesn't break installer / blocked verdict
keeps prior / approved promotes), admin can edit/restore non-owned,
restore deferred promotion, audit log per-version params. 214 tests
green across guardrails + edit + admin + repo + schema suites.

* docs(store): refresh update_entity docstring to match deferred-promotion + submission-status gate

Bring the docstring in sync with the actual fixes from the prior
commit. The pre-fix wording said the gate read
visibility_status='pending' AND submission status — under deferred
promotion that would never fire for v2+ edits. Now describes:

- Block-while-pending gates on submission.status DIRECTLY,
  independent of visibility (so v2+ deferred-promotion edits don't
  slip through).
- Display-name + bundle change defers the live rename to promotion;
  metadata-only renames stay immediate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-10 00:14:33 +04:00
minasarustamyan
6fe67d5279
Curated marketplace enrichment via agnes-metadata.json + curator metadata (#234)
* Curated marketplace enrichment via agnes-metadata.json + curator metadata

Adds a second well-defined metadata file `.claude-plugin/agnes-metadata.json`
that upstream marketplace repos can opt into, providing per-plugin (and
per-skill / per-agent) cover photo, demo video URL, doc links, and
category override. The Claude Code marketplace contract is untouched —
agnes-metadata.json + the convention `.agnes/` directory are stripped
from the synthetic Claude Code marketplace served via /marketplace.zip
and /marketplace.git/*, so user instances see a clean Claude Code repo
with no Agnes-only metadata.

Highlights:
- DB schema v32 — adds curator_name + curator_email on marketplace_registry,
  cover_photo_url + video_url + doc_links on marketplace_plugins.
- Mandatory curator at marketplace registration, editable later through
  the admin UI; surfaces on cards + detail pages in place of owner_todo.
- External-asset mirror cache at ${DATA_DIR}/marketplace-cache/<slug>/
  with conditional GET, 60s timeout, 10 MB body cap, SSRF guards, and
  Wikipedia-policy-compliant User-Agent.
- Strict drop semantics — anything Agnes can't deliver as a real PDF /
  Markdown / plain text doc, or a real PNG / JPEG / WebP cover, is
  dropped from the served metadata; UI looks identical to no-entry case
  (gradient placeholder for missing covers, no row in the doc list).
- Doc allowlist + image allowlist enforced on both the curated mirror
  flow and the Flea upload flow (/store/new); shared module
  src/marketplace_assets.py.
- New /api/marketplace/curated/{mp}/{plugin}/{asset,doc,mirrored}/...
  endpoints with path-traversal guards + RBAC + Content-Disposition
  attachment for docs.
- Curator-focused format guide at /marketplace/format-guide; canonical
  source is docs/curated-marketplace-format.md, also linked from the
  admin /admin/marketplaces page next to + Add Marketplace.

See CHANGELOG.md under [Unreleased] for the full breakdown.

* Fix format-guide test assertion to match shortened disclaimer

The 'Flea Market' phrase was trimmed out of the disclaimer in
docs/curated-marketplace-format.md after the curator-focused rewrite.
Update the rendered-HTML test to assert the channel-scoping phrase
that's actually present ('Curated Marketplace channel only') rather
than the 'Flea Market' contrast that's no longer in the doc.

* Drop unused 'version' field from agnes-metadata.json schema

The parser never read it; it was a YAGNI placeholder for future
schema evolution. Curators don't need to wonder what to put there
when adding the file for the first time. Will be re-added if and
when we actually introduce a backwards-incompatible schema change.

* Harden asset mirror against SSRF via redirect + DNS rebinding

The pre-flight _is_safe_url check validated only the initial URL;
urllib.request.urlopen then followed redirects and re-resolved DNS for
the actual connection — both bypassable. Attacker-controlled origin
could 302 to http://169.254.169.254/... and exfil cloud metadata;
attacker-controlled DNS could return public IP first / 127.0.0.1 second.

Replace urlopen call with a shared OpenerDirector wired through three
custom handlers: _SafeRedirectHandler re-runs SSRF allowlist on every
redirect Location (max 5 hops, down from urllib's 10), and
_PinnedHTTPHandler / _PinnedHTTPSHandler connect to the IP that passed
validation rather than re-resolving the hostname. TLS SNI + cert verify
stay bound to the original hostname.

_resolve_safe returns the validated IP (the existing _is_safe_url
2-tuple wrapper stays for backwards compatibility) and rejects round-
robin DNS that mixes a public + private record. _UnsafeRedirectError
is a typed exception so _fetch_url can map redirect blocks to terminal
'rejected' status (not transient 'failed'). _http_open is the single
call site so tests can mock at one well-defined seam.

Tests cover redirect blocking (link-local, loopback), redirect-error
unwrapping inside URLError, pinned-IP connection target, and the
end-to-end DNS-rebinding scenario. Existing tests that mocked
urllib.request.urlopen are migrated to mock _http_open.

* Harden /asset/ endpoint against stored XSS

The endpoint served any file in the cloned marketplace repo with
stdlib-detected Content-Type, so a curator who landed evil.html (or a
renamed evil.png carrying HTML bytes) in the working tree got a
same-origin XSS — the response shares cookie scope with /admin and
/api/me/*.

The asset endpoint is image-only by contract (cover photos referenced
from agnes-metadata.json + inner skill / agent cards), so applying the
same allowlist + magic-bytes pattern that /doc/ already uses closes
the gap without breaking any legitimate use case. Three layered
checks: extension in IMAGE_EXTENSIONS (.png/.jpg/.jpeg/.webp; SVG
excluded — <script> inside SVG executes), validate_image_file magic
bytes (defeats rename-extension attack), Content-Type pinned from the
validated extension (never stdlib mimetypes).

Defense-in-depth: X-Content-Type-Options: nosniff stops browser MIME
sniffing; Content-Security-Policy: default-src 'none' blocks script /
iframe execution even if a future regression let HTML through.

Tests cover the .html extension reject, the renamed-HTML-as-PNG magic-
bytes reject, the .svg reject, and the happy-path PNG with security
headers attached. The pre-existing path-traversal test seeds a real
PNG instead of ok.txt now that the endpoint is image-only.

* Enforce mandatory curator on marketplace PATCH

The POST handler enforced curator_name + curator_email at create time,
but PATCH treated empty / missing curator inputs as 'no change'. Legacy
rows that pre-date v32 (curator_name=NULL) could be edited indefinitely
without ever filling the curator gap, and OWNER_TODO_PLACEHOLDER lingered
on every /marketplace card.

Reject the PATCH with 400 when the post-merge row would persist with
empty curator. The check fires after the existing field-merge logic, so
once-filled rows that don't touch curator still pass through (their
existing values fall through from the DB row). DB column stays nullable
so untouched legacy rows continue to coexist — the gate fires only the
moment an admin opens the edit modal.

Existing PATCH semantics preserved: empty-string input still means 'leave
existing value alone', and once-filled curator can't be cleared (those
test cases pass unchanged). New test seeds a legacy row directly via the
repository, then exercises url-only PATCH (rejected), partial-fill PATCH
(rejected), and full-fill PATCH (succeeds); a follow-up no-curator PATCH
on the now-formed row also passes.

* Drop unused curated-marketplace helpers (PR #234 review)

* build_db_payload — imported by src/marketplace.py but never called.
  The strict-drop semantics it would have implemented were re-written
  inline in _refresh_plugin_cache (see the comment block there). The
  standalone helper still carried the old fall-back-to-original-external-
  URL-on-mirror-failure behaviour, which contradicts the documented
  drop-when-can't-deliver contract — a future contributor who re-wired
  it would have introduced a silent regression. Delete with the helper
  + the import + the comment that referenced it.

* _resolve_marketplace_name — one-line shim with no remaining call
  sites. Callers use _resolve_marketplace_meta which returns name +
  curator together, avoiding the double DB hit the shim exists to
  hide.

* '# noqa: F401  Optional kept for forward-compat' was wrong — Optional
  IS used in src/marketplace.py (line 70 and line 238). Drop the noqa
  comment so a future ruff run doesn't try to remove a real import.

Removing build_db_payload also drops the only remaining use of Optional
in src/marketplace_metadata.py, so the import comes out there too.

* Cap agnes-metadata.json size + catch RecursionError on parse

The reader is invoked once per marketplace per sync and the file is
curator-controlled. Two failure modes were unguarded:

* Multi-GB JSON: path.read_text() pulled the whole file into memory
  before json.loads even ran. A curator with commit access to an
  upstream repo could OOM the sync worker.

* Deeply-nested JSON under any size cap: cpython's recursive object /
  array parser raises RecursionError at ~1000 levels of depth.
  RecursionError is a RuntimeError, not ValueError, so the existing
  catch let it propagate up and abort the entire sync — every other
  marketplace in the same pass got skipped.

Add AGNES_METADATA_MAX_BYTES = 1 MiB (a real metadata file with covers,
docs, categories for ~50 plugins fits in <100 KB so the cap is
generous) and gate the size check on path.stat().st_size before the
body read. Broaden the parse except to (ValueError, RecursionError)
with a unified log line. Both failure modes degrade to the same
empty-dict fall-back the malformed-JSON path already used, so one bad
upstream never aborts the rest of the sync.

Tests cover the size cap firing before json.loads (whitespace-padded
valid JSON exceeding the cap) and the recursion path (5000 nested
arrays — past cpython's default recursion limit but well under the
size cap).

* Persist asset-mirror manifest per body write, before unlink

sync_assets wrote each body atomically (tmp + rename) but persisted
the manifest only at the end of the batch. A kill -9 mid-Phase 2 left
on-disk files the manifest never referenced. Once a curator dropped
that URL from agnes-metadata.json, Phase 3's cleanup had no record of
the file and the orphan stayed forever — there's no GC pass walking
the cache dir today, so disk would slowly bloat.

Phase 2 (body-write iteration): after the in-memory manifest mutation,
persist BEFORE unlinking the previous body. The crash window narrows
from 'all of Phase 2' to 'between persist and unlink' (microseconds).
A persist failure mid-batch keeps the previous body on disk — the on-
disk manifest still references it, and a stale-but-existing file beats
a 404. Cost: one extra tmp+rename per body write; manifest is a few KB
so the overhead is negligible vs. the HTTP fetches.

Phase 3 (curator-removed URLs): same discipline. Collect the to-delete
relpaths, persist the manifest with the entries already gone, THEN
unlink. A crash mid-cleanup leaves at most a microsecond window where
files exist despite the manifest no longer naming them. The next sync
reads the (correct) manifest and the orphan stays orphaned, but the
served state is consistent.

Tests cover per-body persist call count, the post-update on-disk
manifest content, and Phase 3 ordering verified by reading the on-disk
manifest from inside Path.unlink.

* Consolidate marketplace video embeds + format-guide CSS

The YouTube nocookie / Vimeo / <video> / link-fallback detection logic
was duplicated verbatim in marketplace_plugin_detail.html and
marketplace_item_detail.html (~40 JS lines each, with subtly-different
inline styles). Both templates now {% include %} a single
_marketplace_video_embed.html partial inside their IIFE so the regex,
the nocookie attribute set, and the unknown-host link fallback live in
ONE place — future tweaks (new host, new attribute, fixed sandbox flag)
no longer need to be applied twice in lockstep.

The .video-wrap selectors (one inline <style> rule in plugin_detail,
one inline style='...' attribute in item_detail) are replaced by the
existing .video-embed 16:9 wrapper in style-custom.css, with new
.video-embed video / .video-embed a child rules added so the wrapper
handles all four embed shapes uniformly without per-template
positioning.

The 60-line inline <style> block in marketplace_format_guide.html
moves verbatim to style-custom.css under a new 'Marketplace format
guide page' section, scoped to .format-guide so other pages aren't
affected.

No user-visible behaviour change: the rendered HTML for valid
YouTube / Vimeo / mp4 / external links is byte-identical to before,
and the format-guide page renders the same.

* Maintainability cleanup batch (PR #234 review)

#10: drop _path_under from app/api/marketplace.py — it was a byte-
equivalent clone of _safe_join (same Path.resolve(strict=True) +
relative_to() containment check). The three v32 endpoint handlers
(/asset, /doc, /mirrored) now share the existing helper.

#14: rename src/marketplace_assets.py → src/marketplace_asset_validation.py
so the file's purpose is obvious from the name and the previous
overlap with src/marketplace_asset_mirror.py is gone. Six call-site
imports updated in lockstep; CHANGELOG references under [Unreleased]
updated to track the new path.

#11: consolidate the URL builders that resolve
/api/marketplace/curated/<slug>/<plugin>/{asset,doc,mirrored}/...
paths. _internal_asset_url / _internal_doc_url / _mirrored_asset_url
lived in src/marketplace.py, while a copy named _mirrored_url lived
in app/api/marketplace.py with a 'must stay aligned' comment. New
module src/marketplace_urls.py is the single source of truth — both
call sites import from it and a future URL-format tweak only needs
to change one file. The _ROUTE_PREFIX constant collapses the per-
function f-string repetition. The route-handler endpoints themselves
still own the path string literals (keeping the builders identical
to the route declarations remains a checklist item, not a runtime
guarantee).

* Re-key asset-mirror manifest by (plugin, url) + dedup HTTP fetches

The manifest used to be keyed by URL alone, so two plugins in the
same marketplace referencing the same external image (a shared CDN
icon, a common cover) collided on entry.plugin_name — last writer
won. The DB row for the losing plugin then stored a served URL
pointing under the winning plugin's tree, and require_resource_access
denied legitimate access on one side and let the other plugin's user
reach the wrong asset.

In-memory: Dict[Tuple[str, str], MirrorEntry] keyed (plugin_name, url).
On disk: format flips from {url: entry} dict to [entry, ...] list of
self-describing entries (each carries plugin_name + url + the
previous fields). JSON keys can't be tuples; encoding 'plugin::url'
would just shift the parsing burden.

Phase 1 of sync_assets deduplicates fetches by URL — three plugins
sharing one URL share one HTTP request. The conditional-GET prior is
picked from any owning plugin's prior entry; if their etags diverge
(rare) we miss one 304 and pay for a full re-download instead.
Phase 2 still creates a per-(plugin, url) manifest entry pointing
under the plugin's own subdir, and Phase 3 cleanup is keyed the same
way so dropping a URL from one plugin's metadata doesn't disturb
another plugin still referencing it.

Body files stay per plugin (RBAC-clean isolation: deleting plugin A's
cache can't strand plugin B). Bandwidth saved by fetch dedup.

Consumer code re-keyed: src.marketplace._refresh_plugin_cache rebuilt
served_url_for / mirror_status as composite-keyed maps;
app.api.marketplace._resolve_external_via_mirror /
_curated_inner_cover / _curated_inner_enrichment look up by
(plugin_name, url).

Tests cover per-plugin manifest entries with shared URL, the single
HTTP fetch for N plugins, and Phase 3 drop-one-keep-other. All
existing tests migrated to composite key access; v2 list format
assertions verify on-disk shape.

* Migrate asset mirror from urllib.request to httpx

The asset mirror was the only HTTP call site in Agnes still using
urllib.request; every other module (CLI, Jira / OpenMetadata / OpenAI
connectors, scheduler, Telegram bot) already used httpx. The asset
mirror was added in this PR's base commit, so this is the only chance
to bring it into convention before someone copies it as 'the pattern
for HTTP fetches in Agnes'.

Three concrete benefits beyond consistency:

* SSRF defence collapses from five urllib classes
  (_PinnedHTTPConnection, _PinnedHTTPSConnection, _PinnedHTTPHandler,
  _PinnedHTTPSHandler, _SafeRedirectHandler) into one
  _SSRFGuardTransport. httpx invokes handle_request() on every redirect
  hop, so re-validation is free — we don't need a custom redirect
  handler at all.

* DNS-rebinding defence: the transport rewrites request.url.host to the
  SSRF-validated IP before delegating to super().handle_request().
  httpcore connects to whatever URL.host says, so this pins the
  connection without subclassing HTTPSConnection. The original hostname
  goes into the Host header + the sni_hostname extension so TLS / vhost
  routing still bind to the curator-supplied hostname.

* Error handling: one httpx.HTTPError catch-all for transport errors,
  plus specific httpx.TimeoutException / httpx.TooManyRedirects branches
  for clearer diagnostics. Matches the _translate_transport_error shape
  in cli/client.py.

The shared httpx.Client is built lazily at module load (same pattern as
cli/client.py:_get_shared_client) with follow_redirects=True,
max_redirects=5, timeout=HTTP_TIMEOUT_SEC, and our custom transport.

Externally observable behaviour is unchanged: same FetchOutcome
statuses, same manifest format, same conditional GET semantics, same
body-size cap.

Tests migrated from urllib-shaped fakes to httpx-shaped (status_code,
iter_bytes, context manager). Five urllib-specific tests replaced with
httpx equivalents — three transport unit tests + one DNS-rebinding
integration test that verifies host rewrite via monkey-patched
super().handle_request. One test deleted without replacement
(unwrap-URLError-wrapping-an-_UnsafeRedirectError — urllib-specific,
not applicable to httpx).

* Surface curated agnes-metadata enrichment on My Stack tab

GET /api/marketplace/items?tab=my built each curated row from the
on-disk marketplace.json by way of resolve_allowed_plugins, which
doesn't carry the agnes-metadata enrichment columns
(cover_photo_url, video_url, category override, doc_links). The
handler then hard-coded cover_photo_url=None on the synthetic row.
Result: once a user clicked '+ Add to my stack' on a curated card,
the same plugin in tab=my rendered with the gradient placeholder
instead of its cover photo — confusing parity break vs. the curated
tab where the same row goes through MarketplacePluginsRepository
and gets the enriched columns.

Pre-load the enriched marketplace_plugins rows for every marketplace
the user is subscribed to, then look each granted+subscribed plugin
up by (marketplace_id, plugin_name). Fall back to the on-disk
synthetic shape only when the DB row is missing — happens during
the rare race where RBAC is granted before the first sync cycle
ingests the plugin. RBAC gating (granted set from
resolve_allowed_plugins) is unchanged so this fix can't widen
visibility; it just upgrades the data shape behind cards the user
was already going to see.

Per-marketplace list_for_marketplace beats N gets — typical user is
subscribed to <5 marketplaces, so this is at most a handful of
queries vs. one per subscribed plugin.

Regression test seeds a plugin with cover_photo_url + category
override, subscribes the user, hits /api/marketplace/items?tab=my,
and asserts photo_url + category come through. The misleading
'fall through to gradient until the user re-visits the curated tab'
comment is gone.

---------

Co-authored-by: Minas Arustamyan <arustamyan.minas@gmail.com>
2026-05-09 17:01:37 +02:00
Vojtech
d6ad08f107
Flea-market upload guardrails + soft delete + JOIN-based admin queue (#233)
* feat(store): flea-market upload guardrails + soft delete + JOIN-based admin queue

Adds an end-to-end guardrails pipeline for store uploads (manifest +
static-security + LLM review), persists blocked bundles for forensics,
introduces soft-delete (Archive) semantics, consolidates the legacy
/store/{id} surface into /marketplace/flea/{id}, and reworks the admin
queue so lifecycle filters read live entity visibility via LEFT JOIN
rather than a denormalized submission column.

Schema v29 → v35:
  * v29 store_submissions table + store_entities.visibility_status
  * v30 file_size, bundle_sha256, bundle_purged_at on submissions
  * v31 reshape store_submissions (drop legacy unique on entity_id)
  * v32 store_entities.archived_at/by + 'archived' visibility value
  * v33 drop store_submissions.retry_count (unused)
  * v34 ensure idx_store_submissions_entity exists post column-drop
  * v35 broaden visibility_status enum + JOIN architecture cutover

Pipeline (src/store_guardrails/):
  * Inline checks: manifest_check, static_scan, quality_check
  * LLM review configurable haiku|sonnet|opus (default haiku)
  * BackgroundTasks-driven async path with structured-output JSON
  * Per-submitter daily quota (default 50)
  * 30-day TTL purge job (POST /api/admin/run-blocked-purge)
  * Bundle SHA256 + size persisted; sha256 survives purge for forensics

Visibility model:
  * pending | approved | hidden | archived
  * _enforce_visibility returns 404 (no leak) for non-owner non-admin
  * Owner sees own non-approved entries via include_owner_id widening
  * Install refused with 409 entity_not_approved when not approved

Soft-delete (DELETE /api/store/entities/{id}):
  * Default = soft (visibility_status='archived'); existing installs
    keep getting served the bundle so users don't lose the plugin
  * ?hard=true admin-only: drops bundle + cascades user_store_installs
  * Hard-delete preserves entity_id on submission as tombstone so
    audit_log linkage survives for the activity timeline

Admin queue lifecycle (the JOIN refactor):
  * Verdict (store_submissions.status) is immutable forensic record
  * Lifecycle (store_entities.visibility_status) is live state
  * /admin/store/submissions Archived chip translates to
    `e.visibility_status='archived'` via LEFT JOIN — any path that
    flips visibility surfaces in the queue immediately
  * Detail page renders Status (verdict) and Entity lifecycle side by
    side so admins see "approved at review, now archived" at a glance

URL consolidation:
  * /store/{id} deleted (no redirect, stale bookmarks 404)
  * /marketplace/flea/{id} is the canonical detail surface
  * Three in-tree callers (upload-success, my-stack card, store
    listing card) updated to point at the new URL
  * Quarantine banner extracted to _quarantine_banner.html partial,
    self-guarded, included from both flea detail templates
  * Banner JS auto-refreshes when the verdict lands by polling
    /api/marketplace/flea/{id}/detail (visibility_status +
    submission_status — the latter is needed because blocked_llm
    keeps the entity at visibility_status='pending')

Audit log resource format:
  * runner.py emits prefixed `store_submission:{id}` (post-fix)
  * Detail-page timeline query handles three patterns: prefixed
    submission, helper-emitted `store_entity:{sub_id}`, and bare-id
    legacy rows — all surface in the activity timeline

UX fixes:
  * Owner sees Under review / Quarantined / Hidden banner with status
  * Install button gray-disabled (not blue) when non-approved
  * Owner cannot delete quarantined entries (403); admin can
  * Admin queue: filter chips, sortable columns, paging, page-size
  * Auto-refresh queue every 5s while pending rows are visible
  * Store upload page file picker no longer opens twice (label →
    input default action collided with explicit JS handler)

Tests: 168 passed across the guardrails suites (admin submissions,
store API, inline / LLM / purge guardrails, store repositories,
marketplace filter, schema version). New regression coverage
includes: archive surfaces via JOIN even when API path is bypassed;
deleted submission renders activity timeline (tombstone); flea
detail surfaces submission_status only for owner/admin; detail page
renders Entity lifecycle row; audit log resource format covers both
helper and runner paths.

* fix(store-guardrails): PR #233 follow-up — prompt injection, atomic PUT, BG race, schema, reaper, sort whitelist

Addresses 9 of the 23 findings from the PR #233 review (spec at
docs/superpowers/specs/2026-05-09-pr233-guardrails-fixes-spec.md).
Merge-gate items #1-#6 plus high-value mediums #7, #9-#12, #23.
Architectural items (#8 enum split, #14 factory) and pure
maintainability (#15-#22) deferred to follow-ups.

Security:
* #1 prompt injection — SYSTEM_PROMPT now passed via the SDK's
  dedicated system= parameter; bundle wrapped in <bundle>...</bundle>
  sentinels declared data-only by the system prompt; literal
  sentinel strings in user content are escaped so an adversarial
  README can't forge a close tag.
* #6 static scan honesty — module docstring + admin copy + docs
  declare static scan as signal not gate; .md/.txt/.rst/.html/.json/
  .yaml/.yml/.toml skipped to avoid false positives on prose.
  AST mode for Python deferred (separate flag, FP comparison work).

Correctness:
* #2 PUT atomicity — bundles bake into plugin.staging-<rand>/
  alongside live, atomic-rename on success; failed checks leave
  live tree byte-for-byte intact.
* #3 BG-task race — set_visibility_if_pending guards verdict flips
  to the (pending, hidden) review window; admin archives during
  review survive; skipped flips audit-logged.
* #4 v35 NOT NULL/DEFAULT — schema v35→v36 re-applies them on
  store_entities.visibility_status. CHECK constraint enforced
  application-side (DuckDB ADD CHECK on existing column unsupported).
* #7 stuck-review reaper — reap_stuck_llm_reviews flips pending_llm
  rows older than guardrails.stuck_review_grace_seconds (default
  1800) to review_error. Scheduler runs every 15 min via new
  /api/admin/run-reap-stuck-reviews. Set knob to 0 to disable.
* #9 quota counter — count_blocked_for_submitter_since now counts
  blocked_inline + blocked_llm + review_error so a submitter
  triggering only LLM-blocked verdicts is bounded.
* #10 missing risk_level — surfaces as review_error with
  error='missing_risk_level' instead of silently defaulting to
  'medium' (which looked like a model-decided block).
* #11 archived_at clear — set_visibility nulls archived_at +
  archived_by when transitioning out of 'archived' so a future
  read doesn't show stale archive forensics on an approved row.

Maintainability:
* #12 FSM doc comment — accurate insert/transition/lifecycle
  description in src/db.py near store_submissions schema.
* #23 sort-key whitelist — admin queue rejects unknown sort keys
  with 400 invalid_sort_key; substring-replace footgun removed.

Deferred (separate PRs):
* #5 quota race — proper fix requires asyncio.Lock spanning the
  full pipeline; threading.Lock blocks event loop, DuckDB MVCC
  doesn't help. API-level slowapi bounds worst case for now.
* #6 part 3 (AST static scan), #8 (enum split), #13 (import
  bundle docs), #14 (factory consolidation), #15-#22 (maint).

Tests:
* New: tests/test_store_guardrails_prompt_injection.py (corpus +
  trust-boundary invariants), tests/test_store_put_atomic.py,
  tests/test_store_guardrails_reaper.py.
* Extended: test_store_guardrails_llm.py (system param, missing
  risk_level, BG race), test_admin_store_submissions.py (quota
  counter widening, sort whitelist 400), test_store_repositories.py
  (un-archive metadata clear), test_db_schema_version.py (v36).
* Full suite: 3738 passed; 17 pre-existing baseline failures
  unchanged (db migration tests, cli binary rename, catalog export,
  user mgmt v5 backfill — confirmed by stash + rerun on clean tree).
2026-05-09 17:32:53 +04:00
ZdenekSrotyr
8d8d2c219e refactor(cli-store): pull/info → agnes admin store; add agnes store mine
Backup-orchestration commands were split across two namespaces (pull in
agnes store, push in agnes admin store), which broke the operator
mental model — pull/push are a paired operation and should sit
together.

Move pull + info into agnes admin store so all bulk operations share
one help screen. Add agnes store mine as the user-facing equivalent —
calls the same /api/store/bundle.zip endpoint with ?owner=me, which
the server resolves to the caller's user_id. Authors can archive
their own uploads without admin role; whole-Store bulk reads stay
admin-flavored as a discoverability hint.

Server: 3-line addition to export_bundle handles owner='me' as a
magic alias for the caller. No new endpoint.

Tests updated: pull/info expectations move from agnes store to
agnes admin store; new tests cover agnes store mine and the
?owner=me server resolution. 69/69 store tests green locally.
2026-05-05 13:49:18 +02:00
ZdenekSrotyr
a8f9d065c8 feat(store): bundle export/import + agnes store update + agnes admin store push
Adds whole-Store backup/restore primitives so an external CI/CD job can
mirror the Store to a git repo (and restore back from one).

REST:
- GET /api/store/bundle.zip — deterministic ZIP of all (filtered) Store
  entities. Layout: manifest.json + entities/<id>/{plugin,assets}/.
  Manifest carries owner_email for cross-instance restore. Auth: any
  authenticated user (Store is community-open).
- POST /api/store/import-bundle — admin-only restore. Modes
  merge|replace|skip; owner resolution by email with stub-disabled-user
  fallback when the email is unknown on the target instance.

CLI:
- agnes store update <id> [--description X] [--zip PATH] ... — in-place
  edit (server PUT permits owner OR admin per F4). Closes the missing
  edit affordance for analysts who want to fix a typo or push a new
  ZIP without losing install_count.
- agnes store pull [-o store.zip] [--unpack DIR] — download the bundle.
  --unpack streams + extracts so an external git-backup workflow can
  drop the tree straight into a repo and `git add .`.
- agnes store info [--json] — counts + size summary.
- agnes admin store push <zip-or-dir> [--mode ...] — admin-only restore.
  Auto-zips a directory client-side so a working-tree → server
  round-trip is one command.

cli/v2_client.py gains api_get_stream helper for binary downloads.

Tests: 5 new server tests (bundle shape + filters + round-trip + stub
user creation + skip mode + admin-only gate) + 11 new CLI tests
(update, pull/unpack, info, admin push). 66/66 store-related tests
green locally.
2026-05-05 11:51:31 +02:00
ZdenekSrotyr
f0d091f721 fix(store): scratch dir leak on ZIP validation failure (Devin Review)
create_entity + update_entity created the `scratch` temp dir inside one
try/finally but cleaned it up in a separate one. Validation HTTPExceptions
raised by _safe_zip_extract (zip_unsafe_path, zip_too_large_uncompressed)
or the BadZipFile→422 conversion exited the first scope, and the second
finally was never entered → temp dir leaked on every failed upload.

Devin flagged this on the F2 commit. The leak pre-existed (zip_unsafe_path
was the original vector); F2 added zip_too_large_uncompressed to the same
broken cleanup path. Fixed by collapsing scratch creation + cleanup into
one outer try/finally that covers both extraction AND metadata/bake; the
inner try/except/finally still handles BadZipFile→422 + tmp file cleanup.

Same restructure in update_entity. Regression test
`test_scratch_dir_cleaned_up_after_failed_extraction` triggers a
zip_unsafe_path 422 and asserts tmp/agnes_store_* contains no leaked
dirs.
2026-05-05 08:52:15 +02:00
ZdenekSrotyr
fd3c76d21b fix(store): security + correctness blockers found in PR review (F1, F2, F4, F5)
Three independent reviews of PR #180 surfaced four real defects in the new
Store / my-ai-stack surface. CHANGELOG entries detail each; one-liners:

- F1 video_url XSS: any authenticated user could upload a Store entity
  with `video_url=javascript:...` and pop XSS in any viewer's session via
  the `<a href=...>` "Watch video" link in store_detail.html. Jinja2
  autoescape doesn't block URI schemes inside attribute values. Fixed by
  scheme-validating to http(s) only on create + update; 400 invalid_video_url.

- F2 ZIP decompression bomb: _safe_zip_extract checked path-traversal but
  not declared file_size totals — a 50 MB compressed upload at 1:1000
  ratio decompresses to 50 GB and DOS the host disk. Fixed by summing
  zinfo.file_size across infolist() and refusing > 200 MB before
  extractall touches disk. 413 zip_too_large_uncompressed.

- F4 admin authz parity: PUT /api/store/entities/{id} was owner-only while
  DELETE allowed owner OR admin; the store-detail page hid Edit/Delete
  buttons from admin even though DELETE was permitted. Fixed by allowing
  admin on PUT and passing is_admin to the template; gate is now
  is_owner OR is_admin everywhere.

- F5 cross-owner suffix collision: sanitize_username is many-to-one
  (alice.smith / alice_smith both → alice-smith). Two such users uploading
  entities with the same display name produced identical
  `<name>-by-<username>` suffixes, silently colliding in the served
  agnes-store-bundle on-disk paths AND the manifest catalog (Claude Code
  dedupes by plugin.json `name`). Fixed by enforcing global uniqueness on
  the suffixed value at create_entity; 409 conflict_global_suffix.

F3 (ZIP symlink members) was investigated and confirmed to be a
false-positive — Python's stdlib ZipFile.extractall does not honor
symlink mode bits, so no exploit exists.

9 new regression tests in tests/test_store_api.py::TestStoreSecurityFixes
covering all four. Test run locally: 60/60 store-related tests pass.
2026-05-05 08:18:02 +02:00
Minas Arustamyan
d5a7c9ad79 feat(store): /store + /my-ai-stack — community marketplace + per-user composition
Adds a community-driven Store where any authenticated user uploads
skills/agents/plugins as ZIPs, plus /my-ai-stack as the per-user
composition view. The served Claude Code marketplace is now:

    (admin_granted ∖ opt_outs) ∪ store_installs

Skill + agent installs are merged into a single `agnes-store-bundle`
plugin in the served marketplace; type=plugin uploads stay standalone.
Names are suffixed with `-by-<owner-username>` at upload time so two
owners can use the same display name without colliding in Claude Code's
flat skill/agent namespace.

Schema v23 → v24 adds three tables:
  - store_entities       — community-uploaded skills/agents/plugins
  - user_store_installs  — what each user has chosen to install
  - user_plugin_optouts  — opt-out overlay on top of admin grants

Admin grant-delete drops every user's opt-out for that plugin so
re-grant resets cleanly to enabled (no sticky personal preference).

UI:
  - /store      — e-commerce-style listing with type/category/owner
                  filters, search, pagination, owner-aware [Install]
                  buttons, clickable cards
  - /store/new  — 2-step upload wizard with drag & drop, preview
                  validation (POST /api/store/entities/preview), docs
                  multi-upload, photo + video URL
  - /store/{id} — detail page with hero, file list, docs, owner
                  actions (Edit/Delete) for the uploader
  - /my-ai-stack — Granted plugins (toggle opt-out) + From the Store
                  (uninstall) sections
  - Admin nav: Marketplaces moved into Admin dropdown, renamed to
                "Curated Marketplaces"

Validation hardening: type-mismatch guards reject skill ZIP uploaded as
agent (or vice versa), and plugin ZIPs masquerading as skills/agents.
Human-readable error messages mapped client-side from machine codes.

Cross-source naming: Store entity-id-prefixed dirs (`plugins/store-<id>/`)
plus the bundle (`plugins/store-bundle/`) avoid collisions with admin
marketplaces (whose `store` slug is reserved by `is_valid_slug`).

Bundle composition is content-hashed at serve time — install/uninstall
or owner re-upload bumps the bundle's plugin.json `version`, so Claude
Code's auto-update toggle picks up changes.

Tests: 50+ new tests across naming, repositories, filter (admin ∪ store
∪ bundle), API (upload/install/uninstall/delete/preview/docs), end-to-end
marketplace.zip with bundle merging.
2026-05-05 02:53:49 +02:00