test: cover hosted + local schemas with agent_view test cases by mwbrooks · Pull Request #75 · slackapi/manifest-schema

mwbrooks · 2026-06-19T04:07:43Z

Summary

Stacks on top of #74 — base branch is mwbrooks-manifest-agent-view.

Parametrizes the existing v1/v2 test_success and test_invalid cases across both schema surfaces:
- hosted — the GitHub-pinned router schema that IDEs and downstream consumers actually fetch
- local — the versioned schema files in schemas/ that the next release will ship
Hoists a shared agent_view valid fixture and a shared invalid-mutation table into tests/utils.py. v1 and v2 reuse the same data because agent_view is defined identically across versions.
Adds TestManifest{V1,V2}AgentView covering valid manifest + 4 invalid mutations (maxItems on suggested_prompts, missing description on actions[], agent_description over 300 chars, additionalProperties rejected).
agent_view cases are pinned to local-only via SCHEMAS_LOCAL_ONLY until the property is published in a release. Once published, promote them by deleting the local-only constant.

Why

Existing tests validate against the GitHub-pinned router (currently v0.4.0), so edits to the local versioned schema files were never exercised before release. This made past PRs like mcp_servers and is_mcp_enabled ship without local test coverage. The hosted+local matrix means new properties get validated locally on every PR while still confirming the released router stays compatible with the canonical fixtures.

Test plan

pytest -v — 20/20 pass (10 base × 2 schemas + 10 agent_view local-only)
./scripts/lint.sh — clean
CI green

Follow-ups

Once agent_view ships in a release, delete SCHEMAS_LOCAL_ONLY so the agent_view tests run against the hosted schema too.

Mirrors the canonical agent_view definition from webapp's manifest schema. Sibling of assistant_view; supports agent_description, suggested_prompts (max 4), and actions. Adds local-schema test coverage so edits to the versioned schema files are validated before release. Existing tests run through the GitHub-pinned schema and would not catch local changes.

Reverts the local-schema tests added in the previous commit so this PR matches the existing schema-only feature pattern (mcp_servers, is_mcp_enabled, optional scopes). Coverage will land in a separate PR that refactors the test suite to run every case against both the hosted router schema and the local versioned files.

Refactors v1/v2 test suites so each content-driven case runs against both the hosted router schema (what consumers fetch) and the local versioned schema file (what the next release will ship). Router-only behavior like test_no_metadata stays a single non-parametrized test. Adds shared agent_view fixture + invalid-mutation table in tests/utils.py so v1 and v2 stay in sync — agent_view is defined identically across both versions. New TestManifest{V1,V2}AgentView classes cover valid manifest + maxItems / maxLength / required field / additionalProperties violations. agent_view tests are pinned to local-only via SCHEMAS_LOCAL_ONLY until the property ships in a release; promote to SCHEMAS afterward by deleting the local-only constant.

WilliamBergamin

I like where this is going 💯 🚀

Left a few comments, the main one is along defining manifest.<feature>.valid.json files to use in the tests rather then using python logic to "compose" new manifests from base json files

Goal: defining manifest.<feature>.valid.json files makes it easier on reviewers and maintainers to understand the changes

WilliamBergamin · 2026-06-19T13:56:02Z

+# Each schema-content test runs against both:
+# - the hosted router schema (what IDEs and downstream consumers fetch)
+# - the local versioned schema (what the next release will ship)
+# Router-specific tests like test_no_metadata stay below, outside this matrix.
+SCHEMAS = [
+    pytest.param(get_schema, id="hosted"),
+    pytest.param(get_schema_v1, id="local"),
+]


Thats a good idea it prevents regressions 💯

I'm not the biggest fan of the naming maybe we could do

get_schema -> load_hosted_schema

get_schema_v1 -> load_local_schema

WilliamBergamin · 2026-06-19T13:57:30Z


    def test_no_metadata(self):
-        # GIVEN
+        # GIVEN — router falls back to v1 when _metadata is absent


Not sure we need this in a comment, maybe we could improve the name of the test instead 🤔

WilliamBergamin · 2026-06-19T14:08:50Z

        # THEN validation exception is raised
+
+
+class TestManifestV1AgentView:


I love having more tests 💯

But I think the tests would be cleaner if we do the following

Defined the manifest in a specific manifest json file for the test and load it directly in the test, rather then loading a base manifest and modifying it programmatically, example a manifest.agent_view..valid.json file for the test.

This aims to make the tests and changes more reviewable and maintainable

Maybe agree on a folder structure or file naming convention that allows to run tests in a "loop" similar to parametrize

Moved the TestManifestV1AgentView to its own class so that this file does not grow out of control

WilliamBergamin · 2026-06-19T14:12:03Z

@@ -10,4 +14,63 @@ def get_json(path: str) -> Dict:


 def get_schema() -> Dict:


Suggested change

def get_schema() -> Dict:

def get_hosted_schema() -> Dict:

WilliamBergamin · 2026-06-19T14:15:12Z

+MANIFEST_SCHEMA_V1_PATH = "schemas/manifest.schema.1.0.0.json"
+MANIFEST_SCHEMA_V2_PATH = "schemas/manifest.schema.2.0.0.json"


Maybe we make this

Suggested change

MANIFEST_SCHEMA_V1_PATH = "schemas/manifest.schema.1.0.0.json"

MANIFEST_SCHEMA_V2_PATH = "schemas/manifest.schema.2.0.0.json"

class MANIFEST_SCHEMA_FILENAMES(Enum):

V1 = "manifest.schema.1.0.0.json"

V2 = "manifest.schema.2.0.0.json"

WilliamBergamin · 2026-06-19T14:18:16Z

+def get_schema_v1() -> Dict:
+    """Local v1 schema — what the next release artifact will ship."""
+    return get_json(MANIFEST_SCHEMA_V1_PATH)
+
+
+def get_schema_v2() -> Dict:
+    """Local v2 schema — what the next release artifact will ship."""
+    return get_json(MANIFEST_SCHEMA_V2_PATH)


Maybe we do something like this with the enum

Suggested change

def get_schema_v1() -> Dict:

"""Local v1 schema — what the next release artifact will ship."""

return get_json(MANIFEST_SCHEMA_V1_PATH)

def get_schema_v2() -> Dict:

"""Local v2 schema — what the next release artifact will ship."""

return get_json(MANIFEST_SCHEMA_V2_PATH)

def get_local_schema(filename: MANIFEST_SCHEMA_FILENAMES) -> Dict:

path = "schemas" / filename

return get_json(path)

mwbrooks added 3 commits June 18, 2026 18:00

mwbrooks mentioned this pull request Jun 19, 2026

feat: add 'features.agent_view' property #74

Merged

4 tasks

mwbrooks changed the title ~~test: cover hosted + local schemas, add agent_view cases~~ test: cover hosted + local schemas with agent_view test cases Jun 19, 2026

mwbrooks mentioned this pull request Jun 19, 2026

feat: add support for 'features.agent_view' app manifest property slackapi/slack-cli#596

Merged

2 tasks

WilliamBergamin requested changes Jun 19, 2026

View reviewed changes

Base automatically changed from mwbrooks-manifest-agent-view to main June 20, 2026 00:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: cover hosted + local schemas with agent_view test cases#75

test: cover hosted + local schemas with agent_view test cases#75
mwbrooks wants to merge 3 commits into
mainfrom
mwbrooks-manifest-schema-tests

mwbrooks commented Jun 19, 2026 •

edited

Loading

Uh oh!

WilliamBergamin left a comment

Uh oh!

WilliamBergamin Jun 19, 2026

Uh oh!

WilliamBergamin Jun 19, 2026

Uh oh!

WilliamBergamin Jun 19, 2026

Uh oh!

WilliamBergamin Jun 19, 2026

Uh oh!

WilliamBergamin Jun 19, 2026

Uh oh!

WilliamBergamin Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		# THEN validation exception is raised


		class TestManifestV1AgentView:

		@@ -10,4 +14,63 @@ def get_json(path: str) -> Dict:


		def get_schema() -> Dict:

		MANIFEST_SCHEMA_V1_PATH = "schemas/manifest.schema.1.0.0.json"
		MANIFEST_SCHEMA_V2_PATH = "schemas/manifest.schema.2.0.0.json"

-MANIFEST_SCHEMA_V1_PATH = "schemas/manifest.schema.1.0.0.json"
-MANIFEST_SCHEMA_V2_PATH = "schemas/manifest.schema.2.0.0.json"
+class MANIFEST_SCHEMA_FILENAMES(Enum):
+    V1 = "manifest.schema.1.0.0.json"
+    V2 = "manifest.schema.2.0.0.json"

Conversation

mwbrooks commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Test plan

Follow-ups

Uh oh!

WilliamBergamin left a comment

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mwbrooks commented Jun 19, 2026 •

edited

Loading