Skip to content

fix(bundler): reject non-string manifest list members - #4091

Merged
mnriem merged 2 commits into
github:mainfrom
marcelsafin:fix/bundle-manifest-string-list-items
Aug 21, 2026
Merged

fix(bundler): reject non-string manifest list members#4091
mnriem merged 2 commits into
github:mainfrom
marcelsafin:fix/bundle-manifest-string-list-items

Conversation

@marcelsafin

Copy link
Copy Markdown
Contributor

Description

Bundle manifest fields declared as string arrays (tags, requires.tools, and requires.mcp) currently coerce every member with str(). Invalid values such as integers, booleans, and objects therefore enter the model as plausible strings instead of failing schema validation.

This validates each member and preserves valid strings without coercion.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest (6,654 passed, 177 skipped)
  • Added contract regressions for non-string tags, tools, and MCP entries
  • uvx ruff@0.15.0 check src tests clean

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

Implemented autonomously by GitHub Copilot CLI (model: GPT-5.6 Sol) under human direction; failing contract regressions were added before the fix, then the full suite and lint were run locally. Commit includes Assisted-by and Co-authored-by trailers.

Assisted-by: GitHub Copilot (model: gpt-5.6-sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 12, 2026 22:05
@marcelsafin
marcelsafin requested a review from mnriem as a code owner August 12, 2026 22:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Validates manifest string-array members instead of coercing invalid values.

Changes:

  • Rejects non-string tags, tools, and MCP entries.
  • Adds contract regression tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/specify_cli/bundler/models/manifest.py Enforces string-only list members.
tests/contract/test_manifest_schema.py Tests invalid member rejection.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/bundler/models/manifest.py
Assisted-by: GitHub Copilot (model: gpt-5.6-sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 12, 2026 22:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@mnriem
mnriem merged commit 36ff015 into github:main Aug 21, 2026
14 checks passed
@mnriem

mnriem commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

KSchlobohm pushed a commit that referenced this pull request Aug 28, 2026
_parse_tags in bundler/models/catalog.py rejected a non-list `tags`
value but silently coerced individual non-string members via
`str(t) for t in value`, letting a catalog entry like
`tags: [1, true, {}]` through as `("1", "True", "{}")` instead of
raising. This is the exact sibling of the bug just fixed for the
manifest's `_parse_str_list` in #4091 (bundler/models/manifest.py) —
same file family, same shape, just not swept across both call sites.


Claude-Session: https://claude.ai/code/session_01FW9fAYsCBCAgdKWovtSyqt

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants