Reduce the paws feature surface - #14
Merged
Merged
Conversation
added 28 commits
August 23, 2026 22:15
paws is this platform's sdk line; the previous pin (584756d3) was a detached older chat tip. The move is a fast-forward. .gitmodules is deliberately untouched -- it still reads branch = chat on main, and that line is owned by the default-model-onboarding branch. Only the pointer moves here.
PLAN.md (design source, 6 invariants lifted verbatim, 13 items, plan-audit verdicts), BASE.md, DESIGN_FIDELITY.md, TESTS.md, DECISIONS.md.
ITEM-1..9 of the paws feature-surface reduction. UI: one shared hide list (src-app/ui/src/modules/pawsHiddenModules.ts) read by four central places -- the per-module shouldLoad literals, the desktop CORE_MODULE_BLOCKLIST, the chat-extension discovery glob and the project-extension registry. The predicate alone is not sufficient: the chat and project extension globs are owned by the SURVIVING modules and register a hidden module's composer pills, panel renderers, rail steps and the project References entry regardless of whether the module loaded. On desktop the predicate does nothing at all -- that loader eager-globs every module and never evaluates shouldLoad -- so the blocklist is the lever that does the work there. A hidden module's route now 404s instead of rendering the in-place 403, which told the user they lacked permission for a feature this instance does not have. Server: web_search / lit_search / voice / js_tool default OFF. The defaults now live in one accessor per switch on Config. Flipping the default_*_enabled() functions alone would NOT have disabled anything in the common case: every read site spelled the fallback out as .unwrap_or(true), so an absent key stayed enabled and only a present-but-empty block would have changed. Closes a real hole in web_search: its chat-extension factory discarded the config and should_attach consulted only DB rows, so with the switch off a stale enabled row still offered the tools to the model. lit_search already guarded this; web_search was the odd one out. Desktop no longer force-enables web_search over the operator's config. bio_mcp is deliberately untouched -- it is not one of the 13 items.
…grants ITEM-10. Six migrations, all above the in-flight PR #10 prefix (202607210100): 202607210200 file_rag semantic_enabled -> false (column default + the seeded singleton; changing only the default would leave every already-migrated deployment on) 202607210210 citations 202607210220 knowledge_base 202607210230 scheduler 202607210240 workflow 202607210250 hub Each revoke sits beside the grant it undoes, per the per-module migration convention. The scheduler one revokes scheduler::use ONLY. Its grant migration bundles notifications::read into the same ARRAY[...], and notifications is a SURVIVING module -- reversing that grant wholesale would have taken the notification list away from every non-admin user. Verified against the migrated build database: the citations/knowledge_base/scheduler/workflows/hub grants are gone and notifications::read remains. Revokes only ever REMOVE a grant, so nothing here weakens a check (INV-6).
…eatures' e2e ITEM-11: removes the /settings/assistant-templates route, its settingsAdminPages entry, its page and its store, and the now-unreachable isTemplate branches in the shared AssistantFormDrawer (the removed page was the only thing that ever set the flag). Deliberately NOT removed: the is_template column, the seeded Default Assistant row, clone-on-signup, the REST routes and the hub install-as-template path. Taking those out would leave every new user with zero assistants -- the drawer is shared with the surviving user-assistants page, so this change is scoped to the template SURFACE only. ITEM-12: deletes the e2e suites for the hidden features (citations, hub, workflows, 14-scheduler, 14-knowledge-base, file-rag, 14-voice) and the assistant-template admin specs. Also removes three specs that reach into those suites and would otherwise fail collection: two 14-split-chat specs covering voice and knowledge-base in a pane, and an llm spec importing the hub navigation helper. template-clone-on-signup.spec.ts is KEPT -- it drives the API, not the removed page, and the behaviour it covers is unchanged.
TEST-1 [acceptance, INV-4] runs the BUILD's own extractModule() over the real module.tsx files and EVALUATES the predicate it lifts, rather than asserting on source text -- so it proves the function the loader will call returns false, and it breaks if the lifting rules change. Verified RED by reverting workflow's predicate: 'workflow/module.tsx: shouldLoad must return false even for a fully-permissioned admin on the module's own route'. Carries a control asserting chat/projects still evaluate true, so a broken extractor cannot make it pass vacuously. TEST-5 [acceptance, INV-5] injects an EMPTY hidden set and shows all four consumers re-admit the module -- reversal is a data change, not a code change. It also pins that web-search and literature extensions survive (both are disable-only rows in the design's item table) and that an unparseable glob key fails OPEN. TEST-7 covers the desktop blocklist, the lever that does the work on the target platform. TEST-8 drives the real ProjectExtensionRegistry to show the citations References entry is gone, with a control proving register() has not simply been made to drop everything.
TEST-4 [acceptance, INV-3] proves a disabled capability registers no built-in MCP server, with a two-server positive control. That control earned its keep immediately: the first draft asserted against /api/mcp/system-servers, and the control failed because built-in rows are deliberately hidden from that endpoint -- so every 'absent' assertion had been passing vacuously. It now reads mcp_servers directly and also asserts the table is non-empty, so 'absent' cannot mean 'nothing registered at all'. TEST-6 [acceptance, INV-6] pins that permission checks still behave: a permitted user still succeeds and an unauthenticated request is still refused. TEST-12 covers the migration's observable effects -- semantic_enabled false with fts_enabled untouched, the DEC-4 grants revoked, and notifications::read SURVIVING (the trap in the scheduler grant migration). TEST-13 proves clone-on-signup still gives a new user an assistant, which is what stops 'remove assistant templates' from silently shipping empty accounts. The harness now writes the four kill switches explicitly, defaulting ON in tests: those suites cover features that still exist and are opt-in, not deleted, and this keeps ~150 existing call sites working unchanged. The paws DEFAULT is asserted where it belongs, in the config unit tests against the real packaged config.
10 specs in tests/e2e/17-paws-surface, all passing.
TEST-2 [acceptance, INV-1]: no hidden module chunk is ever downloaded (the
strongest form of 'absent, not suppressed' -- a CSS-hidden entry still downloads
its module), no nav or settings entry, no hidden route rendering the router 403,
and no hidden feature registering a chat extension.
TEST-3 [acceptance, INV-2]: every assertion POSITIVE -- the composer renders and
accepts input, settings navigates, projects opens, the shell boots with no
uncaught error and no '[loader] failed to load module'. An absence-only test
passes just as happily on an app that failed to boot; this one cannot.
TEST-14 [negative-perm, positive-control]: no References/Knowledge bases on the
project page, with the page itself asserted to LOAD for that same user.
Two hollow drafts were caught and discarded on the way here, both by disabling
the discovery filter and re-running:
1. assertions on guessed button labels ('Knowledge bases', 'Schedule') --
matched nothing, passed with the filter off;
2. assertions on the components' REAL testids -- still passed, because those
affordances need conditions the spec does not create (an open plus-menu, an
existing conversation, a live voice capability).
The surviving version asserts on chat-extension REGISTRATION, which is what the
filter actually controls, and it fails under that same probe. Its positive
controls pin that surviving extensions still register -- including literature,
a disable-only item whose UI must NOT be hidden.
…es page Deleting AssistantsSettings.tsx moves the gallery coverage + state-matrix denominators, and the hand-maintained Records are compile-checked against the generated unions -- a stale key is a tsc error, which is how both surfaced. Regenerated both and pruned the four entries for the removed page. No other surface moved: the hidden modules' files are untouched, so only the one genuinely deleted page changes the counts.
…e grant revokes
Blind audit (design-conformance + correctness), both angles corroborating.
MAKE THE KILL SWITCHES REAL. web_search / lit_search / js_tool skipped only the
mcp_servers upsert; their JSON-RPC endpoints stayed mounted, gated on *::use
permissions the Users group HOLDS, with runtime settings rows defaulting enabled.
So 'disabled' meant 'unadvertised': an ordinary user could still drive live web
searches, keyless scholarly queries and arbitrary QuickJS with the switch off --
falsifying both the design's definition of disable and the comments this branch
had just added. register_routes now guards, per CLAUDE.md 16. The settings/admin
REST is split out and stays mounted, because web_search and lit_search are
disable-ONLY rows whose admin pages the design keeps. voice/js_tool/web_search/
lit_search module structs now default enabled=false so route registration fails
CLOSED if init never ran.
DROP THE DEC-4 GRANT REVOKES. Revoking citations::use would have broken chat for
every non-admin: the citations backend chat extension attaches its tools and a
system nudge to EVERY tool-capable chat with no permission check, so the tools
would be advertised and then 403. That is an INV-2 break introduced by a
'cleanup'. Fixing it properly needs a server-side kill switch for a UI-only item,
which the design puts out of scope -- so the design's recorded limitation (hiding
is not a security control) stands. Only the semantic-search migration remains.
Also: a surviving 'Install from Hub' list in onboarding (no lever reached it --
it lives in the surviving onboarding module and was gated on a hub permission
admins hold via '*'); the Playwright harness now writes the four switches like
the Rust harness does, so e2e suites for those features keep their subject;
restored api-key cross-device sync coverage that was collateral in a deleted
two-entity spec; three more specs that drive hidden features; bio_mcp moved onto
an accessor.
TEST QUALITY. The e2e specs were largely hollow and the audit said so: label
assertions that matched nothing ('Knowledge Bases' vs the real 'Knowledge'),
project assertions on the LIST page where knowledge kinds never render, route
assertions satisfied by the hidden page rendering in full, and a permission-less
user for whom admin-gated surfaces are absent anyway. Now: an ADMIN session,
real labels, a created project opened by id with 'Knowledge files' asserted
PRESENT as the control, feature-content absence alongside the 403 absence, hub
sub-module chunk names, and endpoint-reachability probes with an enabled-server
control.
Unrelated latent bug exposed: app_builder's test sorted its expectation by order
alone while create_modules sorts by (order, name), so the expectation was
linker-dependent and failed once these edits perturbed link order.
The two project e2e assertions failed because the fixture created the project with the ADMIN token while browsing as a fresh user, and projects are owner-scoped — so the detail page was another user's project. Same identity now creates and views. That failure is useful evidence: the strengthened assertion demonstrably goes red when wrong, which the version it replaced could not. LEDGER.jsonl (26 rows), FIX_ROUND-1.md, DEC-4 recorded as REVERSED with the original kept in withdrawn form, TESTS.md amended for TEST-6 and TEST-14.
… hollow spec MERGE-GATE LANDMINE. Config-gating register_routes made the emitted OpenAPI config-dependent: the generators call init() with the real config, so with paws' defaults a regen DELETES /api/voice/* and /api/js-tool/settings from both workspaces' openapi.json and types.ts. Invisible locally -- types_ts_parity regenerates from the COMMITTED spec and stays green -- and would have surfaced only at the merge-gate. openapi-gen.yaml now forces every deploy switch ON: the spec describes the API surface the codebase can serve, not one deployment's. bio_mcp had the identical unguarded-route shape I had just fixed elsewhere, in a file I had just edited. Boot enabled, disable, and the stale mcp_servers row plus a still-mounted route gated on a Users-held permission means the sidecar spawns and query terms egress anyway. Guarded, fail-closed. My round-1 onboarding fix was incomplete in its own file: the section was gated but the sentence above it still told the admin to install from the Hub, and the hub fetch stayed ungated inside a Promise.all where a rejection would show an error alert on the whole step. Same hub copy in MemorySetupStep. All fixed. The spec covering that fix was HOLLOW and would have passed on main -- loginAsAdmin completes onboarding, so /onboarding opens on Welcome and the MCP step never mounts. It now drives to the step and asserts it is visible first. Verified RED under mutation, and the probe confirmed the auditor's prediction exactly: the PROSE assertion fired while the heading assertion (behind hubServers.length > 0) did not. Also: deleted run-js-inner-approval.spec.ts, stranded by my own harness change (server-enabling js_tool does not un-hide its UI, and its chat extension is the sole registrant of every testid that spec waits on); the chunk test now runs as ADMIN and visits /hub, because 10 of its 14 assertions could not fail for a permission-less user; restored /settings/scheduler and the two admin labels; pinned all ten withdrawn grants instead of two; corrected three stale docstrings that still claimed revokes ship.
…surviving surfaces
REGRESSION I INTRODUCED, now reverted. The round-2 bio_mcp route guard broke five
existing integration tests: the harness defaults bio_mcp_enabled:false, so every
test server had /api/bio/mcp unmounted and tests asserting 401/403/503/405 got a
bare 404. Oracle-confirmed by running them. bio_mcp is not one of the 13 items,
so guarding it was scope creep; unmounting redefines its disabled contract, and
one of those tests deliberately uses the disabled path to reach graceful-503.
That call belongs to whoever owns bio_mcp. The reasoning stays in the code so
nobody re-adds it blind. All 14 bio_mcp tests pass again.
FIVE MORE SURVIVING SURFACES, in modules no lever reaches. The worst two: the
skills empty state ('browse the Hub to install one' — the DEFAULT state of
/settings/skills on a fresh instance), and the llm-provider sidebar widget, whose
read of HubModels.$ initialises a lazy store and fires three live /api/hub/*
requests. That is the exact render-gated-but-fetch-ungated hazard I wrote a
paragraph about one round earlier and did not look for elsewhere. Also a
dead-but-focusable citation chip in chat CORE (any assistant message containing
'[1]' produced an aria-labelled affordance for the hidden knowledge base), plus
two pieces of admin copy.
AN INV-5 VIOLATION I INTRODUCED. Round 2 rewrote MemorySetupStep's Hub sentence
IN PLACE instead of gating it, so deleting 'hub' from the list would restore the
hub everywhere except there — a code edit needing manual revert, which is what
INV-5 forbids. Now gated.
TWO HOLLOW ASSERTIONS closed: the grant pins were substring-shadowed
(hub::assistants::read is satisfied by read_version, granted by the same
migration), and the onboarding prose assertion could resolve during the step's
loading spinner.
ESCALATED, not absorbed: control_mcp defaults ON and builds its catalog from the
live router, so the MODEL can enumerate and invoke hidden features. The design
frames the residual as 'a user who knows the URL'; this is sharper than that.
Closing it needs either out-of-scope kill switches or the grant revokes withdrawn
in round 1. Goes to the owner in the PR body.
Loop terminates on T1: profile 26 -> 25 -> 16, promoted fraction 0.25.
The phase-7 gate caught a number I got wrong: I wrote that T1 fired for round 3 with m=2, but ZERO of that round's findings were corroborated within the round. (DownloadIndicatorWidget was flagged by design-conformance in round 2 and again in round 3 -- cross-round recurrence, not two angles agreeing inside one round.) With m=0 the sample is below the estimator's floor, so T1 is not estimable and the decay rule decides alone. Corrected in FIX_ROUND-3.md rather than left standing. Closed both open ledger rows: the built-in-row poll now waits for the SPECIFIC names the caller asserts on rather than for any row at all (memory/files landing first could satisfy it while the three targets were still in flight), and the JsToolApprovalContent comment now says the run-js spec was DELETED and that the CSS-inerting channel is uncovered, instead of citing it as live coverage.
…onverges No new product defect this round. Both material findings were regressions in my own round-3 work, and both were oracle-confirmed by running: The citation gate made citationTokenize a no-op while its unit test still asserted the tokenizing behaviour. Both angles reproduced it independently (26 failures at HEAD vs 25 with the one line removed). Fixed by giving the function the injectable hidden-set parameter its own helper already advertised: the regex-rule cases inject an empty set and keep testing tokenization, and a new case asserts the paws behaviour against the real list -- that new case is what goes red if the gate is removed. The new JSX ternary added the first genuinely new RequiredState key on this branch, so check:state-matrix was red. Regenerated; state-matrix and gallery-coverage both pass. Also: composed both keys where one gate suppressed copy naming TWO features (restoring one would have re-advertised the other); stopped hand-maintaining the consumer count in pawsHiddenModules -- it went stale twice in consecutive rounds, including in the commit that added 'keep this current', so the header now names classes and points at the grep. Judgement call: the design angle called two AgentSettingsSection descriptions inconsistent with three siblings I had gated. Split rather than treated alike -- gated the clause naming workflow agent steps (unreachable: no workflow can be authored), left the scheduler horizon backstop alone because that ceiling still applies server-side whether or not the scheduler has a UI. Profile 26 -> 25 -> 16 -> 11 on progressively smaller diffs, promoted fraction 0.32 -> 0.25 -> 0.09, open set empty. Loop converges.
…ithmetic I claimed convergence with a promoted fraction of ~0.09, counting one corroborated finding. The gate computes it from the ledger: n1=8 n2=7 overlap=4 -> Chapman 13.4 vs 11 observed -> ~1.09 promotable, just above the <1 threshold. Four rows carry corroborated_by:2, not one; I undercounted the overlap and my arithmetic came out flatter than the truth. Round 5 follows.
…two escalations The correctness angle found no defect introduced by the range and mutation-tested its way there. The design angle's surviving-surface sweep came back CLEAN for the first time across all 41 UI modules, the desktop workspace and the api-client. A judgement of mine was overturned, correctly. I gated the 'workflow agent steps' clause and deliberately left 'the scheduler's horizon backstop', reasoning the latter still applies server-side. That is equally true of workflow -- also a hide-only row, API and agent step kind still live. The distinction did not exist. Both gated; the comment records the correction, not the original rationalisation. A vacuous pair I inherited and then vouched for: two 'code span' assertions never exercised CODE_SEGMENT_RE, because in both the bracket is preceded by a word char so the lookbehind alone blocks them -- deleting the whole code-splitting stage left the suite green. My round-4 doc comment had presented them as coverage. Added a bare [n] inside a code span and verified by mutation that the suite now goes RED where it previously did not. Also: the disabled-side built-in wait now polls until two consecutive reads agree rather than settling on the first row to land, and the citation-chip consumer is gated (marked wontfix in round 4, raised again in round 5 -- closing it beat defending it a third time). ESCALATED: the citations built-in auto-attaches to every tool-capable chat, so with citations hidden the model still calls six citation tools and writes a bibliography the user cannot view. That is past the design's stated limitation. Both fixes are out of scope (a server-side gate for a UI-only item, or the grant revoke withdrawn in round 1), so it goes to the owner with control_mcp. Profile 26 -> 25 -> 16 -> 11 -> 7, monotonic. Loop converges.
… ends at cap Both angles reported no real defect. The three findings were polish on my own round-5 fixes that did not deliver what their comments claimed. Corroborated by both angles: the round-5 quiesce heuristic NARROWED the false-pass window rather than closing it -- settling needed only two reads 100ms apart agreeing on names.len(), so an offender landing in a quiet gap still slipped through, and a same-size set swap was invisible to a length comparison, while my comment implied it was shut. Closed properly: the ENABLED server now runs first, the surviving built-in set is DERIVED from that real boot, and the disabled server waits for every one of those names. No window, and no hardcoded list to go stale in the direction that weakens the test. Also: extracted citationChipNumber as a pure function so the renderer gate is unit-tested in both directions (it was the only list consumer with no injectable set and no test), and rewrote the header's consumer enumeration as CLASSES -- that list went stale in three consecutive rounds, twice in the very commit telling the reader to keep it current, which makes it the wrong mechanism rather than a discipline problem. Profile 26 -> 25 -> 16 -> 11 -> 7 -> 3, monotonic, open set 0 of 82. Stopping at the cap rather than chasing a zero-finding round: rounds 5 and 6 produced no product defect between them, and the skill is explicit that 'repeat until 0' is unsound.
origin/main advanced to 1e6d934 and now carries 202607210200_llm_repository_default_model_mirror.sql, colliding with this branch's prefix. Prefixes must be unique within the server sequence because FK ordering depends on them. Renaming is safe here: this migration has never shipped (one commit, never edited, not on main), so the new migration-immutability guard does not apply to it. BASE.md's recorded server max is now stale -- it was measured before #10 merged -- and is updated alongside.
Resolves DEC-13: PR #10 is merged, so the sdk paws-branch testids now have their app-side code on main and the registry can be regenerated honestly. The regen drops ONLY this branch's seven template-assistants-* ids and preserves #10's onboarding ids -- the union I said it would produce rather than the overwrite it would have been before. sdk commit 8693247 pushed to ziee-ai/sdk paws. Conflicts were confined to three GENERATED gallery artifacts (STATE_MATRIX.md, galleryCoverage.generated.ts, stateMatrix.generated.ts) and were resolved by regenerating from the merged tree, not by hand-merging. .gitmodules came in from main pinning sdk to paws; my diff against main on that file is empty, so the line PR #10 owns is untouched. Submodules synced. My migration was renumbered to 202607210300 in the preceding commit -- main now carries 202607210200 for llm_repository, which would have collided.
…text compaction Records worktree/branch/HEAD, phase status (1-6 green, 7 at the cap needing the owner's nod, 8-9 pending), what is already verified by running so it is not re-done, the exact remaining steps, the two escalations that must reach the PR body, the known environment floor with its main-clone attribution, the hard constraints, and the gotchas already paid for (migration prefix, generated-artifact merge conflicts, re-verify-after-last-mutation). Also notes that lifecycle-check now needs --dir: merging main brought in a second feature directory.
… menu leaks
The design's item table listed web search (1) and literature (2) as `disable`,
which was read as "flip the server switch, leave the UI". That is not enough:
with the capability off, each module still registers two settings pages, so the
menu kept "Web Search", "Web Search Keys", "Literature Search" and "Literature
Keys" — two of them USER-facing — inviting a user to configure something that
cannot run. Both modules are now hidden as well.
Hiding the whole module is the only lever available (removing a module's slot
registrations one at a time is out of bounds), and it also drops each module's
chat-extension, which is right: the server never mounts either MCP router.
Also closes the last menu leak found by auditing every settings/sidebar slot
against the 13 items: the knowledge_base built-in was missing from the System
MCP page's exclusion set, so a hidden feature's row was still listed. Added to
both the rows query and the count query — they must stay in step or the
pagination total counts rows the page will not show.
Test fallout, all of it pre-existing and found by the same sweep: six specs
asserted hidden features' behaviour and would have failed. Suites for hidden
features are deleted (literature/**, settings+sync web-search, run-js, workflow
workspace-export, project bibliography). Four specs that test a SURVIVING
feature but used a hidden one as their vehicle are retargeted rather than
dropped, so their coverage is kept:
- chat/right-panel-resize literature panel -> file panel
- chat/activity-rail-seeded KB rail language -> covered by the file/sandbox
labels already asserted
- chat/activity-rail-detail KB card body -> the sibling FILE case proves
the same delegation contract
- settings/permission-visibility web-search -> users (the old control was
about to pass vacuously)
The chat gallery's two literature right-panel surfaces go the same way: the
single-tab one is dropped, and the multi-tab one takes a second FILE so the tab
strip that A8/I5 need stays real instead of silently degrading to one tab.
…ique and its resolution
TEST-11 had no test at all. ITEM-9 was implemented by DELETING the desktop force-on override, so the change was visible in the diff but asserted nowhere, and the first draft of TEST_RESULTS recorded it NOT VERIFIED on the grounds that a test would be disproportionate apparatus. That was wrong, and the lifecycle gate was right to refuse it. What this code must NOT do is as load-bearing as what it does: the block used to read `config.web_search.get_or_insert_with(Default::default).enabled = true`, which writes unconditionally — clobbering an operator's explicit `false` as happily as filling a missing default, making the paws kill switch a no-op on the one platform the reduction targets. The core/config.rs tests prove the SERVER default is off and would have stayed green throughout. Extracts `apply_desktop_feature_defaults(&mut Config)` — the seam that was missing — and adds two tests that parse the REAL packaged config rather than a stub, mirroring core::config::paws_kill_switch_tests. Mutation-probed rather than merely green: re-inserting the original force-on line fails BOTH tests with their own messages; removing it passes both. Each carries a positive control (code_sandbox + bio_mcp still default ON) so neither can be satisfied by the helper degenerating into a no-op. Re-exports ziee::WebSearchConfig, since `core` is private and the desktop crate could not otherwise name the type. Also adds TEST_RESULTS.md (every ID with the run behind it, the 69-failure MCP classification and its control run) and HUMAN_FEEDBACK.md (FB-1..FB-7).
…owner scope call, not a defect this branch can close)
…face # Conflicts: # sdk
main has been completely free of .lifecycle files since PR #12 stripped the set PR #10 left behind, and must stay that way. These 16 are process artifacts of the feature-lifecycle run, not product. Nothing is lost: they remain in this branch's history and are recoverable with `git show <parent>:.lifecycle/paws-feature-surface/<file>`. The two that matter to a reviewer — TEST_RESULTS.md and HUMAN_FEEDBACK.md — are summarised in the PR body, including the two escalations the owner still has to rule on. Note for anyone resuming after this commit: `lifecycle-check` can no longer read its artifacts from the working tree; check out the parent commit to run it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduces the paws feature surface to the 13 items in
docs/design/paws-feature-surface.md— disable (web search, literature, semantic search, voice, programmatic tools), hide (workflow, scheduler, citations, knowledge base, document RAG, hub, voice, js-tool), remove (assistant templates; the project "References" entry).Both are cases where a UI-only lever leaves a server-side capability reachable, which is the recurring theme of this entire change. Neither is fixed here; each needs a decision that belongs to you, not to this branch. They are sharper than the design's stated limitation ("a user who knows the URL") because the leak reaches the model, not just a curious user.
1. The citations built-in auto-attaches to every tool-capable chat.
mcp/chat_extension/mcp.rsmarks it "always available, no admin enable". The model receives six citation tools regardless of any switch, calls them unprompted, and writes a bibliography into a library the user has no surface to view — the citations UI is hidden by this branch.2.
control_mcpdefaults ON and builds its catalog from the live router.The model can
list_capabilities→invoke_capabilityagainst workflow, scheduler, knowledge-base, citations, file-rag and hub. Hiding the UI does not remove the operations from the catalog.Closing either needs one of two things the design deliberately defers: a server-side kill switch for a feature scoped as UI-only (items 6–11 are explicitly "hidden, not disabled"), or the DEC-4 permission revokes, which were withdrawn in audit round 1 because they broke chat for every non-admin — the citations backend extension advertises its tools with no permission check, so revoking
citations::usewould have advertised tools that then 403.A third, smaller one for your ruling: the User Groups permission picker still enumerates hidden features' tokens (
workflow::*,citations::use,knowledge_base::use,hub::*, …). Admin-only, and an admin can type any token into the Advanced JSON editor regardless, so filtering it is cosmetic rather than containment. Unchanged pending your call.How hiding works
shouldLoad: () => falseis the lever, but it is provably not sufficient on its own — several surfaces reach a hidden feature without consulting it. One shared list,src-app/ui/src/modules/pawsHiddenModules.ts, drives all of them:<module>/module.tsxloader.desktop.tsshouldLoad; the blocklist is the only lever that does anything there — and desktop is the targetchat/extensions/index.tsprojects/core/extensions/registry.tsx/api/hub/*callsRestoring a hidden feature is deleting its entry from that list plus restoring the module's original predicate, which each
module.tsxcarries in a comment. Two honest caveats are documented in the file's header: the four disabled capabilities also need their server config key, and semantic search is a DB default, not a predicate.Owner correction during implementation
The design table listed web search and literature as
disable. That was read as "flip the server switch, leave the UI" — which left four settings pages in the menu ("Web Search", "Web Search Keys", "Literature Search", "Literature Keys"), two of them user-facing, inviting configuration of something that cannot run. The owner found them by using the app.Rows 1–2 are amended to
hide + disablein the design doc, with the correction recorded rather than silently applied. Worth noting: no artifact on this branch would have caught this — every test agreed with the wrong reading, including one written specifically to assert those modules survive.Sweeping every settings/sidebar slot against the 13 items then found one more leak nobody had reported: the
knowledge_basebuilt-in was missing from the System MCP page's exclusion set. Fixed in both the rows query and the count query — they must stay in step or the pagination total counts rows the page will not display.Test fallout, and what was not deleted
Six specs asserted hidden features' behaviour and would have failed. Suites whose subject is a hidden feature are deleted (the owner's standing decision). Four specs that test a surviving feature through a hidden vehicle were retargeted, not deleted, so their coverage survives:
chat/right-panel-resizechat/activity-rail-seededchat/activity-rail-detailsettings/permission-visibilityweb-searchentryusersentryThe last is the one worth flagging: with web-search hidden, that assertion would have gone on passing while proving nothing about permission filtering — the only thing the spec exists to prove.
Verification
All on the merged tree, exit codes captured with
set -o pipefail:npm run check (ui): PASS ·npm run check (desktop/ui): PASS (114/114)gate:ui (ui): PASS — 200/200 surfaces, validity 590/590 cells, 0 transport artifactsgate:ui (desktop/ui): PASS — 35/35, validity 222/222. Itsvisualstage reads PASS because it is not configured, not because pixel baselines matched.17-paws-surface(--workers=1): 11/11, including the four new route assertions proving the reported menu entries now 404paws_surfaceintegration 5/5 ·mcp::run_in_sandbox_test+ system-server listing 12/12 ·cargo check -p zieecleanTEST-11 was missing and is now written. ITEM-9 had been implemented by deleting the desktop force-on override, so it was visible in the diff but asserted nowhere. What the code must not do was the load-bearing part — the block used to write
web_search.enabled = trueunconditionally, clobbering an operator's explicitfalseand making the kill switch a no-op on the target platform.apply_desktop_feature_defaults()is extracted as the missing seam; the tests are mutation-probed (re-inserting the original line fails both; removing it passes both) and carry positive controls so they cannot pass as a no-op.The full backend
mcp::suite reports 69 failures. None is attributable to this branch, and this was classified rather than asserted: 62 areNo AI provider API keys found(tests/.env.testdoes not exist on the build host), 5 are missingsquashfuse, and the remaining 2 were settled by a control run with the change reverted — one failed identically without it, the other is a different case per run in an npx-dependent file.Merge notes
origin/mainthrough PRs fix(realtime): deliver chat tokens and download progress to the UI #12 and fix(local-runtime): stop silently shipping the CPU engine to GPU hosts #13. The sdk pointer needed more care than taking main's: main pinsbb0a60e52, which does not contain this branch's testid regen, and taking it leftcheck:testid-registryfailing (verified by running it). The sdkpawstipfa9a577is exactly main's pin plus that regen — the tree diff is only the 7template-assistants-*ids — so it is pinned instead, with no force-push over anyone's branch..gitmodulesis untouched.202607210300is still the server-sequence max and there are no duplicate prefixes..lifecycle/artifacts are stripped in the tip commit so main stays free of them; they remain in this branch's history.Known limitation (unchanged from the design)
Items 6–11 are UI-only: workflow, scheduler, citations, knowledge base, document RAG and hub keep answering on REST. That is the design's recorded scope, and it is the same root as the two escalations above.