From ec8ec22985a752325707038714bb4253ad02c6d7 Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Wed, 29 Jul 2026 08:19:18 -0600 Subject: [PATCH 1/2] fix(marketplace): require PUBLIC visibility to publish an agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The store gates browse on `listing.state` alone, while pinning gates on `visibility`. An agent could therefore be published while still SHARED or PRIVATE: a tile everyone saw and only the author could open. Two users hit this during the dev demo — `POST /agents/{id}/pin` returned a bare "Agent not found" for a tile the store had just offered them. The marketplace is public-only. Sharing an agent with named coworkers is a separate mechanism, and a listing carries no audience of its own, so a published non-PUBLIC agent is incoherent state rather than a team listing. - Block submission unless the agent is PUBLIC, surfaced by `preflight_listing` (the dialog already renders a block reason and hides the form) and enforced by `submit_listing`. A refusal, not a silent widening: publication must not be a side door that changes who can reach an agent. - Re-check at approval. `visibility` can be narrowed between submitting and being reviewed, so the submit-time gate says nothing about approval time. - Answer a pin denial on an already-published agent with a legible 403 instead of collapsing to 404. The store advertised that id, so its existence is not a secret; the collapse still applies to everything else, and the extra lookup is best-effort so it can never escalate a 404 into a 500. `_reachability` stays: an agent published as PUBLIC and narrowed afterwards is the case no gate can catch. Its comment — and the SPA's — claimed publishing a SHARED agent to a team was legitimate, which is what made this look intended. Co-Authored-By: Claude Opus 5 --- .../services/listing_service.py | 93 ++++++++++++++++--- .../agent_designer/services/pin_service.py | 38 +++++++- .../tests/routes/test_admin_agent_listings.py | 33 ++++++- backend/tests/routes/test_agent_listing.py | 74 ++++++++++++++- backend/tests/routes/test_agent_pins.py | 67 +++++++++++++ .../src/app/agents/models/reachability.ts | 10 +- 6 files changed, 295 insertions(+), 20 deletions(-) diff --git a/backend/src/apis/app_api/agent_designer/services/listing_service.py b/backend/src/apis/app_api/agent_designer/services/listing_service.py index 03e818bd6..1173cfed2 100644 --- a/backend/src/apis/app_api/agent_designer/services/listing_service.py +++ b/backend/src/apis/app_api/agent_designer/services/listing_service.py @@ -191,6 +191,47 @@ async def _memory_space_block(assistant: Assistant, user: User) -> None: raise ListingError(reason, status_code=400) +def _visibility_block_reason(assistant: Assistant) -> Optional[str]: + """The blocking message for an Agent that is not ``PUBLIC``, or ``None`` if clear. + + **The marketplace is public-only.** Sharing an Agent with named coworkers is a separate + mechanism with its own control on the agent tile, and a listing carries no audience of + its own — ``AgentListing`` has no scope field, and the store is one global shelf. So a + published SHARED or PRIVATE Agent is not a "team listing"; it is a tile shown to + everyone that nobody but the author can open, and every pin against it 404s. + + Split from the raising path for the same reason as ``_memory_space_block_reason``: + ``preflight_listing`` shows it, ``submit_listing`` enforces it, and one function + decides so the dialog and the transition cannot drift apart. + """ + if assistant.visibility == "PUBLIC": + return None + if assistant.visibility == "SHARED": + return ( + "This agent can't be published while it's shared with specific people. The " + "store is public — everyone would see it, but only the people it's shared " + "with could open it. Set Visibility to Public to publish it, or keep sharing " + "it directly instead." + ) + return ( + "This agent can't be published while it's private. The store is public, so people " + "would see it and get an error when they opened it. Set Visibility to Public and " + "submit again." + ) + + +def _visibility_block(assistant: Assistant) -> None: + """Block submission unless the Agent is ``PUBLIC``. + + Deliberately a refusal rather than a silent widening: publication must not be a side + door that changes who can reach an Agent. The author says "make this public" in the one + place that means it, and the store trusts ``visibility`` rather than overwriting it. + """ + reason = _visibility_block_reason(assistant) + if reason: + raise ListingError(reason, status_code=400) + + async def _exposed_skills(assistant: Assistant) -> List[SkillExposure]: """Skills the author wrote that publication makes readable (D7.1). @@ -255,17 +296,22 @@ async def preflight_listing( Owner-only, like every other author path: the skill exposure is a statement about what the *owner's* publication would reveal, and it is not an editor's to see. - Also returns reachability, which is *not* a D7 check and deliberately not a block: - the author is told their store tile will 404 for people who cannot already reach the - Agent, and left to decide. Returned even alongside a ``block_reason`` — the two are - independent facts and suppressing one behind the other just hides work from the - author's next attempt. + Also returns reachability, which is now a *consequence* of the visibility block rather + than an independent warning: anything short of ``everyone`` is refused below, so the + author sees the block's actionable wording instead. It stays on the response because + the reviewer's surface still needs it — an Agent published as PUBLIC can be narrowed + afterwards, which no submit-time gate can catch. """ assistant = await _load_for_author(agent_id, user) reachability = _reachability(assistant) - block_reason = await _memory_space_block_reason(assistant, user) - # Order mirrors submit_listing: an agent that cannot be published at all is not first - # walked through a skill-exposure confirmation. + # Order mirrors submit_listing. Visibility is last because it is the cheapest to fix: + # an author told to widen access, who then hits the memory-space block, has been sent + # round twice for one submission. + block_reason = await _memory_space_block_reason(assistant, user) or _visibility_block_reason( + assistant + ) + # An agent that cannot be published at all is not first walked through a + # skill-exposure confirmation. if block_reason: return [], block_reason, reachability return await _exposed_skills(assistant), None, reachability @@ -286,6 +332,7 @@ async def submit_listing( # Order matters: block before disclosing. An author whose agent cannot be published # at all should not first be walked through a skill-exposure confirmation. await _memory_space_block(assistant, user) + _visibility_block(assistant) exposed = await _exposed_skills(assistant) publisher_id = await _resolve_proposed_publisher(user, request.publisher_id) @@ -370,6 +417,18 @@ async def review_listing( except ListingTransitionError as e: raise ListingError(str(e), status_code=400) from e + # Re-checked here, not just at submit: ``visibility`` is an independent axis the author + # can narrow at any point after submitting, so the gate that ran then says nothing about + # now. Approving anyway would shelve a tile that 404s for every person who taps it. + # Reviewer-facing wording — it is not this admin's job to widen someone else's access. + if target == "published" and assistant.visibility != "PUBLIC": + raise ListingError( + f"This agent's visibility is now {assistant.visibility.title()}, so it can't be " + "published — the store is public, and everyone but the author would get an " + "error opening it. Request changes and ask the author to set it to Public.", + status_code=400, + ) + if category is not None: await _validate_category(category) if publisher_id is not None: @@ -560,13 +619,19 @@ def _drift(assistant: Assistant, listing: AgentListing) -> Optional[str]: def _reachability(assistant: Assistant) -> str: """Who can actually open this Agent, projected from ``visibility`` (see ``ListingReachability``). - The store browse read applies no access check, so this is the only thing standing - between "approved" and a shelf tile that 404s for everyone but the author. Derived on - every read rather than stored — ``visibility`` can change at any time and a cached - copy would be wrong exactly when it mattered. + Derived on every read rather than stored — ``visibility`` can change at any time and a + cached copy would be wrong exactly when it mattered. + + ⚠️ This used to say publishing a SHARED Agent to a team was legitimate, and that it was + the *only* thing standing between "approved" and a tile that 404s for everyone but the + author. Both were wrong. The marketplace is public-only — sharing with named coworkers + is a separate mechanism, and a listing has no audience of its own — so anything short + of PUBLIC is now refused outright at submit and again at approve + (``_visibility_block_reason``). - ⚠️ Advisory only. Publishing a SHARED Agent to a team is legitimate; the fix for the - bad case is the author widening visibility, not this function refusing. + What survives is the case no gate can catch: an Agent published as PUBLIC and narrowed + afterwards. This is what tells a reviewer, and the admin listings table, that an + already-published row has gone unreachable. """ if assistant.visibility == "PUBLIC": return "everyone" diff --git a/backend/src/apis/app_api/agent_designer/services/pin_service.py b/backend/src/apis/app_api/agent_designer/services/pin_service.py index d8919ec7e..5123926d4 100644 --- a/backend/src/apis/app_api/agent_designer/services/pin_service.py +++ b/backend/src/apis/app_api/agent_designer/services/pin_service.py @@ -55,9 +55,13 @@ get_pin_state, remove_pin, ) +from apis.shared.assistants.listing import is_published from apis.shared.assistants.publishers import list_publishers from apis.shared.assistants.role_pins import list_pins_for_roles -from apis.shared.assistants.service import get_assistant_with_access_check +from apis.shared.assistants.service import ( + get_assistant_with_access_check, + resolve_assistant_permission, +) from apis.shared.auth.models import User from apis.shared.rbac.service import get_app_role_service @@ -258,8 +262,38 @@ async def pin_agent(user: User, agent_id: str) -> PinnedAgentResponse: agent_id, user.user_id, user.email ) if assistant is None or permission is None: - # Not-found and access-denied deliberately collapse: telling a stranger that an id + # Not-found and access-denied normally collapse: telling a stranger that an id # exists but is not theirs is a disclosure the store has no reason to make. + # + # The exception is an Agent the store has *already advertised*. Its existence is + # not a secret — the caller is here because a published tile offered them an Add + # button — so collapsing to "not found" withholds nothing and leaves them with an + # error that contradicts the page they are looking at. Publication now requires + # PUBLIC, so this should only be reachable for a listing narrowed after approval; + # it is the failure that has no submit-time gate, which is exactly why it must + # explain itself. The extra read costs nothing on the success path. + # + # Best-effort: this lookup exists only to word the failure better, so it must + # never make the failure worse. A raise here would turn a well-defined 404 into a + # 500 on a path that already knows its answer. + try: + existing, _ = await resolve_assistant_permission(agent_id, user.user_id, user.email) + except Exception: + logger.warning( + f"Could not classify pin denial for {agent_id}; falling back to 404", + exc_info=True, + ) + existing = None + if existing is not None and existing.listing and is_published(existing.listing.state): + logger.warning( + f"Pin denied on published agent {agent_id} for {user.user_id}: " + f"visibility is {existing.visibility}" + ) + raise PinError( + 403, + "This agent is listed in the store but its owner has restricted who can " + "open it, so it can't be added right now.", + ) raise PinError(404, f"Agent not found: {agent_id}") try: diff --git a/backend/tests/routes/test_admin_agent_listings.py b/backend/tests/routes/test_admin_agent_listings.py index e36fc7174..2f990fdd2 100644 --- a/backend/tests/routes/test_admin_agent_listings.py +++ b/backend/tests/routes/test_admin_agent_listings.py @@ -31,7 +31,9 @@ def _make_assistant(**overrides) -> Assistant: description="Find and cite university policy", instructions="Answer from the policy manual.", vectorIndexId="idx-001", - visibility="PRIVATE", + # PUBLIC by default: approval now refuses anything else, so a publishable agent is + # the baseline. Tests of the narrowed-after-submit case override this explicitly. + visibility="PUBLIC", usageCount=12, createdAt="2026-07-01T00:00:00Z", updatedAt="2026-07-01T00:00:00Z", @@ -307,6 +309,35 @@ def test_reviewer_may_recategorize_at_approval(self, app, _no_writes): assert resp.json()["category"] == "Teaching" + @pytest.mark.parametrize("visibility", ["PRIVATE", "SHARED"]) + def test_cannot_approve_an_agent_narrowed_since_submission( + self, app, _no_writes, visibility + ): + """``visibility`` is an independent axis — the submit-time gate says nothing about now. + + The author can narrow access between submitting and being reviewed, and approving + anyway shelves a tile that 404s for everyone who taps it. + """ + with _loaded(_make_assistant(visibility=visibility, listing=_listing("in_review"))): + resp = TestClient(app).post( + "/admin/agents/ast-001/review", json={"decision": "approve"} + ) + + assert resp.status_code == 400 + assert visibility.title() in resp.json()["detail"] + _no_writes.assert_not_called() + + def test_changes_may_still_be_requested_on_a_narrowed_agent(self, app, _no_writes): + """The gate is on publishing, not on reviewing — sending it back must still work.""" + with _loaded(_make_assistant(visibility="PRIVATE", listing=_listing("in_review"))): + resp = TestClient(app).post( + "/admin/agents/ast-001/review", + json={"decision": "request_changes", "note": "Set visibility to Public."}, + ) + + assert resp.status_code == 200 + assert resp.json()["state"] == "changes_requested" + def test_cannot_approve_something_not_in_review(self, app, _no_writes): """Approval is the only door into the store, and in_review is the only way to it.""" with _loaded(_make_assistant(listing=_listing("private"))): diff --git a/backend/tests/routes/test_agent_listing.py b/backend/tests/routes/test_agent_listing.py index 9ed2bda44..aa1d587b9 100644 --- a/backend/tests/routes/test_agent_listing.py +++ b/backend/tests/routes/test_agent_listing.py @@ -29,7 +29,10 @@ def _make_assistant(**overrides) -> Assistant: description="Find and cite university policy", instructions="Answer from the policy manual.", vectorIndexId="idx-001", - visibility="PRIVATE", + # PUBLIC by default because publication now requires it: the marketplace is + # public-only, so an agent that cannot be published is the special case, not the + # baseline. Tests that exercise the block pass ``visibility=`` explicitly. + visibility="PUBLIC", usageCount=0, createdAt="2026-07-01T00:00:00Z", updatedAt="2026-07-01T00:00:00Z", @@ -165,6 +168,75 @@ def test_blocked_submission_writes_nothing(self, app, make_user, _no_writes): _no_writes.assert_not_called() +# ── the marketplace is public-only ─────────────────────────────────────────────────── +class TestVisibilityBlock: + """Publication requires PUBLIC. + + Sharing an agent with named coworkers is a *separate* mechanism, and a listing carries + no audience of its own — so a published SHARED or PRIVATE agent is a tile everyone sees + and nobody but the author can open. That was a live incident: two demo users tapped Add + on a published-but-SHARED agent and got a bare 404. + """ + + @pytest.mark.parametrize( + "visibility,expected", + [("PRIVATE", "private"), ("SHARED", "shared with specific people")], + ) + def test_a_non_public_agent_cannot_be_submitted( + self, app, make_user, _no_writes, visibility, expected + ): + assistant = _make_assistant(visibility=visibility) + mock_auth_user(app, make_user()) + with _owner(assistant): + resp = TestClient(app).post( + "/agents/ast-001/listing/submit", json={"category": "Administration"} + ) + + assert resp.status_code == 400 + assert expected in resp.json()["detail"] + _no_writes.assert_not_called() + + def test_a_public_agent_submits_normally(self, app, make_user, _no_writes): + """The gate must not be so eager it blocks the ordinary path.""" + mock_auth_user(app, make_user()) + with _owner(_make_assistant(visibility="PUBLIC")): + resp = TestClient(app).post( + "/agents/ast-001/listing/submit", json={"category": "Administration"} + ) + + assert resp.status_code == 200 + assert resp.json()["listing"]["state"] == "in_review" + + def test_preflight_shows_the_block_so_the_dialog_can_disable_submit( + self, app, make_user, _no_writes + ): + """Shown and enforced by one function — the dialog and the transition cannot drift.""" + mock_auth_user(app, make_user()) + with _owner(_make_assistant(visibility="SHARED")): + resp = TestClient(app).get("/agents/ast-001/listing/preflight") + + body = resp.json() + assert resp.status_code == 200 + assert body["blockReason"] is not None + assert "shared with specific people" in body["blockReason"] + assert body["reachability"] == "shared_only" + + def test_the_memory_space_block_still_wins_when_both_apply( + self, app, make_user, _no_writes + ): + """Ordering is deliberate: the harder problem is named first, not the cheaper one.""" + assistant = _make_assistant( + visibility="PRIVATE", + bindings=[AgentBinding(kind="memory_space", ref="mem-042", config={})], + ) + mock_auth_user(app, make_user()) + with _owner(assistant), patch(f"{SERVICE_MODULE}.MemorySpaceService") as svc: + svc.return_value.list_spaces_for_user.return_value = [] + resp = TestClient(app).get("/agents/ast-001/listing/preflight") + + assert "memory space" in resp.json()["blockReason"].lower() + + # ── D7.1 — skill exposure is enumerated ────────────────────────────────────────────── class TestSkillDisclosure: def test_submission_enumerates_the_authors_own_skills(self, app, make_user, _no_writes): diff --git a/backend/tests/routes/test_agent_pins.py b/backend/tests/routes/test_agent_pins.py index 239f4d98a..5555f0c3c 100644 --- a/backend/tests/routes/test_agent_pins.py +++ b/backend/tests/routes/test_agent_pins.py @@ -194,6 +194,9 @@ def test_pinning_an_agent_you_cannot_reach_is_a_404(client): patch( f"{PIN_SERVICE}.get_assistant_with_access_check", AsyncMock(return_value=(None, None)) ), + patch( + f"{PIN_SERVICE}.resolve_assistant_permission", AsyncMock(return_value=(None, None)) + ), patch(f"{PIN_SERVICE}.add_pin", add), ): response = client.post("/agents/ast-secret/pin") @@ -202,6 +205,70 @@ def test_pinning_an_agent_you_cannot_reach_is_a_404(client): add.assert_not_awaited() +def test_pinning_a_published_agent_you_cannot_open_is_a_legible_403(client): + """A tile the store already advertised is not a secret — say what went wrong. + + This is the shape that hit two users during the marketplace demo: a published agent + whose visibility denies them. Publication now requires PUBLIC, so it should only be + reachable via a listing narrowed after approval — the case no submit gate can catch. + """ + unreachable = _make_assistant( + visibility="SHARED", + listing={"state": "published", "category": "Administration", "publisherId": "pub-1"}, + ) + add = AsyncMock() + with ( + patch( + f"{PIN_SERVICE}.get_assistant_with_access_check", AsyncMock(return_value=(None, None)) + ), + patch( + f"{PIN_SERVICE}.resolve_assistant_permission", + AsyncMock(return_value=(unreachable, None)), + ), + patch(f"{PIN_SERVICE}.add_pin", add), + ): + response = client.post("/agents/ast-001/pin") + + assert response.status_code == 403 + assert "restricted who can" in response.json()["detail"] + add.assert_not_awaited() + + +def test_an_unpublished_agent_still_collapses_to_404(client): + """The disclosure rule only relaxes for ids the store itself handed out.""" + private = _make_assistant(visibility="PRIVATE", listing=None) + with ( + patch( + f"{PIN_SERVICE}.get_assistant_with_access_check", AsyncMock(return_value=(None, None)) + ), + patch( + f"{PIN_SERVICE}.resolve_assistant_permission", + AsyncMock(return_value=(private, None)), + ), + patch(f"{PIN_SERVICE}.add_pin", AsyncMock()), + ): + response = client.post("/agents/ast-001/pin") + + assert response.status_code == 404 + + +def test_a_failure_classifying_the_denial_falls_back_to_404(client): + """The nicety must never escalate a clean 404 into a 500.""" + with ( + patch( + f"{PIN_SERVICE}.get_assistant_with_access_check", AsyncMock(return_value=(None, None)) + ), + patch( + f"{PIN_SERVICE}.resolve_assistant_permission", + AsyncMock(side_effect=RuntimeError("dynamo is having a day")), + ), + patch(f"{PIN_SERVICE}.add_pin", AsyncMock()), + ): + response = client.post("/agents/ast-001/pin") + + assert response.status_code == 404 + + def test_pinning_past_the_ceiling_is_a_409(client): from apis.shared.assistants.pins import PinLimitError diff --git a/frontend/ai.client/src/app/agents/models/reachability.ts b/frontend/ai.client/src/app/agents/models/reachability.ts index 6ad82b872..d1018bb3d 100644 --- a/frontend/ai.client/src/app/agents/models/reachability.ts +++ b/frontend/ai.client/src/app/agents/models/reachability.ts @@ -9,8 +9,14 @@ * that, and they must not be told two different things — this is the same reason * `runnabilityMessage` exists next door. * - * ⚠️ Advisory, never a gate. Publishing a SHARED Agent to a team is a legitimate thing to - * do; the wording says what will happen, and leaves the decision where it belongs. + * ⚠️ This used to describe publishing a SHARED Agent to a team as legitimate. It is not: + * the marketplace is public-only, sharing with named coworkers is a separate mechanism, + * and the backend now refuses anything short of PUBLIC at submit and at approve. So the + * *author* message below is a version-skew guard rather than the normal path — the submit + * dialog renders the backend's block instead, and the two are mutually exclusive there. + * + * The reviewer message and label still carry their original weight: an Agent published as + * PUBLIC can be narrowed afterwards, and no submit-time gate can catch that. */ /** Mirrors the backend `ListingReachability`. */ From 2ece2695955b13407b1fd96ecd11ca55e6db9a1c Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Wed, 29 Jul 2026 08:47:26 -0600 Subject: [PATCH 2/2] feat(marketplace): let authors go public from the submit dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requiring PUBLIC to publish left the *common* path a dead end. Every agent is created PRIVATE, so a first-time author opened Submit to a red block telling them to go set visibility on the agent editor and come back — two screens for one decision, and worse than the amber warning it replaced. Consent now lives where the decision is made. The submit dialog shows a checkbox ("Make this agent public"), `makePublic` rides the submit request, and `write_listing` widens visibility in the same write as the listing. One write matters: two could leave an agent listed but unreachable, which is the exact state this whole gate exists to prevent. It stays consent rather than a side door. The box starts unticked, Submit is disabled until it is ticked, and the flag defaults to false — so a direct API caller who omits it is refused exactly as before, and an already-public agent never has its visibility rewritten. `blockReason` and `requiresPublic` are now separate signals. A block means "leave the dialog and fix something" and hides the form; needing to go public is resolvable in place. Collapsing them was what made this a dead end, and the memory-space block is now the only true dead end left. Ticking the box does not wave it through. Drops `reachabilityAuthorMessage`: it told authors to "set Visibility to Public first", which is now wrong advice. That file is reviewer-facing only — an agent published as PUBLIC and narrowed afterwards is still the case no gate catches. Co-Authored-By: Claude Opus 5 --- .../src/apis/app_api/agent_designer/routes.py | 12 +- .../services/listing_service.py | 64 +++++---- .../shared/assistants/listing_repository.py | 9 ++ backend/src/apis/shared/assistants/models.py | 39 +++++- backend/tests/routes/test_agent_listing.py | 90 +++++++++++-- .../submit-listing-dialog.component.spec.ts | 122 ++++++++++++++++++ .../submit-listing-dialog.component.ts | 87 ++++++++++--- .../app/agents/models/reachability.spec.ts | 14 +- .../src/app/agents/models/reachability.ts | 34 ++--- .../src/app/agents/models/store.model.ts | 24 +++- 10 files changed, 402 insertions(+), 93 deletions(-) create mode 100644 frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.spec.ts diff --git a/backend/src/apis/app_api/agent_designer/routes.py b/backend/src/apis/app_api/agent_designer/routes.py index e6fc8e972..3e154942c 100644 --- a/backend/src/apis/app_api/agent_designer/routes.py +++ b/backend/src/apis/app_api/agent_designer/routes.py @@ -561,17 +561,21 @@ async def agent_listing_preflight_endpoint( ): """What the submit dialog needs before the author commits (D7, owner only). - A read-only rehearsal of the submit checks: the skills publication would expose, - and the memory-space block if there is one. Declared before ``/listing/submit`` - only for reading order — the paths are literal and do not collide. + A read-only rehearsal of the submit checks: the skills publication would expose, the + memory-space block if there is one, and whether the author still has to consent to + going public. Declared before ``/listing/submit`` only for reading order — the paths + are literal and do not collide. """ try: - exposed, block_reason, reachability = await preflight_listing(agent_id, current_user) + exposed, block_reason, reachability, requires_public = await preflight_listing( + agent_id, current_user + ) return ListingPreflightResponse( agent_id=agent_id, exposed_skills=exposed, block_reason=block_reason, reachability=reachability, + requires_public=requires_public, ) except ListingError as e: raise HTTPException(status_code=e.status_code, detail=e.message) diff --git a/backend/src/apis/app_api/agent_designer/services/listing_service.py b/backend/src/apis/app_api/agent_designer/services/listing_service.py index 1173cfed2..4afcf2ebe 100644 --- a/backend/src/apis/app_api/agent_designer/services/listing_service.py +++ b/backend/src/apis/app_api/agent_designer/services/listing_service.py @@ -220,13 +220,21 @@ def _visibility_block_reason(assistant: Assistant) -> Optional[str]: ) -def _visibility_block(assistant: Assistant) -> None: - """Block submission unless the Agent is ``PUBLIC``. - - Deliberately a refusal rather than a silent widening: publication must not be a side - door that changes who can reach an Agent. The author says "make this public" in the one - place that means it, and the store trusts ``visibility`` rather than overwriting it. +def _visibility_block(assistant: Assistant, *, consented: bool) -> None: + """Refuse an Agent that is not ``PUBLIC`` and whose author has not consented to it. + + Not a silent widening — publication must never be a side door that changes who can + reach an Agent. But the refusal alone made the *common* path a dead end: every Agent + starts PRIVATE, so a first-time author was told to go set visibility on another screen + and come back. ``consented`` is the submit dialog's checkbox: the author is looking at + what the store will say about their Agent and ticks a box that says it becomes public. + That is consent captured where it means something, and it is why the widening is + allowed to ride the same write. + + An omitted flag still refuses, so a direct API caller cannot widen an Agent by accident. """ + if consented: + return reason = _visibility_block_reason(assistant) if reason: raise ListingError(reason, status_code=400) @@ -285,7 +293,7 @@ async def _resolve_proposed_publisher(user: User, publisher_id: Optional[str]) - # ── author transitions ─────────────────────────────────────────────────────────────── async def preflight_listing( agent_id: str, user: User -) -> Tuple[List[SkillExposure], Optional[str], str]: +) -> Tuple[List[SkillExposure], Optional[str], str, bool]: """Run the D7 checks **without** transitioning, for the submit dialog. D7.1 asks the dialog to enumerate the exposed skills *before* the author commits, @@ -296,25 +304,23 @@ async def preflight_listing( Owner-only, like every other author path: the skill exposure is a statement about what the *owner's* publication would reveal, and it is not an editor's to see. - Also returns reachability, which is now a *consequence* of the visibility block rather - than an independent warning: anything short of ``everyone`` is refused below, so the - author sees the block's actionable wording instead. It stays on the response because - the reviewer's surface still needs it — an Agent published as PUBLIC can be narrowed - afterwards, which no submit-time gate can catch. + ``requires_public`` is deliberately **not** folded into ``block_reason``. A block sends + the author out of the dialog; needing to go public is something the dialog itself can + resolve, with the consent checkbox that sets ``make_public``. Returning them as one + field is what made the ordinary path a dead end. + + Reachability still rides along for the same reason it always did — an Agent published + as PUBLIC can be narrowed afterwards, which no submit-time gate can catch. """ assistant = await _load_for_author(agent_id, user) reachability = _reachability(assistant) - # Order mirrors submit_listing. Visibility is last because it is the cheapest to fix: - # an author told to widen access, who then hits the memory-space block, has been sent - # round twice for one submission. - block_reason = await _memory_space_block_reason(assistant, user) or _visibility_block_reason( - assistant - ) + requires_public = _visibility_block_reason(assistant) is not None + block_reason = await _memory_space_block_reason(assistant, user) # An agent that cannot be published at all is not first walked through a # skill-exposure confirmation. if block_reason: - return [], block_reason, reachability - return await _exposed_skills(assistant), None, reachability + return [], block_reason, reachability, requires_public + return await _exposed_skills(assistant), None, reachability, requires_public async def submit_listing( @@ -332,7 +338,7 @@ async def submit_listing( # Order matters: block before disclosing. An author whose agent cannot be published # at all should not first be walked through a skill-exposure confirmation. await _memory_space_block(assistant, user) - _visibility_block(assistant) + _visibility_block(assistant, consented=request.make_public) exposed = await _exposed_skills(assistant) publisher_id = await _resolve_proposed_publisher(user, request.publisher_id) @@ -355,9 +361,23 @@ async def submit_listing( # path). ``None`` means "leave it alone" — an author resubmitting without touching the # field must not have their existing subtitle blanked. tagline = (request.tagline or "").strip() or None + # Only widen when it actually needs widening: an Agent that is already PUBLIC must not + # have ``visibility`` rewritten just because the box was ticked, and a no-op write is + # a lie in the audit trail. + widen_to = "PUBLIC" if (request.make_public and assistant.visibility != "PUBLIC") else None await write_listing( - agent_id, listing, assistant.created_at, updated_at=now, tagline=tagline + agent_id, + listing, + assistant.created_at, + updated_at=now, + tagline=tagline, + visibility=widen_to, ) + if widen_to: + logger.info( + f"🌐 Agent {agent_id} widened {assistant.visibility} → PUBLIC on submission " + f"by {user.user_id}" + ) logger.info(f"📨 Agent {agent_id} submitted for review by {user.user_id}") return listing, exposed diff --git a/backend/src/apis/shared/assistants/listing_repository.py b/backend/src/apis/shared/assistants/listing_repository.py index faf4901c5..3d4e3bf47 100644 --- a/backend/src/apis/shared/assistants/listing_repository.py +++ b/backend/src/apis/shared/assistants/listing_repository.py @@ -58,6 +58,7 @@ async def write_listing( icon_key: Optional[str] = None, name: Optional[str] = None, updated_at: Optional[str] = None, + visibility: Optional[str] = None, ) -> None: """Persist a listing block and reconcile the sparse directory keys in one write. @@ -65,6 +66,11 @@ async def write_listing( is why browse is newest-first. The optional presentation fields let an admin D13 edit ride along in the same call rather than racing a second update. + ``visibility`` rides along for the same reason, and for one more: publishing an Agent + the author has just consented to make public must not be able to half-happen. Two + writes could leave it listed but unreachable — precisely the state this whole gate + exists to prevent — so the widening and the listing land together or not at all. + Raises ``ValueError`` (via the conditional check) if the agent no longer exists. """ from botocore.exceptions import ClientError @@ -93,6 +99,9 @@ async def write_listing( set_parts.append("#name = :name") names["#name"] = "name" values[":name"] = name + if visibility is not None: + set_parts.append("visibility = :visibility") + values[":visibility"] = visibility if keys: for attr, value in keys.items(): diff --git a/backend/src/apis/shared/assistants/models.py b/backend/src/apis/shared/assistants/models.py index 61706e12c..ec0060b22 100644 --- a/backend/src/apis/shared/assistants/models.py +++ b/backend/src/apis/shared/assistants/models.py @@ -544,6 +544,19 @@ class SubmitListingRequest(BaseModel): ), ) note: Optional[str] = Field(None, max_length=2000, description="Optional note to the reviewer") + make_public: bool = Field( + False, + alias="makePublic", + description=( + "The author's explicit consent to widen this Agent's visibility to PUBLIC as " + "part of publishing it. The marketplace is public-only, and every new Agent " + "starts PRIVATE, so without this the common path is a dead end: the author is " + "told to go set visibility on a different screen and come back. Defaulting to " + "``False`` is what keeps this consent rather than a side door — an omitted " + "flag is refused exactly as before, so a direct API caller cannot widen an " + "Agent's access by accident." + ), + ) tagline: Optional[str] = Field( None, max_length=80, @@ -583,10 +596,15 @@ class ListingSubmissionResponse(BaseModel): class ListingPreflightResponse(BaseModel): """What the submit dialog shows before the author commits (D7). - The same two checks ``submit_listing`` runs, answered without a transition: the - skills publication would expose, and the memory-space block if there is one. A - ``blockReason`` means the Submit control is disabled and this text explains why — - the dialog never re-derives that rule for itself. + The same checks ``submit_listing`` runs, answered without a transition: the skills + publication would expose, the memory-space block if there is one, and whether the + author has to consent to going public. + + ⚠️ ``blockReason`` and ``requiresPublic`` are deliberately **separate signals**, not + one field. A block is "leave this dialog and go fix something"; ``requiresPublic`` is + "tick the box and I will handle it". Folding the second into the first is what made + the common path a dead end — every new Agent starts PRIVATE, so the author was being + bounced to another screen on their very first submission. """ model_config = ConfigDict(populate_by_name=True) @@ -600,7 +618,18 @@ class ListingPreflightResponse(BaseModel): block_reason: Optional[str] = Field( None, alias="blockReason", - description="Why this agent cannot be submitted at all (D7.2); null when it can", + description=( + "Why this agent cannot be submitted at all (D7.2), and cannot be fixed from " + "the dialog; null when it can. Submit is hidden, not merely disabled." + ), + ) + requires_public: bool = Field( + False, + alias="requiresPublic", + description=( + "This Agent is not PUBLIC yet, so submitting must widen it. The dialog shows " + "the consent control and sends ``makePublic``; it is not a block." + ), ) reachability: ListingReachability = Field( ..., diff --git a/backend/tests/routes/test_agent_listing.py b/backend/tests/routes/test_agent_listing.py index aa1d587b9..43ef57d21 100644 --- a/backend/tests/routes/test_agent_listing.py +++ b/backend/tests/routes/test_agent_listing.py @@ -207,24 +207,36 @@ def test_a_public_agent_submits_normally(self, app, make_user, _no_writes): assert resp.status_code == 200 assert resp.json()["listing"]["state"] == "in_review" - def test_preflight_shows_the_block_so_the_dialog_can_disable_submit( + def test_preflight_asks_for_consent_rather_than_blocking( self, app, make_user, _no_writes ): - """Shown and enforced by one function — the dialog and the transition cannot drift.""" + """Needing to go public is fixable *in the dialog*, so it is not a block. + + Folding it into ``blockReason`` sent the author to another screen on their very + first submission — every agent starts PRIVATE. + """ mock_auth_user(app, make_user()) with _owner(_make_assistant(visibility="SHARED")): resp = TestClient(app).get("/agents/ast-001/listing/preflight") body = resp.json() assert resp.status_code == 200 - assert body["blockReason"] is not None - assert "shared with specific people" in body["blockReason"] + assert body["requiresPublic"] is True + assert body["blockReason"] is None, "not a block — the checkbox resolves it" assert body["reachability"] == "shared_only" - def test_the_memory_space_block_still_wins_when_both_apply( + def test_preflight_asks_nothing_of_an_already_public_agent( self, app, make_user, _no_writes ): - """Ordering is deliberate: the harder problem is named first, not the cheaper one.""" + mock_auth_user(app, make_user()) + with _owner(_make_assistant(visibility="PUBLIC")): + body = TestClient(app).get("/agents/ast-001/listing/preflight").json() + + assert body["requiresPublic"] is False + assert body["blockReason"] is None + + def test_the_memory_space_block_is_still_a_block(self, app, make_user, _no_writes): + """A real dead end stays one — nothing in the dialog can resolve it.""" assistant = _make_assistant( visibility="PRIVATE", bindings=[AgentBinding(kind="memory_space", ref="mem-042", config={})], @@ -232,9 +244,71 @@ def test_the_memory_space_block_still_wins_when_both_apply( mock_auth_user(app, make_user()) with _owner(assistant), patch(f"{SERVICE_MODULE}.MemorySpaceService") as svc: svc.return_value.list_spaces_for_user.return_value = [] - resp = TestClient(app).get("/agents/ast-001/listing/preflight") + body = TestClient(app).get("/agents/ast-001/listing/preflight").json() + + assert "memory space" in body["blockReason"].lower() + assert body["requiresPublic"] is True, "still true — it just is not what stops them" + + +class TestGoingPublicOnSubmission: + """The consent checkbox: the author widens visibility from the submit dialog.""" + + @pytest.mark.parametrize("visibility", ["PRIVATE", "SHARED"]) + def test_consent_widens_visibility_in_the_same_write( + self, app, make_user, _no_writes, visibility + ): + """One write, or an agent could end up listed but unreachable — the whole bug.""" + mock_auth_user(app, make_user()) + with _owner(_make_assistant(visibility=visibility)): + resp = TestClient(app).post( + "/agents/ast-001/listing/submit", + json={"category": "Administration", "makePublic": True}, + ) + + assert resp.status_code == 200 + assert _no_writes.call_args.kwargs["visibility"] == "PUBLIC" + + def test_an_already_public_agent_is_not_rewritten(self, app, make_user, _no_writes): + """A no-op write would be a lie in the audit trail.""" + mock_auth_user(app, make_user()) + with _owner(_make_assistant(visibility="PUBLIC")): + resp = TestClient(app).post( + "/agents/ast-001/listing/submit", + json={"category": "Administration", "makePublic": True}, + ) - assert "memory space" in resp.json()["blockReason"].lower() + assert resp.status_code == 200 + assert _no_writes.call_args.kwargs["visibility"] is None + + def test_consent_does_not_bypass_the_memory_space_block( + self, app, make_user, _no_writes + ): + """Ticking a visibility box must not wave through an unrelated, real block.""" + assistant = _make_assistant( + visibility="PRIVATE", + bindings=[AgentBinding(kind="memory_space", ref="mem-042", config={})], + ) + mock_auth_user(app, make_user()) + with _owner(assistant), patch(f"{SERVICE_MODULE}.MemorySpaceService") as svc: + svc.return_value.list_spaces_for_user.return_value = [] + resp = TestClient(app).post( + "/agents/ast-001/listing/submit", + json={"category": "Administration", "makePublic": True}, + ) + + assert resp.status_code == 400 + _no_writes.assert_not_called() + + def test_an_omitted_flag_still_refuses(self, app, make_user, _no_writes): + """Consent defaults off, so a direct API caller cannot widen an agent by accident.""" + mock_auth_user(app, make_user()) + with _owner(_make_assistant(visibility="PRIVATE")): + resp = TestClient(app).post( + "/agents/ast-001/listing/submit", json={"category": "Administration"} + ) + + assert resp.status_code == 400 + _no_writes.assert_not_called() # ── D7.1 — skill exposure is enumerated ────────────────────────────────────────────── diff --git a/frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.spec.ts b/frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.spec.ts new file mode 100644 index 000000000..2c6e7e054 --- /dev/null +++ b/frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.spec.ts @@ -0,0 +1,122 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { TestBed } from '@angular/core/testing'; +import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog'; + +import { SubmitListingDialogComponent } from './submit-listing-dialog.component'; +import { AgentListingService } from '../services/agent-listing.service'; +import { ListingPreflight, SubmitListingRequest } from '../models/store.model'; + +/** + * The dialog owns the *consent* rule, and getting it wrong has two bad shapes: an author + * blocked from publishing at all (the dead end this control exists to remove), or an + * agent's visibility widened without the author having said so. + * + * DI tokens rather than vi.mock, per project convention — a shared worker pool makes + * module mocks leak across specs. + */ +describe('SubmitListingDialogComponent — going public', () => { + let submitted: { agentId: string; request: SubmitListingRequest }[]; + + function build(preflight: Partial): SubmitListingDialogComponent { + submitted = []; + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + { provide: DialogRef, useValue: { close: () => undefined } }, + { provide: DIALOG_DATA, useValue: { agentId: 'ast-001', agentName: 'Policy Lookup' } }, + { + provide: AgentListingService, + useValue: { + loadCategories: async () => [{ id: 'Administration', label: 'Administration' }], + preflight: async (): Promise => ({ + agentId: 'ast-001', + exposedSkills: [], + blockReason: null, + requiresPublic: false, + reachability: 'everyone', + ...preflight, + }), + submit: async (agentId: string, request: SubmitListingRequest) => { + submitted.push({ agentId, request }); + return { listing: { state: 'in_review', category: request.category } }; + }, + }, + }, + ], + }); + return TestBed.createComponent(SubmitListingDialogComponent).componentInstance; + } + + describe('when the agent is not public yet', () => { + let component: SubmitListingDialogComponent; + + beforeEach(async () => { + component = build({ requiresPublic: true, reachability: 'owner_only' }); + await component.ngOnInit(); + component.category.set('Administration'); + }); + + it('asks for consent instead of dead-ending the author', () => { + // The whole point: no blockReason, so the form renders and the author can act here + // rather than being sent to the agent editor and back. + expect(component.requiresPublic()).toBe(true); + expect(component.blockReason()).toBeNull(); + }); + + it('starts unticked — going public is a decision, not a default', () => { + expect(component.makePublic()).toBe(false); + }); + + it('holds Submit until the author consents', () => { + expect(component.canSubmit()).toBe(false); + component.makePublic.set(true); + expect(component.canSubmit()).toBe(true); + }); + + it('sends the consent so the backend widens in the same write', async () => { + component.makePublic.set(true); + await component.onSubmit(); + + expect(submitted).toHaveLength(1); + expect(submitted[0].request.makePublic).toBe(true); + }); + + it('says what going public changes from, per starting state', async () => { + expect(component.makePublicHelp()).toMatch(/only you can open it/i); + + const shared = build({ requiresPublic: true, reachability: 'shared_only' }); + await shared.ngOnInit(); + expect(shared.makePublicHelp()).toMatch(/shared/i); + expect(shared.makePublicHelp()).not.toBe(component.makePublicHelp()); + }); + }); + + describe('when the agent is already public', () => { + it('asks nothing and does not send a consent it never sought', async () => { + const component = build({ requiresPublic: false, reachability: 'everyone' }); + await component.ngOnInit(); + component.category.set('Administration'); + + expect(component.requiresPublic()).toBe(false); + expect(component.canSubmit()).toBe(true); + + await component.onSubmit(); + expect(submitted[0].request.makePublic).toBeUndefined(); + }); + }); + + describe('when something really does block submission', () => { + it('stays a dead end — the consent checkbox must not wave it through', async () => { + const component = build({ + blockReason: 'This agent cannot be published while it is bound to a memory space.', + requiresPublic: true, + reachability: 'owner_only', + }); + await component.ngOnInit(); + component.category.set('Administration'); + component.makePublic.set(true); + + expect(component.canSubmit()).toBe(false); + }); + }); +}); diff --git a/frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.ts b/frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.ts index 21a3a52a9..1a42071da 100644 --- a/frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.ts +++ b/frontend/ai.client/src/app/agents/components/submit-listing-dialog.component.ts @@ -3,7 +3,7 @@ import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog'; import { NgIcon, provideIcons } from '@ng-icons/core'; import { heroXMark, heroExclamationTriangle, heroEye, heroEyeSlash } from '@ng-icons/heroicons/outline'; import { AgentListingService } from '../services/agent-listing.service'; -import { reachabilityAuthorMessage } from '../models/reachability'; +import { ListingReachability } from '../models/reachability'; import { AgentCategory, AgentListingBlock, @@ -30,14 +30,20 @@ export type SubmitListingDialogResult = AgentListingBlock | undefined; * Submit an Agent to the marketplace (D2), with the D7 disclosures. * * The dialog does not decide anything. It asks `GET /agents/{id}/listing/preflight`, - * which runs the same two checks the transition enforces, and renders the answers: + * which runs the same checks the transition enforces, and renders the answers: * * * **Skill exposure (D7.1)** — publishing an Agent effectively publishes the contents * of every skill its author wrote and bound, because Skills v2 resolves a `skill` * binding on `skill.owner_id == agent.owner_id`. The names are listed, not counted. * * **Memory spaces (D7.2)** — a `memory_space` binding blocks submission outright, so - * Submit is disabled and the backend's message (which names the space) explains why. + * the form is hidden and the backend's message (which names the space) explains why. * The author learns this before filling in a category, not after clicking. + * * **Going public** — the marketplace is public-only, and every Agent starts PRIVATE, + * so most first submissions need visibility widened. That is a *checkbox here*, not a + * block: `makePublic` rides the submit request and the backend widens in the same + * write. Sending the author to the agent editor to change a setting and come back was + * the whole reason this needed fixing — but it stays consent, so the box starts + * unticked and Submit is disabled until it is ticked. */ @Component({ selector: 'app-submit-listing-dialog', @@ -100,15 +106,34 @@ export type SubmitListingDialogResult = AgentListingBlock | undefined;

{{ reason }}

} @else { - - @if (reachabilityWarning(); as warning) { + + @if (requiresPublic()) {
-
} @@ -246,11 +271,15 @@ export class SubmitListingDialogComponent implements OnInit { readonly exposedSkills = signal([]); readonly blockReason = signal(null); /** - * Who will be able to open this once it is shelved. A *warning*, not a block — the - * author may legitimately publish a SHARED agent to their team, so this never disables - * Submit; it just makes sure "nobody can open my tile" is not discovered afterwards. + * This agent is not PUBLIC yet. Not a block: the checkbox below resolves it, and + * submitting widens visibility in the same write. The marketplace is public-only, and + * every agent starts PRIVATE, so this is the ordinary first-submission path. */ - readonly reachabilityWarning = signal(null); + readonly requiresPublic = signal(false); + /** The author's consent. Starts unticked — going public is a decision, not a default. */ + readonly makePublic = signal(false); + /** The current reachability, kept to word the consent copy for what it actually changes. */ + private readonly reachability = signal('everyone'); readonly loading = signal(true); readonly submitting = signal(false); readonly error = signal(null); @@ -263,7 +292,25 @@ export class SubmitListingDialogComponent implements OnInit { readonly isResubmission = computed(() => !!this.data.listing); readonly canSubmit = computed( - () => !!this.category() && !this.submitting() && !this.blockReason(), + () => + !!this.category() && + !this.submitting() && + !this.blockReason() && + // Consent is required, not implied: the backend refuses an omitted flag, so a + // Submit that looked enabled here would fail on the round trip. + (!this.requiresPublic() || this.makePublic()), + ); + + /** + * Says what going public actually changes *from*, so "shared with 3 people → everyone" + * and "only me → everyone" do not read as the same sentence. + */ + readonly makePublicHelp = computed(() => + this.reachability() === 'shared_only' + ? 'Right now only the people it is shared with can open it. The store is public, so ' + + 'publishing it makes it available to everyone at Boise State.' + : 'Right now only you can open it. The store is public, so publishing it makes it ' + + 'available to everyone at Boise State.', ); /** A resubmission is answering a reviewer; a first submission is introducing itself. */ @@ -289,7 +336,8 @@ export class SubmitListingDialogComponent implements OnInit { this.categories.set(categories); this.exposedSkills.set(preflight.exposedSkills ?? []); this.blockReason.set(preflight.blockReason ?? null); - this.reachabilityWarning.set(reachabilityAuthorMessage(preflight.reachability)); + this.requiresPublic.set(preflight.requiresPublic ?? false); + this.reachability.set(preflight.reachability); // Only preselect a category that is still open for new listings. if (!categories.some((c) => c.id === this.category())) { this.category.set(''); @@ -313,6 +361,10 @@ export class SubmitListingDialogComponent implements OnInit { this.tagline.set((event.target as HTMLInputElement).value); } + onMakePublicChange(event: Event): void { + this.makePublic.set((event.target as HTMLInputElement).checked); + } + async onSubmit(): Promise { if (!this.canSubmit()) return; this.submitting.set(true); @@ -321,6 +373,9 @@ export class SubmitListingDialogComponent implements OnInit { const response = await this.listings.submit(this.data.agentId, { category: this.category(), note: this.note().trim() || undefined, + // Sent only when it is actually being asked for, so an already-public agent's + // request does not carry a consent it never sought. + makePublic: this.requiresPublic() ? this.makePublic() : undefined, // Omitted rather than blanked when empty — the backend reads `undefined` as // "leave the existing tagline alone". tagline: this.tagline().trim() || undefined, diff --git a/frontend/ai.client/src/app/agents/models/reachability.spec.ts b/frontend/ai.client/src/app/agents/models/reachability.spec.ts index a065d5154..60ca82465 100644 --- a/frontend/ai.client/src/app/agents/models/reachability.spec.ts +++ b/frontend/ai.client/src/app/agents/models/reachability.spec.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from 'vitest'; import { ListingReachability, - reachabilityAuthorMessage, reachabilityIsLimited, reachabilityLabel, reachabilityReviewerMessage, @@ -13,35 +12,30 @@ describe('reachability', () => { it('says nothing at all when the agent is public', () => { // The whole point is that this is quiet in the common case — a warning shown on every // row is a warning nobody reads. - expect(reachabilityAuthorMessage('everyone')).toBeNull(); expect(reachabilityReviewerMessage('everyone')).toBeNull(); expect(reachabilityIsLimited('everyone')).toBe(false); }); - it.each(LIMITED)('warns both audiences when reachability is %s', (value) => { + it.each(LIMITED)('warns the reviewer when reachability is %s', (value) => { expect(reachabilityIsLimited(value)).toBe(true); - expect(reachabilityAuthorMessage(value)).toBeTruthy(); expect(reachabilityReviewerMessage(value)).toBeTruthy(); }); it.each(LIMITED)('names the consequence, not just the state, for %s', (value) => { // A message that only says "this is private" tells the reader something they can // already see. Both voices have to say what will actually happen. - expect(reachabilityAuthorMessage(value)).toMatch(/error when they open it/); expect(reachabilityReviewerMessage(value)!).toMatch(/nobody else can use|get an error/); }); - it('tells the author how to fix it, and does not tell the reviewer to', () => { + it('does not tell the reviewer to change someone else\u2019s visibility', () => { // The author owns visibility; the reviewer does not, and telling them to change it // would be inviting exactly the silent access-widening this feature refuses to do. - expect(reachabilityAuthorMessage('owner_only')).toMatch(/set Visibility to Public/i); + // The author is no longer told anything here at all — the submit dialog's consent + // checkbox widens visibility for them. expect(reachabilityReviewerMessage('owner_only')).not.toMatch(/set Visibility/i); }); it('distinguishes owner_only from shared_only rather than collapsing them', () => { - expect(reachabilityAuthorMessage('owner_only')).not.toBe( - reachabilityAuthorMessage('shared_only'), - ); expect(reachabilityReviewerMessage('owner_only')).not.toBe( reachabilityReviewerMessage('shared_only'), ); diff --git a/frontend/ai.client/src/app/agents/models/reachability.ts b/frontend/ai.client/src/app/agents/models/reachability.ts index d1018bb3d..03c498e82 100644 --- a/frontend/ai.client/src/app/agents/models/reachability.ts +++ b/frontend/ai.client/src/app/agents/models/reachability.ts @@ -11,12 +11,12 @@ * * ⚠️ This used to describe publishing a SHARED Agent to a team as legitimate. It is not: * the marketplace is public-only, sharing with named coworkers is a separate mechanism, - * and the backend now refuses anything short of PUBLIC at submit and at approve. So the - * *author* message below is a version-skew guard rather than the normal path — the submit - * dialog renders the backend's block instead, and the two are mutually exclusive there. + * and the backend now refuses anything short of PUBLIC at submit and at approve. * - * The reviewer message and label still carry their original weight: an Agent published as - * PUBLIC can be narrowed afterwards, and no submit-time gate can catch that. + * So this file is now **reviewer-facing only**. The author never sees these strings: the + * submit dialog asks them to consent to going public and does it for them. What survives + * here is the case no submit-time gate can catch — an Agent published as PUBLIC and + * narrowed afterwards, which is what the reviewer and the admin table need to know about. */ /** Mirrors the backend `ListingReachability`. */ @@ -27,26 +27,12 @@ export function reachabilityIsLimited(value: ListingReachability): boolean { return value !== 'everyone'; } -/** - * What the **author** is told before submitting — second person, and it names the fix. - * Returns null when there is nothing to say. +/* + * There was a `reachabilityAuthorMessage` here. It told the author to go "set Visibility + * to Public first" — advice that is now wrong, because the submit dialog widens + * visibility itself via its consent checkbox. The author-facing copy lives with that + * control (`makePublicHelp`), where it can say what going public changes *from*. */ -export function reachabilityAuthorMessage(value: ListingReachability): string | null { - if (value === 'owner_only') { - return ( - 'Only you can open this agent. If it is published, people will see it in the store ' + - 'but get an error when they open it — set Visibility to Public first.' - ); - } - if (value === 'shared_only') { - return ( - 'Only people this agent is shared with can open it. Anyone else will see it in the ' + - 'store but get an error when they open it — set Visibility to Public to reach the ' + - 'whole university.' - ); - } - return null; -} /** * What the **reviewer** is told before approving — third person, and it does not tell them diff --git a/frontend/ai.client/src/app/agents/models/store.model.ts b/frontend/ai.client/src/app/agents/models/store.model.ts index 550d65637..94e5d0428 100644 --- a/frontend/ai.client/src/app/agents/models/store.model.ts +++ b/frontend/ai.client/src/app/agents/models/store.model.ts @@ -85,6 +85,13 @@ export interface SubmitListingRequest { /** Omit to publish under the author's own individual profile (D12). */ publisherId?: string; note?: string; + /** + * The author's explicit consent to make this agent PUBLIC as part of publishing it. + * The marketplace is public-only and every agent starts PRIVATE, so this is the normal + * path, not an edge case. Omitting it is refused by the backend — the widening is + * consent, never a side effect of submitting. + */ + makePublic?: boolean; /** * Shelf subtitle (D4). Omit to leave any existing tagline untouched — a resubmission * that does not touch the field must not blank it. @@ -101,16 +108,25 @@ export interface SkillExposure { /** * The D7 answers, before the author commits. * - * `blockReason` non-null means submission is impossible (a `memory_space` binding, - * D7.2) and the text explains why. The dialog renders it; it never re-derives it. + * ⚠️ `blockReason` and `requiresPublic` are separate on purpose. A block is "leave this + * dialog and go fix something" — the form is hidden, because nothing below it would help. + * `requiresPublic` is "tick the box and submitting handles it". They were briefly one + * field, and that made the *common* path a dead end: every agent starts PRIVATE, so a + * first-time author was bounced to another screen to set visibility and come back. */ export interface ListingPreflight { agentId: string; exposedSkills: SkillExposure[]; blockReason?: string | null; /** - * Who would be able to open this agent once shelved. Advisory — publishing a - * PRIVATE/SHARED agent is allowed, but the tile 404s for everyone it is not shared with. + * This agent is not PUBLIC yet, so submitting must widen it. Drives the consent + * checkbox — never disables Submit on its own. + */ + requiresPublic?: boolean; + /** + * Who would be able to open this agent once shelved. At submit time this is now a + * consequence of `requiresPublic`; it still matters to the reviewer, because an agent + * published as PUBLIC can be narrowed afterwards. */ reachability: ListingReachability; }