Skip to content

refactor!: MCP-only toolset with account discovery, search/execute, and hardened security - #199

Open
willleeney wants to merge 42 commits into
mainfrom
chore/remove-nix-toolchain
Open

willleeney wants to merge 42 commits into
mainfrom
chore/remove-nix-toolchain

Conversation

@willleeney

@willleeney willleeney commented Sep 16, 2026

Copy link
Copy Markdown

Summary

A major (3.0.0) rewrite. The SDK is now a thin wrapper over the StackOne MCP endpoint, it works with just an API key, and it has been tested against the live API rather than only against mocks. The previous release could not list a single tool while its whole test suite passed — most of this PR is about making sure that can't happen again.

What changed for users

  • An API key is enough. With no account given, the toolset calls GET /accounts and uses every active one. Before, every call to /mcp went out without x-account-id and was rejected.
  • search() + execute() — find an action in plain English and run it, without loading the full tool catalog into the model's context.
  • Package reduced to three modulestypes.py, tools.py, toolset.py. Client-side search, OpenAPI parsing and the CrewAI integration are gone. mcp is now a core dependency; framework adapters are lazy extras.
  • Schemas reach the model unchanged. Across all 139 live tools, to_openai_function() matches the served schema byte for byte. The LangChain adapter now passes the same schema through; before, it dropped every nested field, and every call made through it failed with a 400.
  • Errors: everything derives from StackOneError. An API error's message now leads with the server's own explanation instead of generic httpx text.
  • timeout= now actually applies to MCP calls. Before, it was ignored, so timeout=2 against a host that never answered hung for over five minutes.

Security (tool arguments are attacker-controlled under prompt injection)

  • A model-supplied action_id inside execute()'s arguments could replace the action the caller had pinned.
  • Headers were filtered by a two-name denylist that " authorization" bypassed, and not filtered at all on the MCP path. They now go through an allowlist built from the served schema. Zero live actions declare a header, and the server was found to ignore the envelope's headers object.
  • Download filenames came from an attacker-controllable header unchecked, so ../../.ssh/authorized_keys was returned as-is. They are now reduced to a safe basename.

Testing and CI

  • Mocks now reject what the real API rejects: an unscoped /mcp or /actions/rpc request gets a 400, an unknown account a 404. The old mock filled in a missing account as 'default', which is how the original bug stayed invisible.
  • Every fix was mutation-tested. Each was reverted in turn and the suite re-run; all 13 now fail the suite if undone. Three of them initially went unnoticed and got new tests.
  • Publishing to PyPI is gated. It now runs in a pypi environment and re-runs the tests first; before, a red main could still ship.
  • nix removed; the toolchain is uv, ruff, ty and make. CI type-checks examples/, and a new ci-ok job aggregates the required checks.

Test plan

  • make test: 241 passed
  • make format: ruff and ty clean, examples/ included
  • make validate: conformance with strict schema (now the default), SDK smoke, Pydantic AI 1.x and 2.x. The ADK smoke skips because the plugin pins stackone-ai>=3.0.0, which isn't on PyPI yet.
  • Live against the eu1 API: fetch_accounts, fetch_tools, search, execute, pagination, and write actions (create → read → update → delete, all cleaned up)
  • Every README code block run live
  • Every example run live, 3 runs each: 18/18 passed
  • Conformance harness verified by reverting fixes in a scratch copy of the SDK: both prompt-injection fixes, routing and ranking are all caught

Needs repo-settings changes

  • Add a required reviewer to the new pypi environment
  • Make ci-ok the single required status check
  • The release workflow pushes its uv.lock commit with GITHUB_TOKEN, which doesn't trigger CI. It needs a personal access token or GitHub App token.
  • Set the CONFORMANCE_REPO_TOKEN secret

Known gaps, not in this PR

  • There's no async API, so calling the SDK from async code blocks the event loop
  • Each call opens a new HTTP connection instead of reusing one
  • The Node SDK still has no account discovery or search/execute; the conformance harness reports both as failures

Pairs with StackOneHQ/sdk-conformance#2.

🤖 Generated with Claude Code

willleeney and others added 24 commits September 14, 2026 14:52
The mock server was bun-specific in two ways beyond its shebang: it read
Bun.argv and relied on Bun's default-export server convention, which Node
ignores entirely. Replace both with process.argv and @hono/node-server.

Add a root package.json pinning the mock's dependencies to the exact
versions the vendored submodule resolves (sdk 1.24.3, zod 4.1.13,
hono 4.10.7, @hono/mcp 0.1.5). Caret ranges resolve sdk to 1.30.0, which
rejects the raw inputSchema objects the vendor mock passes.

These 38 tests previously skipped because the submodule was never
initialised; they now run and pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
just was supplied by the nix flake and by CI's setup-nix action, both of
which are being removed. make is preinstalled on macOS and Linux, so the
task runner no longer needs a package manager to bootstrap.

Positional arguments become variables: `just install --all-extras` is now
`make install EXTRAS="--all-extras"`, and `just run-example foo.py` is
`make run-example FILE=foo.py`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The flake provided four things: treefmt formatting, pre-commit hooks, the
devShell toolchain, and agent skills delivery. Replace each:

- Formatting: ruff is already a dev dependency, so `make lint` and
  `make format` call it directly. nixfmt is no longer needed (no .nix
  files remain) and oxfmt is dropped, so non-Python files are now
  unformatted.
- Hooks: removed. Lint, type check and tests run in CI on every push.
- CI: setup-nix replaced with astral-sh/setup-uv, plus pnpm and Node for
  the MCP mock server. gitleaks now uses its official action rather than
  installing the binary through nix.
- Skills: dropped. They were gitignored and only materialised inside
  `nix develop`, so they were never visible outside a nix shell.

Also drops the nix-flake and nix-flake-update workflows, the nix-workflow
rule, and the stale .pre-commit-config.yaml ignore entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AGENTS.md is the cross-tool standard that Cursor and other agents read
natively; Claude Code reads CLAUDE.md. Symlinking one to the other gives
both a single source of truth with no duplicated content, matching the
pattern already used in stackone-ai-node.

Also removes the two Available Skills tables (the section was duplicated,
listing different skills each time) and the nix-workflow row now that
skills and the nix rule are gone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fold the seven .claude/rules/ files into CLAUDE.md and delete .claude/
and .cursor/ entirely. AGENTS.md already symlinks to CLAUDE.md, so a
single file now serves every agent with no symlink tree to maintain.

Condensed rather than concatenated: a verbatim merge came to ~505 lines,
against the 200-line target above which adherence drops. Dropped the
illustrative good/bad code blocks and the sample UV script, keeping every
actual rule. Result is 192 lines.

Two corrections made while merging, both previously contradicted by the
repo itself:

- Line length was documented as 88 ("ruff default"); pyproject.toml sets
  110.
- The pre-commit hooks section described hooks that no longer exist.

Trade-off: the four path-scoped rules (examples, scripts, pyproject, *.py)
previously loaded only when touching matching files. They now load every
session.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
scripts/ held only benchmark_search.py, a manual latency benchmark that
required STACKONE_API_KEY and STACKONE_ACCOUNT_ID to run. Nothing
referenced it: no CI job, no test, no Makefile target.

Also drops the now-dead scripts/ ruff per-file-ignore and the Scripts
section of CLAUDE.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.envrc contained `use flake`, the nix-direnv hook that activates the nix
devShell on cd. flake.nix was removed in 3bf1a27, so the file errors for
anyone with direnv installed. It was tracked, so it affected everyone.

uv manages the virtualenv directly, so there is nothing left for direnv
to load.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Python 3.11 minimum. The CI matrix claimed to test 3.10-3.13 but the old
setup-nix action ignored matrix.python-version entirely, so every leg ran
on the same devShell interpreter. setup-uv honours it, which exposed that
the examples extra cannot install on 3.10 (onnxruntime 1.24.3 ships no
cp310 wheels).

CI fixes:
- gitleaks: the GitHub Action requires a paid licence for org-owned repos
  and would have exited 1 on every run. Install the free CLI and call
  `make gitleaks`, which also revives a Makefile target nothing installed.
- Scope pages/id-token write permissions to the coverage jobs instead of
  granting them workflow-wide, so the job running PR code can no longer
  mint an OIDC token.

Security: _build_action_headers stripped only exact-case "Authorization",
but header names are case-insensitive and tool arguments are
model-controlled, so headers_authorization passed straight through into
the RPC envelope. x-account-id was likewise overridable, allowing a
prompt-injected call to retarget another tenant. Both are now reserved and
filtered case-insensitively, with the account id applied after merging.
Adds regression tests for the case variants and the account override.

Also removes the dead epilogue in StackOneTool.execute: two discarded
datetime.now() calls and a metadata dict built in a finally block and
thrown away, left over from the removed implicit-feedback path. Dropping
TYPE_CHECKING means to_pydantic_ai_tool returns Any, since pydantic-ai is
optional and must not be imported at module level.

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

Reduce stackone_ai to three modules built around one property: the toolset is
the served catalog. The schema listed to a model is the schema the MCP server
sent, and the request sent to /actions/rpc matches it.

- types.py: ToolParameters, ExecuteConfig, ParameterLocation, error hierarchy,
  shared aliases, DEFAULT_BASE_URL
- tools.py: StackOneTool, Tools, StackOneRpcTool, the MCP listing client
- toolset.py: StackOneToolSet

BREAKING: to_openai_function no longer filters the schema. It copied only
type/description/enum, silently discarding format, pattern, default,
minimum/maximum, oneOf/anyOf and nested required, so a model could not generate
valid arguments for any constrained field. The served schema now passes through
verbatim; only the SDK's internal `nullable` marker is stripped, becoming the
JSON Schema `required` list. This is what the conformance suite's
--strict-schema gate checks.

BREAKING: client-side search is removed — semantic_search, local_search, the
BM25/TF-IDF index, SearchConfig/SearchMode, the tool_search/tool_execute meta
tools and mode="search_and_execute". It is absent from the conformance contract
(the mock serves no /actions/search) and the contract docs flag client-side
search as something that should move server-side. Frees bm25s and numpy.

BREAKING: removes stackone_ai.integrations. LangGraph's own ToolNode and
bind_tools cover it; examples/langgraph_integration.py already used those
directly.

Replaces the vendored stackone-ai-node submodule with the one file the tests
actually used: mocks/mcp-server.ts, 275 lines with npm-only imports. Drops 764K
and 102 files, and the mock fixture now fails rather than skips when Node
dependencies are missing — a silent skip had let all 19 integration tests vanish
while CI stayed green.

Docs corrected against the code: stackone_ai/oas/, tests/snapshots/,
_process_response(), include_tools=, strict ty config, the context-window
warning, basic_usage/ and integrations/ example directories, and README's
configure_implicit_feedback section all described things that do not exist.

Also drops typing-extensions and pytest-snapshot (no references), the bin/**.py
ruff ignore (that directory has never existed), the T201/T203 codes (flake8-print
rules absent from `select`), and the redundant asyncio marker.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ke tests

One entry point for everything that consumes the SDK: example integrity, the
sdk-conformance wire contract (with --strict-schema), and the SDK / Pydantic AI /
Google ADK smoke suites.

Skips are never counted as passes. A section that cannot run says why and is
listed in the summary, so an absent sibling repo or missing credentials can never
read as green.

Two traps found while writing it, both the silent-green failure this repo keeps
hitting:

- Piping `ty` into grep is scored by `set -o pipefail`, which returns ty's
  non-zero exit even when grep matched — a successful detection read as "no
  match" and the check passed. Output is now captured before grepping.
- Grepping for `stackone_ai` matched ty's echoed source context, so every
  example's own import tripped it and a clean tree reported a failure. It now
  matches the diagnostic line itself.

Verified in both directions: injecting a reference to a removed symbol fails the
check, and a clean tree passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a conformance job running `pnpm test:python -- --strict-schema`, which
fails on any schema keyword the SDK drops between what the server serves and
what the model is shown. Locally this is the `make`-adjacent equivalent of
scripts/validate.sh conformance.

StackOneHQ/sdk-conformance is private, so the default GITHUB_TOKEN cannot check
it out. The job requires a CONFORMANCE_REPO_TOKEN secret (PAT or App token with
read access) and fails with an explicit message when it is absent, rather than
skipping — a contract check that skips reads as green while enforcing nothing.

Also adds mcp to the dev dependency group. fetch_tools() hard-requires the mcp
client and no test guards its import, so a bare `uv sync` produced a venv where
the suite errored instead of skipping. Found by reproducing the job's layout from
a clean checkout: it failed with "MCP dependencies are required for fetch_tools"
where the local run passed only because .venv had been synced with --all-extras.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The SDK told users to `pip install "stackone-ai[mcp]"` in its own ImportError,
and the README and pydantic-ai example did the same, while CLAUDE.md states uv
is used for all dependency management. Switched to `uv add`.

Also strengthens scripts/validate.sh. Running an example live and checking its
exit code proves almost nothing: every integration example filters on
`workday_*`, and against a linked account with no Workday connector they load
zero tools, call the model anyway, and exit 0. Verified against the live API —
all five reported "Loaded 0 tools" while the validator called it a pass. The
live-run check now fails when an example loads nothing and prints the line that
gave it away.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`make` with no arguments ran `install` because it was the first target. It now
prints the target list instead, which is both safer and more discoverable.

Help is generated from the `##` comments above each target, so it cannot drift
from the Makefile the way a hand-maintained list would.

Also surfaces three things that have caught people out: `publish` pushes to PyPI
for real, `gitleaks` needs a binary nothing installs locally, and
`test-examples` only imports each example — the `__main__` guard means no
example body ever runs, so it goes green without exercising anything.

Adds a `validate` target wrapping scripts/validate.sh.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`lint` and `format` were the same ruff invocation in opposite modes, which read
as two concerns rather than check-vs-fix. CI is the only caller that needs the
read-only mode, and it now runs ruff, ty and pytest directly instead of going
through make — so it verifies what was committed and cannot mutate the tree to
make itself pass.

`format` now fixes lint, formats, and type checks: one command before
committing.

Removes `lint` (CI runs the ruff commands itself), `ty` (folded into `format`),
`test-tools` (`test` already collects tests/ — verified, both give 174) and
`run-example` (`uv run examples/<file>` directly).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Publishing is release-please's job: merge to main, merge the release PR, and
release.yaml runs uv build + uv publish with PYPI_API_TOKEN. A local target
meant a developer machine could push to PyPI out of band, bypassing the version
bump, changelog and tag.

release.yaml already invokes uv directly, so nothing depended on the target.
`build` stays for checking the artifact locally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The target invoked a bare `gitleaks` binary that nothing installed once the nix
shell went, so it failed with command-not-found on every machine. CI installs
the CLI and runs the same command itself, with fetch-depth 0 so it scans the
full history rather than a shallow slice — which a local run would not have done
anyway.

.gitleaks.toml stays; it is the config CI passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng the venv

`uv sync` makes the environment match the requested set exactly, so a bare
`uv sync` uninstalls every optional dependency. Measured: `make install` took
the venv from 207 packages to 56, removing openai, crewai, langgraph and
pydantic-ai — leaving the examples unable to import and the mcp-backed tests
unable to run.

The obvious command should not break the environment. EXTRAS now defaults to
--all-extras; EXTRAS="" still gets the minimal set, and the help says plainly
that it removes things.

`uv run` was not the culprit — the package count is unchanged across `make test`.

CI is unaffected: it runs `uv sync --all-extras --locked` directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dependency tiers were inverted. Every user installed langchain-core — and
transitively langsmith — for one of four adapters, while nobody could fetch a
tool out of the box because the MCP client was optional.

Measured on a clean venv:

  pydantic + httpx                 11 packages
  + langchain-core  (old core)     33 packages, and fetch_tools() still fails
  + mcp             (new core)     29 packages, and everything works

BREAKING: `stackone-ai[mcp]` no longer exists — mcp is a core dependency, since
fetch_tools() is the only route to a tool and it talks MCP. `to_langchain()` now
requires `stackone-ai[langchain]` and raises a clear ImportError without it,
mirroring how to_pydantic_ai_tool() already behaved.

Both misplacements were historical: mcp was genuinely optional when the SDK
shipped bundled OpenAPI specs and get_tools() worked offline, but d50d5fb
deleted that path and the extras never followed.

Verified on a bare install (30 packages, no langchain_core, no langsmith):
fetch_tools returns 22 live tools, to_openai works, to_langchain raises the
install hint. Conformance still passes with --strict-schema.

CLAUDE.md now records the rule so it does not drift back: core is what the SDK
needs to function, extras are per-framework adapters with lazy imports.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two tiers: core and extras. The PEP 735 dev group was a third mechanism for
declaring dependencies, and one tier fewer is easier to reason about than the
distinction it bought.

Also drops the `stackone-ai` self-reference from dev, which only ever meant
"install the project", something uv does anyway.

Trade-off, stated plainly: a dev group is never published, an extra is. The
wheel now advertises `Provides-Extra: dev`, so `uv add 'stackone-ai[dev]'`
installs pytest, ruff and ty for a consumer. That is the cost of the simpler
model, not an oversight.

`make install` defaults to --all-extras, so the tooling still arrives with one
command, and CI's `uv sync --all-extras --locked` is unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`make install` now syncs core only and `make extras` syncs everything, so
`make install extras` reads as the sentence it is. Replaces the EXTRAS variable,
which required knowing that a bare `make install` would silently uninstall
openai, crewai and the test tooling.

Measured: `make install` gives 30 packages (mcp yes, pytest no, openai no);
`make install extras` gives 207. The help says outright that `install` removes
anything outside the core set, since that is what `uv sync` does and it is not
obvious.

Also fixes the help parser: it ran `sed 's/:.*//'` over every line, so any
description containing a colon was truncated — "Install everything: adapters,
examples and dev tooling" rendered as "Install everything". Targets and
descriptions are now parsed separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`make install` and `make install extras=1`, one target instead of two. Any
non-empty value works, so extras=1 and extras=True both do.

Reverts the separate `extras` target added in f2acc61 — it was a second command
in the list for something that is a property of how you install, not a different
thing to run.

  make install            30 packages, no pytest
  make install extras=1   207 packages, pytest present

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing called it. CI runs the command inline, and the three report formats
exist for CI alone: coverage/coverage.json feeds the badge action and
coverage/html is uploaded to Pages. Locally `uv run pytest --cov` is the useful
form and needs no target.

Also trims coverage exclude_lines to the one pattern that still matches
anything. `if TYPE_CHECKING:` went when the adapter imports became lazy, and
there is no __repr__ or NotImplementedError in the package — verified 0 hits
each.

Coverage is unchanged at 96%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The live example run was skipping for want of credentials that were sitting in
.env two directories up from the check. validate.sh now sources .env when
present and prints which path it used; when absent it says so, so a skip is
always attributable rather than mysterious.

Only that one check needs credentials — conformance and the smoke suites drive a
mock API on 127.0.0.1 with a dummy key (conformance-key / smoke-key), which is
what lets the oracle assert that the schema an SDK listed matches the request it
sent.

With credentials loaded the examples check now fails rather than skipping, which
is correct: all five load 0 tools because they filter workday_* against accounts
that have no Workday connector.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
validate now runs only against the sdk-conformance mock on 127.0.0.1 with a
dummy key, so it is deterministic, needs no credentials, and cannot spend real
API calls or OpenAI tokens.

Removes the live example run and the .env loading added a commit earlier. The
live run was the one section that called StackOne, and it was also the least
informative: it drove each example against whatever account happened to be
linked, so its result depended on the connectors on that key rather than on
anything in this repo.

The static example checks stay — they catch a removed or renamed SDK symbol,
which is the regression the restructure could actually cause.

  6 passed, 0 failed, 0 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 16, 2026 12:04
Comment thread .github/workflows/ci.yaml
Comment on lines +29 to +31
run: |
curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \
| tar -xz -C /usr/local/bin gitleaks

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Binary, code or archive is pulled from a remote source without integrity verification - medium severity
A GitHub Actions Workflow was built using an artifact from a remote source without any integrity verification. If the remote artifact were silently replaced with a malicious version (for example, through a supply chain attack), the integrity and confidentiality of the environment in which the container is deployed could be compromised.

Show fix

Remediation: Validate the artifact against a trusted SHA-512 checksum in the CI/CD pipeline using sha512sum in check mode. Store the expected checksum in a file (e.g., artifact.sha512), then verify it with: sha512sum -c artifact.sha512. Enable strict error handling (for example, set -e in shell scripts) so the pipeline fails if verification fails or outputs errors.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Comment thread uv.lock

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CVE-2025-71176 in pytest - medium severity
pytest through 9.0.2 on UNIX relies on directories with the /tmp/pytest-of-{user} name pattern, which allows local users to cause a denial of service or possibly gain privileges.

Details

Remediation Aikido suggests bumping this package to version 9.0.3 to resolve this issue

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

10 issues found across 70 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/mocks/serve.ts">

<violation number="1" location="tests/mocks/serve.ts:20">
P3: The header comment still says the file "Imports createMcpApp from stackone-ai-node vendor submodule," but this refactor removed the vendor submodule (no vendor/ dir remains) and the import now resolves to ./mcp-server in the same directory. Update the comment so it doesn't mislead readers about where the mock app comes from.</violation>
</file>

<file name="stackone_ai/types.py">

<violation number="1" location="stackone_ai/types.py:78">
P2: When `ExecuteConfig` receives a non-string method, `validate_method` raises `AttributeError` before Pydantic can validate the `str` field. Reject non-string values explicitly so invalid configuration consistently raises `ValidationError`.</violation>
</file>

<file name="tests/mocks/mcp-server.ts">

<violation number="1" location="tests/mocks/mcp-server.ts:35">
P3: The docstring example references a `createMcpHandler` export and a `./mocks/node` module that do not exist (the file exports `createMcpApp`, and only `mcp-server.ts`/`serve.ts` exist under tests/mocks). Update the example to use `createMcpApp` and the real import path so it doesn't mislead future test authors.</violation>

<violation number="2" location="tests/mocks/mcp-server.ts:81">
P2: When an MCP client calls any registered tool through `/mcp`, `params` is undefined because the SDK passes arguments directly. Use the direct arguments object for `structuredContent` so the mock can serve tool calls.</violation>
</file>

<file name="tests/conftest.py">

<violation number="1" location="tests/conftest.py:69">
P3: The check verifies only that `node_modules` exists, but the mock server actually requires `pnpm` on PATH: `serve.ts` runs through the shebang `#!/usr/bin/env -S pnpm exec tsx`. When `node_modules` was installed by another tool and `pnpm` is missing, the check passes, then the fixture burns the 30 s wait and raises a generic "failed to start" RuntimeError instead of the actionable failure this change is meant to provide. Check for the real prerequisite (`pnpm exec`/`tsx` availability) so the fail-fast diagnostic fires whenever the server cannot launch.</violation>
</file>

<file name="stackone_ai/tools.py">

<violation number="1" location="stackone_ai/tools.py:165">
P2: When a served nested schema uses its own `nullable` keyword, `_strip_internal_keys()` removes it even though normalization adds the SDK marker only to top-level properties. Strip only the SDK-added marker, or preserve nested `nullable` values.</violation>

<violation number="2" location="stackone_ai/tools.py:374">
P2: When a served input schema has root constraints such as `additionalProperties`, `to_openai_function()` drops them because it rebuilds `parameters` from only `type` and `properties`. Preserve the root schema, replacing only `required`, so strict-schema consumers receive the catalog unchanged.</violation>
</file>

<file name="scripts/validate.sh">

<violation number="1" location="scripts/validate.sh:43">
P2: When `make validate` follows the documented core-only `make install`, the examples section reports missing optional framework packages as failures instead of the SKIP behavior promised by this script. Detect missing example dependencies and call `skip`, matching `test_examples.py`.</violation>
</file>

<file name=".github/workflows/ci.yaml">

<violation number="1" location=".github/workflows/ci.yaml:31">
P1: The gitleaks job cannot install its binary because the unprivileged runner cannot write to `/usr/local/bin`. Extract into a writable bin directory or run the tar extraction with `sudo`.</violation>
</file>

<file name="CLAUDE.md">

<violation number="1" location="CLAUDE.md:183">
P2: The documented pass-through contract is false for root schema keywords such as `additionalProperties`. Preserve the served root schema when building OpenAI parameters, or narrow this statement until the converter retains those constraints.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread .github/workflows/ci.yaml
Comment thread .github/workflows/ci.yaml
- name: Install gitleaks
run: |
curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \
| tar -xz -C /usr/local/bin gitleaks

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: The gitleaks job cannot install its binary because the unprivileged runner cannot write to /usr/local/bin. Extract into a writable bin directory or run the tar extraction with sudo.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 31:

<comment>The gitleaks job cannot install its binary because the unprivileged runner cannot write to `/usr/local/bin`. Extract into a writable bin directory or run the tar extraction with `sudo`.</comment>

<file context>
@@ -24,11 +22,15 @@ jobs:
+      - name: Install gitleaks
+        run: |
+          curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \
+            | tar -xz -C /usr/local/bin gitleaks
+        env:
+          GITLEAKS_VERSION: 8.29.0
</file context>
Suggested change
| tar -xz -C /usr/local/bin gitleaks
+ | sudo tar -xz -C /usr/local/bin gitleaks

Comment thread README.md
Comment thread stackone_ai/types.py

def validate_method(v: str) -> str:
"""Validate HTTP method is uppercase and supported"""
method = v.upper()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When ExecuteConfig receives a non-string method, validate_method raises AttributeError before Pydantic can validate the str field. Reject non-string values explicitly so invalid configuration consistently raises ValidationError.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/types.py, line 78:

<comment>When `ExecuteConfig` receives a non-string method, `validate_method` raises `AttributeError` before Pydantic can validate the `str` field. Reject non-string values explicitly so invalid configuration consistently raises `ValidationError`.</comment>

<file context>
@@ -0,0 +1,149 @@
+
+def validate_method(v: str) -> str:
+    """Validate HTTP method is uppercase and supported"""
+    method = v.upper()
+    if method not in {"GET", "POST", "PUT", "DELETE", "PATCH"}:
+        raise ValueError(f"Unsupported HTTP method: {method}")
</file context>
Suggested change
method = v.upper()
if not isinstance(v, str):
raise ValueError(f"Unsupported HTTP method: {v}")
method = v.upper()

Comment thread stackone_ai/tools.py
return {
key: _strip_internal_keys(value)
for key, value in schema.items()
if key not in _INTERNAL_SCHEMA_KEYS

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When a served nested schema uses its own nullable keyword, _strip_internal_keys() removes it even though normalization adds the SDK marker only to top-level properties. Strip only the SDK-added marker, or preserve nested nullable values.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/tools.py, line 165:

<comment>When a served nested schema uses its own `nullable` keyword, `_strip_internal_keys()` removes it even though normalization adds the SDK marker only to top-level properties. Strip only the SDK-added marker, or preserve nested `nullable` values.</comment>

<file context>
@@ -0,0 +1,666 @@
+        return {
+            key: _strip_internal_keys(value)
+            for key, value in schema.items()
+            if key not in _INTERNAL_SCHEMA_KEYS
+        }
+    if isinstance(schema, list):
</file context>

Comment thread CLAUDE.md Outdated
Comment thread tests/mocks/serve.ts
exampleBamboohrTools,
mixedProviderTools,
} from "../../vendor/stackone-ai-node/mocks/mcp-server";
} from "./mcp-server";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The header comment still says the file "Imports createMcpApp from stackone-ai-node vendor submodule," but this refactor removed the vendor submodule (no vendor/ dir remains) and the import now resolves to ./mcp-server in the same directory. Update the comment so it doesn't mislead readers about where the mock app comes from.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/mocks/serve.ts, line 20:

<comment>The header comment still says the file "Imports createMcpApp from stackone-ai-node vendor submodule," but this refactor removed the vendor submodule (no vendor/ dir remains) and the import now resolves to ./mcp-server in the same directory. Update the comment so it doesn't mislead readers about where the mock app comes from.</comment>

<file context>
@@ -16,9 +17,9 @@ import {
   exampleBamboohrTools,
   mixedProviderTools,
-} from "../../vendor/stackone-ai-node/mocks/mcp-server";
+} from "./mcp-server";
 
-const port = parseInt(process.env.PORT || Bun.argv[2] || "8787", 10);
</file context>

Comment thread tests/mocks/mcp-server.ts
* @example
* ```ts
* import { server } from './mocks/node';
* import { createMcpHandler, defaultMcpTools, accountMcpTools } from './mocks/mcp-server';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The docstring example references a createMcpHandler export and a ./mocks/node module that do not exist (the file exports createMcpApp, and only mcp-server.ts/serve.ts exist under tests/mocks). Update the example to use createMcpApp and the real import path so it doesn't mislead future test authors.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/mocks/mcp-server.ts, line 35:

<comment>The docstring example references a `createMcpHandler` export and a `./mocks/node` module that do not exist (the file exports `createMcpApp`, and only `mcp-server.ts`/`serve.ts` exist under tests/mocks). Update the example to use `createMcpApp` and the real import path so it doesn't mislead future test authors.</comment>

<file context>
@@ -0,0 +1,275 @@
+ * @example
+ * ```ts
+ * import { server } from './mocks/node';
+ * import { createMcpHandler, defaultMcpTools, accountMcpTools } from './mocks/mcp-server';
+ *
+ * // In your test setup
</file context>

Comment thread tests/conftest.py
if not (vendor_dir / "package.json").exists():
pytest.skip("stackone-ai-node submodule not initialized. Run 'git submodule update --init'")
if not (project_root / "node_modules").is_dir():
pytest.fail("Node dependencies missing for the MCP mock server. Run 'pnpm install'.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The check verifies only that node_modules exists, but the mock server actually requires pnpm on PATH: serve.ts runs through the shebang #!/usr/bin/env -S pnpm exec tsx. When node_modules was installed by another tool and pnpm is missing, the check passes, then the fixture burns the 30 s wait and raises a generic "failed to start" RuntimeError instead of the actionable failure this change is meant to provide. Check for the real prerequisite (pnpm exec/tsx availability) so the fail-fast diagnostic fires whenever the server cannot launch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/conftest.py, line 69:

<comment>The check verifies only that `node_modules` exists, but the mock server actually requires `pnpm` on PATH: `serve.ts` runs through the shebang `#!/usr/bin/env -S pnpm exec tsx`. When `node_modules` was installed by another tool and `pnpm` is missing, the check passes, then the fixture burns the 30 s wait and raises a generic "failed to start" RuntimeError instead of the actionable failure this change is meant to provide. Check for the real prerequisite (`pnpm exec`/`tsx` availability) so the fail-fast diagnostic fires whenever the server cannot launch.</comment>

<file context>
@@ -58,13 +57,16 @@ def test_mcp_integration(mcp_mock_server):
-    if not (vendor_dir / "package.json").exists():
-        pytest.skip("stackone-ai-node submodule not initialized. Run 'git submodule update --init'")
+    if not (project_root / "node_modules").is_dir():
+        pytest.fail("Node dependencies missing for the MCP mock server. Run 'pnpm install'.")
 
     # find port
</file context>

Comment thread stackone_ai/tools.py Outdated

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.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Refactors the SDK around an MCP-served catalog, removes client-side search, and replaces the Nix toolchain with uv/make and conformance CI.

Changes:

  • Consolidates runtime code into types.py, tools.py, and toolset.py.
  • Adds flat-prefixed RPC handling, schema tests, and case-insensitive header protections.
  • Updates dependencies, examples, mocks, CI, and validation tooling.
File summaries
File Description
tests/test_toolset.py Updated as part of this pull request.
tests/test_tool_calling.py Updated as part of this pull request.
tests/test_tfidf_index.py Updated as part of this pull request.
tests/test_models.py Updated as part of this pull request.
tests/test_local_search.py Updated as part of this pull request.
tests/test_integrations_pydantic_ai.py Updated as part of this pull request.
tests/test_integrations_langgraph.py Updated as part of this pull request.
tests/test_fetch_tools.py Updated as part of this pull request.
tests/test_feedback.py Updated as part of this pull request.
tests/test_agent_tools.py Updated as part of this pull request.
tests/mocks/serve.ts Updated as part of this pull request.
tests/mocks/mcp-server.ts Updated as part of this pull request.
tests/conftest.py Updated as part of this pull request.
stackone_ai/utils/tfidf_index.py Updated as part of this pull request.
stackone_ai/utils/normalize.py Updated as part of this pull request.
stackone_ai/utils/init.py Updated as part of this pull request.
stackone_ai/types.py Updated as part of this pull request.
stackone_ai/semantic_search.py Updated as part of this pull request.
stackone_ai/models.py Updated as part of this pull request.
stackone_ai/local_search.py Updated as part of this pull request.
stackone_ai/integrations/langgraph.py Updated as part of this pull request.
stackone_ai/integrations/init.py Updated as part of this pull request.
stackone_ai/feedback/tool.py Updated as part of this pull request.
stackone_ai/feedback/init.py Updated as part of this pull request.
stackone_ai/constants.py Updated as part of this pull request.
stackone_ai/init.py Updated as part of this pull request.
scripts/validate.sh Updated as part of this pull request.
scripts/benchmark_search.py Updated as part of this pull request.
README.md Updated as part of this pull request.
pyproject.toml Updated as part of this pull request.
package.json Updated as part of this pull request.
Makefile Updated as part of this pull request.
justfile Updated as part of this pull request.
flake.nix Updated as part of this pull request.
flake.lock Updated as part of this pull request.
examples/test_examples.py Updated as part of this pull request.
examples/search_tools.py Updated as part of this pull request.
examples/pydantic_ai_integration.py Updated as part of this pull request.
CLAUDE.md Updated as part of this pull request.
.mcp.json Updated as part of this pull request.
.gitmodules Updated as part of this pull request.
.gitignore Updated as part of this pull request.
.github/workflows/release.yaml Updated as part of this pull request.
.github/workflows/nix-flake.yaml Updated as part of this pull request.
.github/workflows/nix-flake-update.yaml Updated as part of this pull request.
.github/workflows/ci.yaml Updated as part of this pull request.
.github/actions/setup-nix/action.yaml Updated as part of this pull request.
.envrc Updated as part of this pull request.
.claude/rules/uv-scripts.md Updated as part of this pull request.
.claude/rules/release-please-standards.md Updated as part of this pull request.
.claude/rules/package-installation.md Updated as part of this pull request.
.claude/rules/no-relative-imports.md Updated as part of this pull request.
.claude/rules/nix-workflow.md Updated as part of this pull request.
.claude/rules/git-workflow.md Updated as part of this pull request.
.claude/rules/examples-standards.md Updated as part of this pull request.
.claude/rules/development-workflow.md Updated as part of this pull request.
Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (2)

tests/mocks/mcp-server.ts:85

  • registerTool invokes its callback with the tool-arguments object as the first parameter, not { params: ... }. With this destructuring, a call containing {foo: ...} leaves params undefined and the mock returns empty structuredContent, so execution through this server does not reflect the submitted arguments.
    tests/mocks/serve.ts:20
  • The new local ./mcp-server import leaves the header comment claiming this file imports from the stackone-ai-node vendor submodule. Update that comment to match the source actually used by the test server, otherwise the mock setup documentation is misleading.
  • Files reviewed: 65/70 changed files
  • Comments generated: 9
  • Review effort level: Lite

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

Comment thread .github/workflows/ci.yaml
- name: Install gitleaks
run: |
curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \
| tar -xz -C /usr/local/bin gitleaks
Comment thread stackone_ai/toolset.py Outdated
Comment on lines 142 to 144
@@ -1260,11 +143,11 @@ def fetch_tools(
if cached is not None:
return cached
Comment thread stackone_ai/toolset.py
Comment on lines 228 to 232
schema = tool_def.input_schema or {}
parameters = ToolParameters(
type=str(schema.get("type") or "object"),
properties=self._normalize_schema_properties(schema),
)
Comment thread examples/test_examples.py Outdated
Comment on lines 25 to 29
@@ -29,7 +29,6 @@ def get_example_files() -> list[str]:
"crewai_integration.py": ["crewai", "mcp"],
Comment thread pyproject.toml Outdated
[project.optional-dependencies]
mcp = ["mcp>=1.3.0,<2.0.0"]
langchain = ["langchain-core>=0.1.0"]
pydantic-ai = ["pydantic-ai-slim>=1.83.0,<2.0.0"]
Comment thread stackone_ai/tools.py Outdated
Comment on lines +413 to +417
if isinstance(details, dict):
type_str = details.get("type", "string")
is_nullable = details.get("nullable", False)
if type_str == "number":
python_type = float
Comment thread CLAUDE.md Outdated
Comment thread README.md Outdated
Comment on lines 215 to 216
# For agent-driven discovery, enable search on the constructor:
# toolset = StackOneToolSet(search={"method": "auto"})
Comment thread stackone_ai/tools.py
Comment on lines +119 to +122
except ImportError as exc: # pragma: no cover - depends on optional extra
raise ToolsetConfigError(
"MCP dependencies are required for fetch_tools. Install with 'uv add \"stackone-ai[mcp]\"'."
) from exc
It asserted nothing. `test_run_example` loaded each example as module name
"example", so every `if __name__ == "__main__":` guard stayed shut and no
example body ran; the `if spec and spec.loader:` guard meant a falsy spec passed
silently. All it proved was that the file parsed and its top-level imports
resolved.

scripts/validate.sh already does that and more — it also runs ty over the
function bodies, which catches a removed or renamed SDK symbol that importing
cannot see.

Removes the `test-examples` make target with it, and sets testpaths = ["tests"]
so pytest stops globbing examples/ for test files. Test count drops 174 → 167:
the seven that go were the vacuous ones.

Also teaches validate.sh to tell "this consumer is broken" apart from "this
consumer pins a version that is not on PyPI yet" — the ADK plugin now requires
stackone-ai 3.x, which cannot resolve until the release publishes, and that says
nothing about this SDK.

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/validate.sh">

<violation number="1" location="scripts/validate.sh:124">
P2: Any smoke failure whose combined output contains this text is reported as a skip, even when the text came from a later runtime or assertion failure. Have `run_smoke.sh` return a distinct status for an unpublished SDK pin, or otherwise validate that the resolver failed before skipping, so unrelated consumer failures remain failures.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/validate.sh
# A consumer pinned to an unpublished major cannot be installed, which says
# nothing about this SDK. Report it as not-run rather than as a failure here;
# it resolves itself once that version is on PyPI.
if grep -q 'only the following versions of stackone-ai are available' <<<"$out"; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Any smoke failure whose combined output contains this text is reported as a skip, even when the text came from a later runtime or assertion failure. Have run_smoke.sh return a distinct status for an unpublished SDK pin, or otherwise validate that the resolver failed before skipping, so unrelated consumer failures remain failures.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/validate.sh, line 124:

<comment>Any smoke failure whose combined output contains this text is reported as a skip, even when the text came from a later runtime or assertion failure. Have `run_smoke.sh` return a distinct status for an unpublished SDK pin, or otherwise validate that the resolver failed before skipping, so unrelated consumer failures remain failures.</comment>

<file context>
@@ -110,11 +109,23 @@ smoke() {
+    # A consumer pinned to an unpublished major cannot be installed, which says
+    # nothing about this SDK. Report it as not-run rather than as a failure here;
+    # it resolves itself once that version is on PyPI.
+    if grep -q 'only the following versions of stackone-ai are available' <<<"$out"; then
+        skip "$label" "consumer pins a stackone-ai version that is not on PyPI yet"
+        return
</file context>

willleeney and others added 4 commits September 16, 2026 17:06
The SDK could not work from an API key alone and could not say why when it
failed. Found by pointing it at a real key for the first time; none of it was
visible to the test suite, which is fixed in the same commit.

Behaviour:
- fetch_tools() discovers accounts from GET /accounts when none is supplied.
  /mcp requires x-account-id in every mode, so a bare api_key never worked.
  Accounts not in `active` are skipped, and a key with none usable now says
  which providers are broken instead of failing opaquely.
- tool_mode="search_execute" lists two meta tools per connector rather than one
  tool per action — 3 tools instead of 139 on a single linked account.
- Those meta tools have no /actions/rpc action behind them and 404 there, so
  StackOneMcpTool executes them over MCP tools/call.
- A tools/call failure arrives as an ordinary response with isError set. It was
  returned to the caller as a success; it now raises StackOneAPIError.

Errors:
- The MCP client raises inside a TaskGroup, so every failure read as
  "unhandled errors in a TaskGroup (1 sub-exception)". Failures are now unwrapped
  to the leaf and reported as StackOneAPIError carrying .status_code, so a caller
  can branch on 412 rather than parsing a message.
- A failed GET /accounts no longer falls into the catch-all that discarded status.
- One unusable account no longer costs the caller every healthy account's tools.

Schema:
- _normalize_schema_properties used setdefault, so a served `nullable` (OpenAPI
  3.0 style, meaning "accepts null") suppressed the required marker and the model
  omitted the field.
- The envelope split used setdefault, making precedence depend on caller dict
  order, and put a scalar-valued reserved key into the body — the body-field
  smuggling invariant 4 exists to prevent. Now two deterministic passes.

Tests — the mock was the reason none of this was caught. It answered
`c.req.header('x-account-id') ?? 'default'`, inventing an account and making it
more permissive than production, so an SDK that never sent the header still got
a catalog. Two tests had grown around that: one asserted an api-key-only toolset
returns tools, another asserted "x-account-id" not in headers. The mock now
returns the real 400, serves /accounts, and new tests cover account discovery,
inactive-account filtering, listing failures by status code, tool_mode routing,
and isError. A guard test asserts the mock still rejects a missing header, so
these cannot quietly go hollow again.

  178 tests pass; conformance and the SDK/Pydantic AI smoke suites pass;
  verified against the live API.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An API key alone was not enough to use the SDK: the /mcp endpoint requires
x-account-id, so every caller had to know an account id up front. The toolset
now fetches GET /accounts, keeps the active ones, and fans out over them.

- fetch_accounts() exposes the account list, raising StackOneAPIError on failure
- search()/execute() drive the search_execute meta tools directly, so a caller
  can find and run an action without ever naming an account
- README Quick Start rewritten around the three flows: search+execute with no
  account filtering, filtering tools by account_id, and executing with args

The mock served x-account-id as `?? 'default'`, inventing an account that the
real endpoint rejects — which is why the tests stayed green while the SDK was
broken. It now requires the header (or query param) and 400s without it, and
serves the search_execute meta tools so tools/call is exercised for the first
time. Meta tools register with zod shapes because registerTool validates call
arguments with zod, not with the raw JSON Schema used for listing.

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

execute() routed through the catalog when the action_id happened to be listed
there and through the connector's meta tool otherwise — so the same id took flat
`query_pageSize` keys or the nested `{"query": {...}}` envelope depending on
something the caller cannot see. It now always uses the meta tool, which is the
envelope every action's `example_request` shows; the flat form stays on
fetch_tools() tools, whose own served schema names the keys. It also no longer
fetches the whole catalog on every call.

search() raised on the first connector that errored, so one bad connector hid
every other connector's results. It now collects failures and raises only when
nothing succeeded — the rule fetch_tools() already applied to listing.

The mock's _execute_action took flat args, which the real endpoint does not; it
now takes the nested envelope and returns an example_request, and a test pins
that contract. README examples name an action id rather than indexing search
results.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`download.execute({"id": "file-id"})` routed the id into the RPC envelope's
body, not its path — tools are listed flat-prefixed, and an unprefixed key falls
through to the body, so the call downloaded nothing. `path_id` is the key the
served schema actually names.

The download behaviour itself is current: /actions/rpc serves a download action's
body as raw binary, which StackOneRpcTool.execute still routes through the
Content-Type branch (#190).

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

5 issues found across 8 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="stackone_ai/tools.py">

<violation number="1" location="stackone_ai/tools.py:155">
P2: Catching `BaseException` turns interrupts and cancellation into `ToolsetLoadError`. Catch `Exception` instead; ordinary `ExceptionGroup` failures from the MCP task group are still handled.</violation>

<violation number="2" location="stackone_ai/tools.py:227">
P2: When an MCP `tools/call` result uses `structuredContent` without a text part, this parser drops the result. Read `structuredContent` as the primary result or as a fallback before parsing text content.</violation>

<violation number="3" location="stackone_ai/tools.py:774">
P2: When a caller passes a falsey non-dict argument such as `[]`, `arguments or {}` bypasses validation and invokes the tool with `{}`. Preserve non-`None` values before checking their type.</violation>

<violation number="4" location="stackone_ai/tools.py:777">
P1: After `Tools.set_account_id()` changes the inherited account ID, MCP calls still send the constructor's `_mcp_headers` and can target the previous tenant. Build the MCP headers from `get_account_id()` for each call, or update them in an overridden setter.</violation>
</file>

<file name="stackone_ai/toolset.py">

<violation number="1" location="stackone_ai/toolset.py:270">
P1: When the same connector is linked to multiple accounts, `execute()` can run a searched action against the wrong tenant. `search()` drops the originating account and this routing keeps only the connector, so carry account context with each result and execute through that account (or require an explicit account selection).</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread stackone_ai/tools.py Outdated
parsed = arguments or {}
if not isinstance(parsed, dict):
raise ValueError("Tool arguments must be a JSON object")
return call_mcp_tool(self._endpoint, self._mcp_headers, self.name, parsed)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: After Tools.set_account_id() changes the inherited account ID, MCP calls still send the constructor's _mcp_headers and can target the previous tenant. Build the MCP headers from get_account_id() for each call, or update them in an overridden setter.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/tools.py, line 777:

<comment>After `Tools.set_account_id()` changes the inherited account ID, MCP calls still send the constructor's `_mcp_headers` and can target the previous tenant. Build the MCP headers from `get_account_id()` for each call, or update them in an overridden setter.</comment>

<file context>
@@ -605,6 +737,46 @@ def _build_action_headers(self, additional_headers: dict[str, Any] | None) -> di
+            parsed = arguments or {}
+        if not isinstance(parsed, dict):
+            raise ValueError("Tool arguments must be a JSON object")
+        return call_mcp_tool(self._endpoint, self._mcp_headers, self.name, parsed)
+
+
</file context>
Suggested change
return call_mcp_tool(self._endpoint, self._mcp_headers, self.name, parsed)
headers = dict(self._mcp_headers)
account_id = self.get_account_id()
if account_id:
headers["x-account-id"] = account_id
else:
headers.pop("x-account-id", None)
return call_mcp_tool(self._endpoint, headers, self.name, parsed)

Comment thread stackone_ai/toolset.py Outdated
Raises:
ToolsetLoadError: If no connector matches.
"""
connector = action_id.split("_")[0].lower()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: When the same connector is linked to multiple accounts, execute() can run a searched action against the wrong tenant. search() drops the originating account and this routing keeps only the connector, so carry account context with each result and execute through that account (or require an explicit account selection).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/toolset.py, line 270:

<comment>When the same connector is linked to multiple accounts, `execute()` can run a searched action against the wrong tenant. `search()` drops the originating account and this routing keeps only the connector, so carry account context with each result and execute through that account (or require an explicit account selection).</comment>

<file context>
@@ -175,21 +200,81 @@ def _fetch_for_account(account: str | None) -> list[StackOneTool]:
-        if tool is None:
-            raise ToolsetLoadError(f'Tool "{tool_name}" not found')
-        return tool.execute(arguments or {})
+        connector = action_id.split("_")[0].lower()
+        for tool in self._meta_tools("_execute_action", account_ids):
+            if tool.name.split("_")[0].lower() == connector:
</file context>

Comment thread stackone_ai/toolset.py Outdated
Comment thread stackone_ai/toolset.py Outdated
Comment thread stackone_ai/tools.py

try:
return run_async(_list())
except BaseException as exc:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Catching BaseException turns interrupts and cancellation into ToolsetLoadError. Catch Exception instead; ordinary ExceptionGroup failures from the MCP task group are still handled.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/tools.py, line 155:

<comment>Catching `BaseException` turns interrupts and cancellation into `ToolsetLoadError`. Catch `Exception` instead; ordinary `ExceptionGroup` failures from the MCP task group are still handled.</comment>

<file context>
@@ -148,7 +150,127 @@ async def _list() -> list[McpToolDefinition]:
-    return run_async(_list())
+    try:
+        return run_async(_list())
+    except BaseException as exc:
+        raise _describe_mcp_failure(exc, endpoint) from exc
+
</file context>
Suggested change
except BaseException as exc:
except Exception as exc:

Comment thread stackone_ai/tools.py Outdated
Comment thread stackone_ai/tools.py
if isinstance(arguments, str):
parsed = json.loads(arguments)
else:
parsed = arguments or {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When a caller passes a falsey non-dict argument such as [], arguments or {} bypasses validation and invokes the tool with {}. Preserve non-None values before checking their type.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/tools.py, line 774:

<comment>When a caller passes a falsey non-dict argument such as `[]`, `arguments or {}` bypasses validation and invokes the tool with `{}`. Preserve non-`None` values before checking their type.</comment>

<file context>
@@ -605,6 +737,46 @@ def _build_action_headers(self, additional_headers: dict[str, Any] | None) -> di
+        if isinstance(arguments, str):
+            parsed = json.loads(arguments)
+        else:
+            parsed = arguments or {}
+        if not isinstance(parsed, dict):
+            raise ValueError("Tool arguments must be a JSON object")
</file context>
Suggested change
parsed = arguments or {}
parsed = {} if arguments is None else arguments

Comment thread stackone_ai/tools.py
back as an ordinary response with that flag set, so without this check the
error body is handed to the caller as though it were a success.
"""
texts = [getattr(part, "text", "") for part in result.content]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When an MCP tools/call result uses structuredContent without a text part, this parser drops the result. Read structuredContent as the primary result or as a fallback before parsing text content.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/tools.py, line 227:

<comment>When an MCP `tools/call` result uses `structuredContent` without a text part, this parser drops the result. Read `structuredContent` as the primary result or as a fallback before parsing text content.</comment>

<file context>
@@ -148,7 +150,127 @@ async def _list() -> list[McpToolDefinition]:
+            back as an ordinary response with that flag set, so without this check the
+            error body is handed to the caller as though it were a success.
+    """
+    texts = [getattr(part, "text", "") for part in result.content]
+    payload = "".join(t for t in texts if t)
+    # Parts that are not text (images, embedded resources) have no `.text`; keep them
</file context>

Comment thread README.md Outdated
Comment thread README.md Outdated
Quick Start is the one recommended path; integrations, then filtering as a
reference list, then examples and development.

- Features claimed a `"!*_delete_*"` exclusion syntax that does not exist —
  `_filter_by_action` is plain fnmatch, so that pattern still matches a delete
  tool. Replaced the list with what the SDK actually does.
- Adapter extras moved into Installation as a line: nothing else mentioned them,
  and every integration example below needs one.
- Advanced Filtering names three arguments with no signature in sight; one line
  says whose they are.
- Collapsed the blank runs left by the trim.

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread README.md Outdated
willleeney and others added 4 commits September 17, 2026 09:45
Found by an audit sweep that drove the live API rather than a mock. The three
that matter most were all exploitable through model-supplied tool arguments,
which are attacker-controlled whenever the model can be prompt-injected.

- execute() spread `arguments` over the pinned action_id, so a model-supplied
  "action_id" silently replaced the action the caller had fixed. action_id now
  goes last. Verified live: the override no longer takes.
- The reserved-header guard compared `str(key).lower()` with no strip, so
  " authorization" and "AUTHORIZATION\t" sailed past it, and CR/LF in a name or
  value was never rejected. It now normalises, casefolds, and enforces the
  RFC 7230 grammar — and it lives on the base class, because the MCP path, which
  is what search()/execute() actually use, had no guard at all.
- filename_from_content_disposition returned "../../.ssh/authorized_keys"
  verbatim from an attacker-chosen Content-Disposition. Sanitised to a basename
  at the source, after the RFC 5987 percent-decode where a caller's own filter
  would have been bypassed.

Correctness, same sweep:

- _tool_mode was mutated on self and restored in a finally, so a concurrent
  fetch_tools() could cache search_execute meta tools under the individual-mode
  key, permanently. The mode is a parameter now.
- The cache stored the Tools wrapper, whose tools are mutable, so one caller's
  set_account_id() rescoped every later caller's. It caches the listing and
  builds fresh tools per call. api_key and base_url join the key; providers and
  actions leave it (filtering is local and must not refetch); a partial catalog
  is no longer cached.
- The envelope split was caller-order-dependent: {"body_foo":1,"foo":2} and its
  reverse produced different wire bodies, and a third disagreed with the Node
  SDK, breaking the byte-equality the conformance suite asserts. Precedence is
  now a property of the kind of key.
- The split is schema-aware: a body field named path_to_file was being stolen
  into the path bucket. An empty schema means "no schema", not "nothing is
  declared" — the other way round loses every path parameter.
- search() caught only StackOneError, but transport failures arrive as
  ToolsetLoadError, so one unreachable connector aborted the whole search. It
  now catches what fetch_tools() catches, fans out in parallel rather than
  serially, ranks globally by similarity_score, and validates top_k locally
  instead of spending a round trip per connector to be told 1..50.
- Provider matching split on the first underscore, so browser_linkedin tools
  reported provider "browser" and asking for browser_linkedin returned nothing.
  Matched as a full prefix now, longest connector wins.
- fetch_tools no longer re-wraps StackOneAPIError, which threw away the status.
- fetch_accounts rejects a non-list body instead of list(dict) yielding keys.
- A non-list `required` no longer marks every property optional.
- A bodyless 2xx is not a file download; a malformed JSON response no longer
  reports itself as "invalid JSON in arguments".
- A scalar under an envelope key raises instead of vanishing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every code block and every example is now executed live against a real key
before being committed. Several were not true before.

README:
- The Quick Start's sample search() output was invented. The real hits carry
  similarity_score and input_schema, and example_request is {"action_id": ...} —
  a template to edit, not a runnable call.
- Its execute() arguments, {"query": {"page_size": 25}}, matched no action's
  schema. The server drops unmatched arguments without an error, so the first
  line a reader copies returned 50 rows while appearing to ask for 25. Replaced
  with the schema-correct call and a warning that this failure mode is silent.
- Added the section the README most needed: the two calling surfaces take
  different argument shapes (nested vs flat-prefixed) and return different
  envelopes, and mixing them fails quietly.
- Added Accounts and Errors sections. The word "error" did not previously appear
  in the document.
- Integration examples rewritten: they all filtered on workday_*, so a reader
  without Workday silently got zero tools and an agent with an empty toolbox.
  They now discover whatever the key reaches. Added the missing OpenAI block.
- Dropped CrewAI. langchain_core.BaseTool is not crewai.tools.BaseTool, so the
  "works natively, seamless" claim raises a ValidationError on contact — verified
  against crewai 1.6.1.
- Removed StackOneToolSet(search={"method": "auto"}); no such parameter exists.
- Advanced Filtering gets worked examples with real counts, and documents that a
  leading "!" is not exclusion syntax: actions=["*", "!*_delete_*"] returns every
  delete tool, the opposite of what it reads as.

examples/:
- All six exited 0 while loading zero tools, so a green run proved nothing. They
  now discover actions and exit non-zero on an empty catalog.
- Added search_and_execute.py covering the recommended flow, which had no
  example at all.
- Added error handling, which every one of them lacked despite CLAUDE.md
  requiring it.
- Deleted crewai_integration.py; it only "passed" by loading no tools.

Also: an MCP-path failure reported status_code 0 while the payload carried the
real status, so the documented `except StackOneAPIError` could not branch on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…i 2.x

The validate.sh check that guards examples across a breaking restructure was
vacuous. It grepped ty's output for a diagnostic naming `stackone_ai`, but the
case it exists to catch — `Object of type "Tools" has no attribute "gone"` —
never names the package, and a trailing `|| true` discarded ty's exit code, so
every other type error in examples/ was ignored too. It now scores the exit code;
verified by planting a call to a removed method and watching it fail.

That immediately found two real problems: a deprecated `create_react_agent`
import, and the fact that CI never type-checked examples at all. Added the step,
and corrected CLAUDE.md, which asserted a CI gate that did not exist.

- to_langchain() now raises ToolException rather than StackOneError. LangChain's
  handle_tool_error only catches ToolException, so a rejected call killed the
  graph instead of reaching the agent — with this, the langgraph example
  recovers from a bad argument guess and retries, which it could not do before.
- pydantic-ai's <2.0.0 cap is lifted. The smoke suite has been passing against
  2.43.0 all along, so the cap only blocked consumers, and shipping it in a major
  would bake it in for a release cycle.
- crewai leaves the examples extra (its integration is gone); langchain joins it,
  since the langgraph example needs it.
- The MCP ImportError told users to install a `[mcp]` extra that no longer
  exists — mcp has been core since the dependency retiering.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The SDK shipped unusable while the suite was green because every test double was
written to make the client's existing requests succeed, rather than to model what
the server demands. The missing x-account-id on /mcp has been fixed; these were
its surviving siblings.

- /actions/rpc accepted any request whose Authorization merely began with
  "Basic " and never looked at x-account-id at all, so an unscoped execution
  request — the same defect one endpoint over — could not be caught. It now 400s.
  Proven by mutation: dropping the header from _prepare_headers now fails three
  integration tests, where before it failed none.
- /mcp fell back to `accountTools.default` for an unknown account, serving a full
  catalog for an account that does not exist. That is the same `?? 'default'`
  permissiveness as the original bug, one line further down, and it made any
  wrong, stale or mangled account id invisible. It now 404s.
- test_execute_without_account_id asserted only that the envelope omitted
  x-account-id — the same shape as the assertion that pinned the shipped bug, a
  green test recording what the client happened to send. It now also checks the
  HTTP header, which is the one the API reads.
- Added TestServerRefusals covering both refusals directly.

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

10 issues found across 20 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/ci.yaml">

<violation number="1" location=".github/workflows/ci.yaml:79">
P3: The new CI gate type-checks examples/, but the documented local pre-commit command (`make format`) still runs only `uv run ty check stackone_ai`. A contributor who follows the repo instructions can push example code that fails this new CI step without any local warning. Add `uv run ty check examples` to the Makefile `format` target (after the existing `ty check stackone_ai`) so the local fix-before-commit loop matches CI.</violation>
</file>

<file name="tests/mocks/mcp-server.ts">

<violation number="1" location="tests/mocks/mcp-server.ts:79">
P3: The `MockMcpServerOptions.accountTools` docstring still says "Use 'default' for tools when no account header is provided", but this change makes that wrong: a missing header returns 400 before the lookup, and an unknown id returns 404. The `default` key now only matches a literal `x-account-id: default` request, which happens only because serve.ts's `/accounts` discovery lists id "default". Update the comment (and the createMcpApp example framing) to describe this so a future test author doesn't rely on the removed fallback.</violation>

<violation number="2" location="tests/mocks/mcp-server.ts:79">
P3: When an unknown account ID is a prototype property such as `constructor` or `__proto__`, `in` treats it as configured and the later tool loop throws a 500 instead of returning the intended 404. Use an own-property check for the account map.</violation>
</file>

<file name="stackone_ai/tools.py">

<violation number="1" location="stackone_ai/tools.py:477">
P2: When a server labels invalid UTF-8 as JSON, `response.json()` raises `UnicodeDecodeError`, so malformed responses bypass this new error conversion. Catch `UnicodeDecodeError` alongside `JSONDecodeError` and preserve the `StackOneAPIError` response contract.</violation>

<violation number="2" location="stackone_ai/tools.py:640">
P2: When a LangChain MCP tool hits a transport failure, `_run` still propagates `ToolsetLoadError` because this handler catches only `StackOneError`. Catch `ToolsetError` too so `handle_tool_error` can process all MCP execution failures.</violation>
</file>

<file name="tests/test_fetch_tools.py">

<violation number="1" location="tests/test_fetch_tools.py:828">
P2: This test cannot pass as written: the SDK raises `StackOneAPIError` for the mock's 404 on an unknown account, but the assertion expects `ToolsetError`. `_describe_mcp_failure` maps the HTTP 404 to `StackOneAPIError` (subclass of `StackOneError`, not `ToolsetError`), and `fetch_tools` re-raises it unchanged. Assert `StackOneAPIError` and check `status_code == 404`, matching the PR's goal of preserving MCP status codes.</violation>
</file>

<file name="examples/search_and_execute.py">

<violation number="1" location="examples/search_and_execute.py:57">
P2: The recommended generic example hard-codes `body.variables` after selecting an arbitrary action, so its `first` filter is silently ignored for most schemas. Build the arguments from `best['input_schema']` or use an action-specific request.</violation>
</file>

<file name="examples/openai_integration.py">

<violation number="1" location="examples/openai_integration.py:63">
P1: When multiple linked accounts expose the same action, this unscoped fetch lets name-only dispatch execute the model call against the last account. Scope the example to one account or preserve account-aware dispatch.</violation>
</file>

<file name="README.md">

<violation number="1" location="README.md:128">
P2: The new claim that every integration runs against any linked account is false for three advertised scripts, which still require `STACKONE_ACCOUNT_ID`. Update those scripts to use discovery or qualify this guidance.</violation>
</file>

<file name="stackone_ai/toolset.py">

<violation number="1" location="stackone_ai/toolset.py:355">
P1: When `execute()` uses the production `*_execute_action` MCP tool, it sends `action_id`, but the served schema requires `action_name`, so the call cannot execute the selected action. Send the selector field required by the served schema and update the mock to match the production contract.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread stackone_ai/tools.py Outdated
# Read-only list actions, whichever provider this key happens to reach. Keeping
# the filter narrow matters: an unfiltered catalog is hundreds of tools and will
# not fit a model's context.
tools = toolset.fetch_tools(actions=["*_list_*"])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: When multiple linked accounts expose the same action, this unscoped fetch lets name-only dispatch execute the model call against the last account. Scope the example to one account or preserve account-aware dispatch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/openai_integration.py, line 63:

<comment>When multiple linked accounts expose the same action, this unscoped fetch lets name-only dispatch execute the model call against the last account. Scope the example to one account or preserve account-aware dispatch.</comment>

<file context>
@@ -23,55 +32,56 @@
+    # Read-only list actions, whichever provider this key happens to reach. Keeping
+    # the filter narrow matters: an unfiltered catalog is hundreds of tools and will
+    # not fit a model's context.
+    tools = toolset.fetch_tools(actions=["*_list_*"])
+    if not tools:
+        raise SystemExit(
</file context>

Comment thread stackone_ai/toolset.py Outdated
# action_id LAST. Spreading arguments over it let a model-supplied
# "action_id" silently replace the action the caller pinned — the exact
# thing a host app pins it for.
return tool.execute({**(arguments or {}), "action_id": action_id})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: When execute() uses the production *_execute_action MCP tool, it sends action_id, but the served schema requires action_name, so the call cannot execute the selected action. Send the selector field required by the served schema and update the mock to match the production contract.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/toolset.py, line 355:

<comment>When `execute()` uses the production `*_execute_action` MCP tool, it sends `action_id`, but the served schema requires `action_name`, so the call cannot execute the selected action. Send the selector field required by the served schema and update the mock to match the production contract.</comment>

<file context>
@@ -267,10 +336,23 @@ def execute(
+            # action_id LAST. Spreading arguments over it let a model-supplied
+            # "action_id" silently replace the action the caller pinned — the exact
+            # thing a host app pins it for.
+            return tool.execute({**(arguments or {}), "action_id": action_id})
 
         raise ToolsetLoadError(
</file context>

Comment thread examples/search_and_execute.py Outdated
Comment thread examples/search_and_execute.py Outdated
Comment thread tests/mocks/mcp-server.ts
// that does not exist is the same permissiveness that hid the missing-header bug
// one line further up: any bug that sends a wrong, stale or mangled account id
// would be invisible. The real API refuses.
if (!(accountId in accountTools)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The MockMcpServerOptions.accountTools docstring still says "Use 'default' for tools when no account header is provided", but this change makes that wrong: a missing header returns 400 before the lookup, and an unknown id returns 404. The default key now only matches a literal x-account-id: default request, which happens only because serve.ts's /accounts discovery lists id "default". Update the comment (and the createMcpApp example framing) to describe this so a future test author doesn't rely on the removed fallback.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/mocks/mcp-server.ts, line 79:

<comment>The `MockMcpServerOptions.accountTools` docstring still says "Use 'default' for tools when no account header is provided", but this change makes that wrong: a missing header returns 400 before the lookup, and an unknown id returns 404. The `default` key now only matches a literal `x-account-id: default` request, which happens only because serve.ts's `/accounts` discovery lists id "default". Update the comment (and the createMcpApp example framing) to describe this so a future test author doesn't rely on the removed fallback.</comment>

<file context>
@@ -72,7 +72,14 @@ export function createMcpApp(options: MockMcpServerOptions): HonoApp {
+		// that does not exist is the same permissiveness that hid the missing-header bug
+		// one line further up: any bug that sends a wrong, stale or mangled account id
+		// would be invisible. The real API refuses.
+		if (!(accountId in accountTools)) {
+			return c.json({ statusCode: 404, message: `Unknown account ${accountId}` }, 404);
+		}
</file context>

Comment thread examples/langchain_integration.py Outdated
Comment thread examples/pydantic_ai_integration.py Outdated
Comment thread examples/openai_integration.py Outdated
Comment thread tests/mocks/mcp-server.ts
// that does not exist is the same permissiveness that hid the missing-header bug
// one line further up: any bug that sends a wrong, stale or mangled account id
// would be invisible. The real API refuses.
if (!(accountId in accountTools)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: When an unknown account ID is a prototype property such as constructor or __proto__, in treats it as configured and the later tool loop throws a 500 instead of returning the intended 404. Use an own-property check for the account map.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/mocks/mcp-server.ts, line 79:

<comment>When an unknown account ID is a prototype property such as `constructor` or `__proto__`, `in` treats it as configured and the later tool loop throws a 500 instead of returning the intended 404. Use an own-property check for the account map.</comment>

<file context>
@@ -72,7 +72,14 @@ export function createMcpApp(options: MockMcpServerOptions): HonoApp {
+		// that does not exist is the same permissiveness that hid the missing-header bug
+		// one line further up: any bug that sends a wrong, stale or mangled account id
+		// would be invisible. The real API refuses.
+		if (!(accountId in accountTools)) {
+			return c.json({ statusCode: 404, message: `Unknown account ${accountId}` }, 404);
+		}
</file context>
Suggested change
if (!(accountId in accountTools)) {
if (!Object.prototype.hasOwnProperty.call(accountTools, accountId)) {

willleeney and others added 2 commits September 17, 2026 13:32
…onable errors

A second audit sweep drove the live API and found that several of this morning's
fixes were themselves wrong, plus one adapter that was broken for every call.

Security — the header guard becomes an allowlist:

- It was a two-name denylist, so Proxy-Authorization, x-stackone-account-id,
  Cookie, X-Api-Key and every other header reached the envelope. A denylist has
  to enumerate every synonym of "credential" in every provider's vocabulary.
  The allowlist is the served schema itself, so it needs no maintenance: zero of
  the 139 served actions declare a headers_* property, and the RPC server was
  measured to ignore the envelope's headers object outright.
- `$` also matches before a trailing newline, so `match` let "value\n" through —
  the one character class the CR/LF guard exists to reject. Now fullmatch.
- RFC 7230 permits obs-text, so legitimate non-ASCII values were being dropped.
- Filenames: Windows drive-relative paths (C:evil.exe), control characters and
  Unicode bidi overrides (U+202E renders "gnp.exe" as "exe.png") all survived the
  traversal fix. Capped at 255 bytes, and the filename regexes now anchor to a
  parameter boundary so a decoy `notfilename=` cannot win.

LangChain was broken for every tool, live:

- The adapter rebuilt an args schema from each property's top-level type, which
  discarded every nested object's fields, enums, bounds, item types and unions —
  the model was told "pass an object" with no field names. It now hands over the
  served JSON Schema, making this surface byte-equivalent to to_openai_function.
- pydantic materialised every optional as None and BaseTool forwarded them all,
  and the API reads an explicit null as "required field missing", so 10/10 list
  tools 400'd through the adapter while succeeding directly.
- ToolException carried str(exc) — httpx boilerplate linking to MDN. The field
  that is actually wrong is in response_body, so agents retried blind.

Errors and correctness:

- str(StackOneAPIError) now leads with the server's own message. This is the
  error every bad tool call produces and it pointed users at MDN's generic 400
  page while "The required field 'path.id' is missing" sat unread.
- fetch_accounts had no error handling at all — a dead host leaked httpx's own
  exception type out of the SDK.
- RPC and MCP tools parsed JSON before the base class's handler, so the
  documented ValueError never fired for any tool a user can actually obtain.
- _connector_of split on the last underscore, but nanoid's alphabet includes "_",
  so every action on such an account was unroutable. Ambiguous matches now warn.
- search() crashed with a bare TypeError if any connector returned a non-numeric
  similarity_score; an ambiguous connector picked one account silently.
- The "is this schema flat-prefixed" test used any(), which the very key it
  protects satisfies. ALL, not any: one bare name disproves it.
- A declared property named `query` was rejected as a malformed envelope container.
- A zero-byte body with a download content type is a real empty file, not a
  bodyless JSON success.
- Status extraction missed `statusCode`, the casing the API actually emits.
- action_id and set_accounts are type-checked like their siblings already were.

Tests: the MCP header guard — the security fix on the path search()/execute()
actually use — had zero coverage and could be deleted with everything green. It
now has nine cases. Also pinned: provider prefix matching, top_k validation,
the non-list /accounts body, and non-dict arguments. One of my own tests was
vacuous: it passed `set() or None`, which the parser folds to None, so the
branch it claimed to cover never ran.

validate.sh now points ADK at the real plugin path, so its skip reports the true
reason (the plugin pins a version not yet on PyPI) instead of a stale path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With two accounts on one provider the name map silently kept the last one, so
get_tool() routed every call to whichever account happened to list last — an
action running against an account the caller never chose. Nothing downstream
surfaces it: OpenAI accepts duplicate function names without complaint, so the
model just sees the tool twice and pays for it twice.

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

4 issues found across 8 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/validate.sh">

<violation number="1" location="scripts/validate.sh:22">
P2: The new default ADK path `$SDK/../adk-26-ci` no longer matches the canonical downstream repo. The script's own header (line 8) still documents `ADK=/path/to/stackone-adk-plugin`, and this PR's migration notes name `stackone-adk-plugin` as the repo with prepared branches. A name like `adk-26-ci` reads as a CI/testing fork. Consequence: a local `make validate` run with the plugin checked out under the documented name `stackone-adk-plugin` finds no `adk-26-ci` directory and silently SKIPs the ADK smoke while still exiting 0 ("VALIDATE: PASS (with N skipped)"), so ADK coverage drops without any signal. If the fork is the intended default, update the header usage comment and confirm `adk-26-ci` is a stable repo; otherwise revert the default to `stackone-adk-plugin`.</violation>
</file>

<file name="stackone_ai/types.py">

<violation number="1" location="stackone_ai/types.py:131">
P2: When a provider returns a filename longer than 255 bytes without a short ASCII suffix, this guard still returns an overlong filename. Callers saving the download can then hit filesystem name-length errors; calculate the retained suffix in bytes or truncate the complete UTF-8 byte string.</violation>
</file>

<file name="tests/test_models.py">

<violation number="1" location="tests/test_models.py:230">
P2: Lines 230-231 exceed the configured ruff line length of 110 (measured 111 and 113 chars) and would be rewritten by `make format`, so the CI ruff format/lint check fails as committed. Wrap the comparison operands so each line stays within the limit.</violation>
</file>

<file name="stackone_ai/toolset.py">

<violation number="1" location="stackone_ai/toolset.py:449">
P2: When `base_url` is malformed, `fetch_accounts()` still leaks `httpx.InvalidURL` because this handler catches only `httpx.HTTPError`. Catch `httpx.InvalidURL` as well so direct account discovery consistently raises `ToolsetLoadError.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/validate.sh
HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SDK="$(dirname "$HERE")"
CONFORMANCE="${CONFORMANCE:-$SDK/../sdk-conformance}"
ADK="${ADK:-$SDK/../adk-26-ci}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The new default ADK path $SDK/../adk-26-ci no longer matches the canonical downstream repo. The script's own header (line 8) still documents ADK=/path/to/stackone-adk-plugin, and this PR's migration notes name stackone-adk-plugin as the repo with prepared branches. A name like adk-26-ci reads as a CI/testing fork. Consequence: a local make validate run with the plugin checked out under the documented name stackone-adk-plugin finds no adk-26-ci directory and silently SKIPs the ADK smoke while still exiting 0 ("VALIDATE: PASS (with N skipped)"), so ADK coverage drops without any signal. If the fork is the intended default, update the header usage comment and confirm adk-26-ci is a stable repo; otherwise revert the default to stackone-adk-plugin.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/validate.sh, line 22:

<comment>The new default ADK path `$SDK/../adk-26-ci` no longer matches the canonical downstream repo. The script's own header (line 8) still documents `ADK=/path/to/stackone-adk-plugin`, and this PR's migration notes name `stackone-adk-plugin` as the repo with prepared branches. A name like `adk-26-ci` reads as a CI/testing fork. Consequence: a local `make validate` run with the plugin checked out under the documented name `stackone-adk-plugin` finds no `adk-26-ci` directory and silently SKIPs the ADK smoke while still exiting 0 ("VALIDATE: PASS (with N skipped)"), so ADK coverage drops without any signal. If the fork is the intended default, update the header usage comment and confirm `adk-26-ci` is a stable repo; otherwise revert the default to `stackone-adk-plugin`.</comment>

<file context>
@@ -19,7 +19,7 @@ set -uo pipefail
 SDK="$(dirname "$HERE")"
 CONFORMANCE="${CONFORMANCE:-$SDK/../sdk-conformance}"
-ADK="${ADK:-$SDK/../stackone-adk-plugin}"
+ADK="${ADK:-$SDK/../adk-26-ci}"
 
 failed=0
</file context>
Suggested change
ADK="${ADK:-$SDK/../adk-26-ci}"
ADK="${ADK:-$SDK/../stackone-adk-plugin}"

Comment thread stackone_ai/types.py
Comment on lines +131 to +134
if len(base.encode("utf-8", "ignore")) > 255:
stem, dot, suffix = base.rpartition(".")
keep = 255 - len(dot + suffix)
base = (stem.encode("utf-8")[: max(keep, 1)].decode("utf-8", "ignore")) + dot + suffix

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When a provider returns a filename longer than 255 bytes without a short ASCII suffix, this guard still returns an overlong filename. Callers saving the download can then hit filesystem name-length errors; calculate the retained suffix in bytes or truncate the complete UTF-8 byte string.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/types.py, line 131:

<comment>When a provider returns a filename longer than 255 bytes without a short ASCII suffix, this guard still returns an overlong filename. Callers saving the download can then hit filesystem name-length errors; calculate the retained suffix in bytes or truncate the complete UTF-8 byte string.</comment>

<file context>
@@ -118,8 +120,19 @@ def _safe_basename(name: str | None) -> str | None:
+    base = "".join(ch for ch in base if unicodedata.category(ch) not in ("Cc", "Cf")).strip()
+    if base in ("", ".", ".."):
+        return None
+    if len(base.encode("utf-8", "ignore")) > 255:
+        stem, dot, suffix = base.rpartition(".")
+        keep = 255 - len(dot + suffix)
</file context>
Suggested change
if len(base.encode("utf-8", "ignore")) > 255:
stem, dot, suffix = base.rpartition(".")
keep = 255 - len(dot + suffix)
base = (stem.encode("utf-8")[: max(keep, 1)].decode("utf-8", "ignore")) + dot + suffix
if len(base.encode("utf-8", "ignore")) > 255:
stem, dot, suffix = base.rpartition(".")
suffix_bytes = (dot + suffix).encode("utf-8", "ignore")
if len(suffix_bytes) >= 255:
base = suffix_bytes[:255].decode("utf-8", "ignore")
else:
keep = 255 - len(suffix_bytes)
stem = stem.encode("utf-8", "ignore")[:keep].decode("utf-8", "ignore")
base = stem + dot + suffix

Comment thread tests/test_models.py
Comment on lines +230 to +231
assert set(langchain_tools[0].args_schema["properties"]) == set(mock_tool.parameters.properties.keys())
assert set(langchain_tools[1].args_schema["properties"]) == set(second_tool.parameters.properties.keys())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Lines 230-231 exceed the configured ruff line length of 110 (measured 111 and 113 chars) and would be rewritten by make format, so the CI ruff format/lint check fails as committed. Wrap the comparison operands so each line stays within the limit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_models.py, line 230:

<comment>Lines 230-231 exceed the configured ruff line length of 110 (measured 111 and 113 chars) and would be rewritten by `make format`, so the CI ruff format/lint check fails as committed. Wrap the comparison operands so each line stays within the limit.</comment>

<file context>
@@ -229,12 +227,8 @@ def test_to_langchain_multiple_tools(mock_tool):
-    assert set(langchain_tools[1].args_schema.__annotations__.keys()) == set(
-        second_tool.parameters.properties.keys()
-    )
+    assert set(langchain_tools[0].args_schema["properties"]) == set(mock_tool.parameters.properties.keys())
+    assert set(langchain_tools[1].args_schema["properties"]) == set(second_tool.parameters.properties.keys())
 
</file context>
Suggested change
assert set(langchain_tools[0].args_schema["properties"]) == set(mock_tool.parameters.properties.keys())
assert set(langchain_tools[1].args_schema["properties"]) == set(second_tool.parameters.properties.keys())
assert set(langchain_tools[0].args_schema["properties"]) == set(
mock_tool.parameters.properties.keys()
)
assert set(langchain_tools[1].args_schema["properties"]) == set(
second_tool.parameters.properties.keys()
)

Comment thread stackone_ai/tools.py
Comment thread stackone_ai/toolset.py
return cached

endpoint = f"{self.base_url.rstrip('/')}/mcp?param-style={_MCP_PARAM_STYLE}"
except httpx.HTTPError as exc:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When base_url is malformed, fetch_accounts() still leaks httpx.InvalidURL because this handler catches only httpx.HTTPError. Catch httpx.InvalidURL as well so direct account discovery consistently raises `ToolsetLoadError.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/toolset.py, line 449:

<comment>When `base_url` is malformed, `fetch_accounts()` still leaks `httpx.InvalidURL` because this handler catches only `httpx.HTTPError`. Catch `httpx.InvalidURL` as well so direct account discovery consistently raises `ToolsetLoadError.</comment>

<file context>
@@ -401,14 +437,21 @@ def fetch_accounts(self) -> list[JsonDict]:
+                },
+                timeout=self._timeout,
+            )
+        except httpx.HTTPError as exc:
+            # The only public method with no error handling at all: a dead host, a bad
+            # scheme or a timeout leaked httpx's own exception type straight out of the
</file context>
Suggested change
except httpx.HTTPError as exc:
except (httpx.HTTPError, httpx.InvalidURL) as exc:

…ched state

Found by hammering one shared toolset from 28 threads against the live API.

- A listing in flight when clear_catalog_cache() fired wrote its pre-clear
  catalog back afterwards, so the stale catalog the clear existed to drop was
  served for the life of the process. Listings now record the cache generation
  they started under and refuse to write back if it has moved.
- Tools are rebuilt per call, but the rebuild copied only the top-level property
  dicts: nested schema objects, and the MCP headers dict shared by every tuple in
  a cached listing, were still aliased across callers. One caller mutating a
  nested schema changed what every later caller's model was shown.

Verified dead under the same load: the _tool_mode race and cross-caller
set_account_id() leakage, both fixed earlier.

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="stackone_ai/toolset.py">

<violation number="1" location="stackone_ai/toolset.py:137">
P2: When an account discovery is in flight during `clear_catalog_cache()`, the stale discovery can repopulate the cache after the clear. Make discovery generation-aware: capture the generation before `fetch_accounts()` and retry instead of publishing or caching results when the generation changes.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread stackone_ai/toolset.py
def get_search_tool(self, *, search: SearchMode | None = None) -> SearchTool:
"""Get a callable search tool that returns Tools collections.
with self._cache_lock:
self._cache_generation += 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When an account discovery is in flight during clear_catalog_cache(), the stale discovery can repopulate the cache after the clear. Make discovery generation-aware: capture the generation before fetch_accounts() and retry instead of publishing or caching results when the generation changes.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/toolset.py, line 137:

<comment>When an account discovery is in flight during `clear_catalog_cache()`, the stale discovery can repopulate the cache after the clear. Make discovery generation-aware: capture the generation before `fetch_accounts()` and retry instead of publishing or caching results when the generation changes.</comment>

<file context>
@@ -124,8 +133,10 @@ def clear_catalog_cache(self) -> None:
-        self._catalog_cache.clear()
-        self._discovered_account_ids = None
+        with self._cache_lock:
+            self._cache_generation += 1
+            self._catalog_cache.clear()
+            self._discovered_account_ids = None
</file context>

willleeney and others added 3 commits September 17, 2026 14:01
…t the docs

Release and CI, from an adversarial workflow audit:

- Publishing ran in the release-please job with no dependency on CI at all. CI is
  a separate workflow, so a red main still merged the release PR and shipped to
  PyPI. Publish is now its own job behind a `pypi` environment that re-runs the
  suite against the exact commit being published.
- Release runs had no concurrency group, so two quick merges could both reach
  `uv publish`. They now queue rather than cancel.
- CI ran twice per PR (push on every branch plus pull_request, in different
  concurrency groups, testing different commits). Push is now main-only.
- `curl | tar` without pipefail meant a failed download produced an empty tar,
  exited 0, and failed two steps later as "gitleaks: command not found".
- The conformance token was persisted base64-encoded into .git/config, which log
  masking does not match, beside third-party code run by pnpm install.
- Added a `ci-ok` aggregate check to require, since the matrix names checks per
  Python version and adding one silently left it unrequired.
- The coverage job held pages:write and id-token:write it never used.

SDK:

- toolset.execute() returned {"isError", "result"}, but an isError response has
  already raised, so the flag could only ever be False and the wrapper just made
  the two calling surfaces return different shapes. It now returns the payload.
- The Pydantic AI adapter let StackOneError escape the agent loop, ending the run.
  Tool.from_schema does no argument validation, so every wrong guess reached the
  API. It now raises ModelRetry with the server's explanation — verified live: the
  agent survives a failing call and reports the actual reason.

validate.sh printed a bare PASS when four of five sections had been skipped, and
discarded example import tracebacks; it now reports how many checks ran.

Docs: every README code block re-executed live. Fixed an OpenAI block that 400s on
the round trip (it omitted the assistant turn before the tool results), a LangGraph
install that cannot import create_agent, the claim that mixing surfaces fails
silently in both directions (only one), `[]` meaning "no filter" (it means unset),
"all four derive from" (three, from two unrelated bases), and live tool counts that
were true only for one API key. CLAUDE.md now describes the two surfaces, the
silent argument drop, the header allowlist, the fullmatch trap, and why test doubles
must model the server's refusals — the things agents here kept getting wrong.

Examples: removed a prompt sentence duplicated in all four by an earlier edit, and
which had only been masking the LangChain null-argument bug. Without it every
example passes three of three live runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…x from this sweep

timeout= did nothing for listing, search() or execute(). The MCP client's own
defaults are a 30s connect and a 300s SSE read, and the SDK passed neither, so
StackOneToolSet(timeout=2) against a host that accepts and never answers hung for
over five minutes. The execution path honoured it; the MCP path — which every
listing and the whole search()/execute() flow use — did not. The timeout now
reaches every transport leg, and anyio.fail_after bounds the exchange overall so a
slow handshake cannot evade it either. Measured: 300s+ -> 2.0s on all three.

ToolsetError now subclasses StackOneError, so `except StackOneError` catches
everything this SDK raises. They were unrelated siblings, which meant the obvious
catch-all silently missed ToolsetConfigError and ToolsetLoadError — the errors a
user is most likely to hit on their first call. Existing `except ToolsetError`
clauses are unaffected.

Tests: every fix made in this sweep was mutation-tested by reverting it and
running the suite. Three reverts survived, and each was a real gap:

- The fullmatch fix. The allowlist now drops undeclared headers before the
  grammar check, so the CR/LF tests never reached it — the check is only
  exercised for a declared header, which is exactly where it matters.
- Filename traversal had no test at all.
- The timeout test used the default thread, so a regression would have hung the
  suite for five minutes rather than failing. It now fails within 15s.

With those closed, reverting any of these fails the suite: the action_id hijack
fix, underscore account routing, longest-connector routing, global search ranking,
full-prefix provider matching, the cache-generation guard, deep schema copies, the
top_k guard, fullmatch, the header allowlist, LangChain's null-argument drop, the
MCP timeout, the single error root, and filename sanitising. Also pinned: that
adapter errors carry the server's reason to the model on both LangChain and
Pydantic AI.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ctually pass CI

- A lone surrogate — what a model emits when a token boundary splits an emoji —
  or a value JSON cannot encode (a set, bytes) escaped from deep inside httpx as a
  bare UnicodeEncodeError or TypeError, outside the SDK's exception contract. It
  is an argument problem, so it now raises ValueError naming the tool.
- The sdist shipped the whole dev tree: uv.lock, pnpm-lock.yaml, CLAUDE.md,
  .github/ and the TypeScript mocks. It is now the package, README, LICENSE,
  CHANGELOG and pyproject. The py.typed force-include was redundant — hatchling
  ships it from `packages`, verified by building without it.
- Dependabot used the `pip` ecosystem, which bumps pyproject.toml without
  regenerating uv.lock, so every Python PR it opened failed `uv sync --locked`.
  Switched to `uv`, and added `npm` for the mock server, ignoring the MCP SDK,
  which is pinned exactly because newer versions reject the mock's raw schemas.
- `make install` now uses --locked, as CI does, so a local sync cannot silently
  re-lock and drift from what CI verified. The help parser no longer lets an
  undocumented target inherit the previous target's description.
- Deleted dead config: an `integration` pytest marker applied to no test, an
  unused fixture alias, and `langgraph` from the examples extra — nothing imports
  it, and `langchain`, where `create_agent` lives, depends on it.

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

8 issues found across 14 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".env.example">

<violation number="1" location=".env.example:4">
P3: The comment is inaccurate: langchain_integration.py, langgraph_integration.py, and pydantic_ai_integration.py each require STACKONE_ACCOUNT_ID (they print "Set STACKONE_ACCOUNT_ID..." and return when absent), not just auth_management.py. Only openai_integration.py actually runs without it, since fetch_tools() discovers accounts from the key. Update the comment so users don't skip the variable and hit failures in the listed examples.</violation>
</file>

<file name=".github/workflows/release.yaml">

<violation number="1" location=".github/workflows/release.yaml:71">
P1: A failed repository CI gate can still publish to PyPI because this job depends only on `release-please` and reruns pytest, not `ci-ok` or the strict-schema conformance checks. Gate `uv publish` on the complete required CI result, or run the same conformance and quality checks in this workflow before publishing.</violation>

<violation number="2" location=".github/workflows/release.yaml:72">
P1: When release-please reports no release, this job still runs because the `"false"` output string is truthy in a GitHub Actions condition. Compare the output with `'true'` before starting the publish job.</violation>
</file>

<file name="CLAUDE.md">

<violation number="1" location="CLAUDE.md:210">
P3: This sentence inaccurately states that str(StackOneAPIError) leads with the server's own message. In the code, every StackOneAPIError message is prefixed with SDK-side text ("MCP request to ... failed with", "Tool 'name' failed:", "Listing accounts at ... failed with") and the server body only follows that prefix. Reword to reflect that the message prefixes the SDK's own context, or drop the claim.</violation>
</file>

<file name=".github/workflows/ci.yaml">

<violation number="1" location=".github/workflows/ci.yaml:5">
P2: Pushes to non-main branches no longer run CI. A branch pushed before opening a PR, or one never opened as a PR, gets no tests, lint, or secret scan despite the repository's documented every-push CI contract. Run this workflow for all branch pushes (or change the documented contract).</violation>

<violation number="2" location=".github/workflows/ci.yaml:151">
P1: Fork pull requests cannot pass `ci-ok`: GitHub withholds `CONFORMANCE_REPO_TOKEN` from fork-triggered workflows, the conformance job exits for the missing token, and this new `needs` list propagates that failure. Make the required gate fork-safe by running the private conformance check in a trusted context or handling fork PRs without treating the unavailable secret-dependent result as a failure.</violation>
</file>

<file name="README.md">

<violation number="1" location="README.md:232">
P2: The LangGraph example now installs `langchain` but drops `langgraph`, while the code below it calls `create_agent` from `langchain.agents`, which requires langgraph at runtime. A user following the README will hit an import error. Restore `langgraph` in the install line.</violation>

<violation number="2" location="README.md:261">
P2: Without an `account_ids` filter, this action can produce one tool per active account, not exactly one. Add an account scope to make the example deterministic or describe the per-account result; otherwise `get_tool()` can route to the last account listed.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

# so a red main still merged the release PR and shipped to PyPI. The suite runs
# again here against the exact commit being published.
publish:
needs: release-please

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: A failed repository CI gate can still publish to PyPI because this job depends only on release-please and reruns pytest, not ci-ok or the strict-schema conformance checks. Gate uv publish on the complete required CI result, or run the same conformance and quality checks in this workflow before publishing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yaml, line 71:

<comment>A failed repository CI gate can still publish to PyPI because this job depends only on `release-please` and reruns pytest, not `ci-ok` or the strict-schema conformance checks. Gate `uv publish` on the complete required CI result, or run the same conformance and quality checks in this workflow before publishing.</comment>

<file context>
@@ -44,17 +63,46 @@ jobs:
+  # so a red main still merged the release PR and shipped to PyPI. The suite runs
+  # again here against the exact commit being published.
+  publish:
+    needs: release-please
+    if: ${{ needs.release-please.outputs.release_created }}
+    runs-on: ubuntu-latest
</file context>

Comment thread .github/workflows/ci.yaml
# missing when a job is renamed or a matrix entry is added, and nothing notices.
ci-ok:
if: always()
needs: [gitleaks, ci, conformance]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Fork pull requests cannot pass ci-ok: GitHub withholds CONFORMANCE_REPO_TOKEN from fork-triggered workflows, the conformance job exits for the missing token, and this new needs list propagates that failure. Make the required gate fork-safe by running the private conformance check in a trusted context or handling fork PRs without treating the unavailable secret-dependent result as a failure.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 151:

<comment>Fork pull requests cannot pass `ci-ok`: GitHub withholds `CONFORMANCE_REPO_TOKEN` from fork-triggered workflows, the conformance job exits for the missing token, and this new `needs` list propagates that failure. Make the required gate fork-safe by running the private conformance check in a trusted context or handling fork PRs without treating the unavailable secret-dependent result as a failure.</comment>

<file context>
@@ -138,13 +144,25 @@ jobs:
+  # missing when a job is renamed or a matrix entry is added, and nothing notices.
+  ci-ok:
+    if: always()
+    needs: [gitleaks, ci, conformance]
+    runs-on: ubuntu-latest
+    steps:
</file context>

# again here against the exact commit being published.
publish:
needs: release-please
if: ${{ needs.release-please.outputs.release_created }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: When release-please reports no release, this job still runs because the "false" output string is truthy in a GitHub Actions condition. Compare the output with 'true' before starting the publish job.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yaml, line 72:

<comment>When release-please reports no release, this job still runs because the `"false"` output string is truthy in a GitHub Actions condition. Compare the output with `'true'` before starting the publish job.</comment>

<file context>
@@ -44,17 +63,46 @@ jobs:
+  # again here against the exact commit being published.
+  publish:
+    needs: release-please
+    if: ${{ needs.release-please.outputs.release_created }}
+    runs-on: ubuntu-latest
+    # Configure a required reviewer on this environment in the repository settings.
</file context>
Suggested change
if: ${{ needs.release-please.outputs.release_created }}
if: ${{ needs.release-please.outputs.release_created == 'true' }}

Comment thread README.md

```bash
pip install langgraph langchain-openai
uv add 'stackone-ai[langchain]' langchain langchain-openai

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The LangGraph example now installs langchain but drops langgraph, while the code below it calls create_agent from langchain.agents, which requires langgraph at runtime. A user following the README will hit an import error. Restore langgraph in the install line.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 232:

<comment>The LangGraph example now installs `langchain` but drops `langgraph`, while the code below it calls `create_agent` from `langchain.agents`, which requires langgraph at runtime. A user following the README will hit an import error. Restore `langgraph` in the install line.</comment>

<file context>
@@ -214,7 +229,7 @@ print(agent.run_sync("Use a tool to list a few records, then summarise them.").o
 
 ```bash
-uv add 'stackone-ai[langchain]' langgraph langchain-openai
+uv add 'stackone-ai[langchain]' langchain langchain-openai

</file context>


</details>

```suggestion
uv add 'stackone-ai[langchain]' langchain langchain-openai langgraph

Comment thread .github/workflows/ci.yaml

on:
push:
branches:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Pushes to non-main branches no longer run CI. A branch pushed before opening a PR, or one never opened as a PR, gets no tests, lint, or secret scan despite the repository's documented every-push CI contract. Run this workflow for all branch pushes (or change the documented contract).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 5:

<comment>Pushes to non-main branches no longer run CI. A branch pushed before opening a PR, or one never opened as a PR, gets no tests, lint, or secret scan despite the repository's documented every-push CI contract. Run this workflow for all branch pushes (or change the documented contract).</comment>

<file context>
@@ -2,6 +2,8 @@ name: CI
 
 on:
   push:
+    branches:
+      - main
   pull_request:
</file context>

Comment thread README.md Outdated
Comment thread README.md
toolset.fetch_tools() # every tool, every active account
toolset.fetch_tools(providers=["linear"]) # one connector
toolset.fetch_tools(actions=["linear_list_*"]) # one connector's list actions
toolset.fetch_tools(actions=["linear_get_issue"]) # exactly one tool

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Without an account_ids filter, this action can produce one tool per active account, not exactly one. Add an account scope to make the example deterministic or describe the per-account result; otherwise get_tool() can route to the last account listed.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 261:

<comment>Without an `account_ids` filter, this action can produce one tool per active account, not exactly one. Add an account scope to make the example deterministic or describe the per-account result; otherwise `get_tool()` can route to the last account listed.</comment>

<file context>
@@ -240,12 +255,12 @@ applied locally to one cached listing — changing a filter never refetches.
+toolset.fetch_tools()                                    # every tool, every active account
+toolset.fetch_tools(providers=["linear"])                # one connector
+toolset.fetch_tools(actions=["linear_list_*"])           # one connector's list actions
+toolset.fetch_tools(actions=["linear_get_issue"])        # exactly one tool
 toolset.fetch_tools(providers=["linear"],
-                    actions=["*_get_*"])                 #  23  (AND)
</file context>
Suggested change
toolset.fetch_tools(actions=["linear_get_issue"]) # exactly one tool
toolset.fetch_tools(actions=["linear_get_issue"], account_ids=["acc-123"]) # exactly one tool for this account

Comment thread .env.example
# Required for all examples. Account ids are discovered from the key.
STACKONE_API_KEY=your-stackone-api-key

# Only examples/auth_management.py needs a specific account

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The comment is inaccurate: langchain_integration.py, langgraph_integration.py, and pydantic_ai_integration.py each require STACKONE_ACCOUNT_ID (they print "Set STACKONE_ACCOUNT_ID..." and return when absent), not just auth_management.py. Only openai_integration.py actually runs without it, since fetch_tools() discovers accounts from the key. Update the comment so users don't skip the variable and hit failures in the listed examples.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .env.example, line 4:

<comment>The comment is inaccurate: langchain_integration.py, langgraph_integration.py, and pydantic_ai_integration.py each require STACKONE_ACCOUNT_ID (they print "Set STACKONE_ACCOUNT_ID..." and return when absent), not just auth_management.py. Only openai_integration.py actually runs without it, since fetch_tools() discovers accounts from the key. Update the comment so users don't skip the variable and hit failures in the listed examples.</comment>

<file context>
@@ -1,6 +1,8 @@
+# Required for all examples. Account ids are discovered from the key.
 STACKONE_API_KEY=your-stackone-api-key
+
+# Only examples/auth_management.py needs a specific account
 STACKONE_ACCOUNT_ID=your-account-id
 
</file context>
Suggested change
# Only examples/auth_management.py needs a specific account
# Needed by examples that scope to one account (auth_management, langchain_integration, langgraph_integration, pydantic_ai_integration); openai_integration discovers accounts from the key

Comment thread CLAUDE.md Outdated
- Response handling in `_process_response()`
- **Error handling**: `StackOneError`/`StackOneAPIError` and `ToolsetError`/
`ToolsetConfigError`/`ToolsetLoadError` are two **unrelated** hierarchies in
`types.py`. `str(StackOneAPIError)` leads with the server's own message.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: This sentence inaccurately states that str(StackOneAPIError) leads with the server's own message. In the code, every StackOneAPIError message is prefixed with SDK-side text ("MCP request to ... failed with", "Tool 'name' failed:", "Listing accounts at ... failed with") and the server body only follows that prefix. Reword to reflect that the message prefixes the SDK's own context, or drop the claim.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLAUDE.md, line 210:

<comment>This sentence inaccurately states that str(StackOneAPIError) leads with the server's own message. In the code, every StackOneAPIError message is prefixed with SDK-side text ("MCP request to ... failed with", "Tool 'name' failed:", "Listing accounts at ... failed with") and the server body only follows that prefix. Reword to reflect that the message prefixes the SDK's own context, or drop the claim.</comment>

<file context>
@@ -159,28 +190,43 @@ via release-please after a merge to main, never from a developer machine.
-- **File downloads**: non-JSON responses return raw bytes plus metadata, not decoded text
+- **Error handling**: `StackOneError`/`StackOneAPIError` and `ToolsetError`/
+  `ToolsetConfigError`/`ToolsetLoadError` are two **unrelated** hierarchies in
+  `types.py`. `str(StackOneAPIError)` leads with the server's own message.
+- **File downloads**: non-JSON responses return raw bytes plus metadata. The filename
+  comes from an attacker-controllable header and is reduced to a safe basename.
</file context>
Suggested change
`types.py`. `str(StackOneAPIError)` leads with the server's own message.
`types.py`. `str(StackOneAPIError)` prefixes its own context and appends the server's body.

…ucture

Moves the two calling surfaces under Advanced Filtering and drops the Accounts,
Errors and input_schema callout sections, keeping the README to the recommended
path plus reference material.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@willleeney willleeney changed the title refactor!: MCP-only toolset, uv/make toolchain, and a conformance gate refactor!: MCP-only toolset with account discovery, search/execute, and hardened security Sep 17, 2026
…d-written

The LangChain and Pydantic AI integrations run tool calls for you inside the
framework. With raw OpenAI every caller wrote the loop themselves — look up each
tool by name, execute it, catch failures, json.dumps the result, pair it with its
tool_call_id — and it is easy to get wrong. Ours was: the README version omitted
the assistant turn, so OpenAI returned a 400 on the round trip, and the example
carried its own hand-rolled error handling.

Tools.execute_openai_tool_calls(tool_calls) returns the `tool` messages ready to
send back. It pairs with to_openai(): that turns tools into what OpenAI accepts,
this turns what OpenAI returns back into messages for it.

- A failed call becomes an error message the model can read and retry from,
  rather than raising — what the other two adapters already do.
- A call to a tool not in the collection is reported the same way.
- A file download's raw bytes are base64-encoded; json.dumps crashed on them.
- Accepts the openai package's objects or plain dicts, so it needs no extra.

Verified live: the example passes three of three runs, and the README block's
messages are accepted by OpenAI on the follow-up call.

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

4 issues found across 15 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/dependabot.yaml">

<violation number="1" location=".github/dependabot.yaml:42">
P3: The npm `ignore` covers only `@modelcontextprotocol/sdk`, but `zod`, `hono`, and `@hono/mcp` are pinned exactly in `package.json` under the mock-server policy that these pins must never be changed (mock must model what the real API demands). Dependabot will open weekly PRs bumping those three exact pins, churning the very dependencies the repo pins for behavioral stability and raising the risk of silent mock drift. Either add them to `ignore` too, or document in the comment why they are safe to bump (e.g. CI integration tests cover them).</violation>

<violation number="2" location=".github/dependabot.yaml:42">
P2: This ignore rule blocks all Dependabot updates for `@modelcontextprotocol/sdk`, not just major releases. Patch and security updates will never be proposed; add `version-update:semver-major` under `update-types` if only major upgrades must remain blocked.</violation>
</file>

<file name="stackone_ai/tools.py">

<violation number="1" location="stackone_ai/tools.py:1074">
P1: When the catalog contains the same action name for multiple active accounts, this lookup silently executes the last-listed account's tool. Require an account-scoped catalog or reject ambiguous names before dispatching OpenAI calls.</violation>
</file>

<file name="CLAUDE.md">

<violation number="1" location="CLAUDE.md:210">
P2: The new error-handling rule is false for public tool methods: invalid tool arguments raise `ValueError`, and missing optional adapters raise `ImportError`. Either wrap those failures in `StackOneError` or document these exceptions instead of telling callers that only `StackOneError` can escape.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread stackone_ai/tools.py
messages: list[JsonDict] = []
for call in tool_calls or []:
call_id, name, arguments = _read_openai_tool_call(call)
tool = self.get_tool(name)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: When the catalog contains the same action name for multiple active accounts, this lookup silently executes the last-listed account's tool. Require an account-scoped catalog or reject ambiguous names before dispatching OpenAI calls.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stackone_ai/tools.py, line 1074:

<comment>When the catalog contains the same action name for multiple active accounts, this lookup silently executes the last-listed account's tool. Require an account-scoped catalog or reject ambiguous names before dispatching OpenAI calls.</comment>

<file context>
@@ -990,6 +1050,40 @@ def to_openai(self) -> list[JsonDict]:
+        messages: list[JsonDict] = []
+        for call in tool_calls or []:
+            call_id, name, arguments = _read_openai_tool_call(call)
+            tool = self.get_tool(name)
+            if tool is None:
+                result: Any = {"error": f"Unknown tool {name!r}"}
</file context>

Comment thread .github/dependabot.yaml
labels:
- dependencies
ignore:
- dependency-name: "@modelcontextprotocol/sdk"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This ignore rule blocks all Dependabot updates for @modelcontextprotocol/sdk, not just major releases. Patch and security updates will never be proposed; add version-update:semver-major under update-types if only major upgrades must remain blocked.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/dependabot.yaml, line 42:

<comment>This ignore rule blocks all Dependabot updates for `@modelcontextprotocol/sdk`, not just major releases. Patch and security updates will never be proposed; add `version-update:semver-major` under `update-types` if only major upgrades must remain blocked.</comment>

<file context>
@@ -23,6 +24,23 @@ updates:
+    labels:
+      - dependencies
+    ignore:
+      - dependency-name: "@modelcontextprotocol/sdk"
+
   # GitHub Actions
</file context>
Suggested change
- dependency-name: "@modelcontextprotocol/sdk"
- dependency-name: "@modelcontextprotocol/sdk"
update-types:
- version-update:semver-major

Comment thread CLAUDE.md
- Response handling in `_process_response()`
- **Error handling**: everything derives from `StackOneError` (`types.py`) — both
`StackOneAPIError` and the `ToolsetError` family. Nothing outside that hierarchy
should escape a public method. `str(StackOneAPIError)` leads with the server's own

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The new error-handling rule is false for public tool methods: invalid tool arguments raise ValueError, and missing optional adapters raise ImportError. Either wrap those failures in StackOneError or document these exceptions instead of telling callers that only StackOneError can escape.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLAUDE.md, line 210:

<comment>The new error-handling rule is false for public tool methods: invalid tool arguments raise `ValueError`, and missing optional adapters raise `ImportError`. Either wrap those failures in `StackOneError` or document these exceptions instead of telling callers that only `StackOneError` can escape.</comment>

<file context>
@@ -205,9 +205,10 @@ tools = toolset.fetch_tools(providers=["linear"], actions=["*_list_*"])
-  `types.py`. `str(StackOneAPIError)` leads with the server's own message.
+- **Error handling**: everything derives from `StackOneError` (`types.py`) — both
+  `StackOneAPIError` and the `ToolsetError` family. Nothing outside that hierarchy
+  should escape a public method. `str(StackOneAPIError)` leads with the server's own
+  message.
 - **File downloads**: non-JSON responses return raw bytes plus metadata. The filename
</file context>

Comment thread .github/dependabot.yaml
labels:
- dependencies
ignore:
- dependency-name: "@modelcontextprotocol/sdk"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The npm ignore covers only @modelcontextprotocol/sdk, but zod, hono, and @hono/mcp are pinned exactly in package.json under the mock-server policy that these pins must never be changed (mock must model what the real API demands). Dependabot will open weekly PRs bumping those three exact pins, churning the very dependencies the repo pins for behavioral stability and raising the risk of silent mock drift. Either add them to ignore too, or document in the comment why they are safe to bump (e.g. CI integration tests cover them).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/dependabot.yaml, line 42:

<comment>The npm `ignore` covers only `@modelcontextprotocol/sdk`, but `zod`, `hono`, and `@hono/mcp` are pinned exactly in `package.json` under the mock-server policy that these pins must never be changed (mock must model what the real API demands). Dependabot will open weekly PRs bumping those three exact pins, churning the very dependencies the repo pins for behavioral stability and raising the risk of silent mock drift. Either add them to `ignore` too, or document in the comment why they are safe to bump (e.g. CI integration tests cover them).</comment>

<file context>
@@ -23,6 +24,23 @@ updates:
+    labels:
+      - dependencies
+    ignore:
+      - dependency-name: "@modelcontextprotocol/sdk"
+
   # GitHub Actions
</file context>

@StuBehan StuBehan 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.

Ran this locally: 247 tests pass, ruff and ty clean, and py.typed still ships in the wheel (checked, dropping the force-include is fine). pnpm test:python in the conformance repo is a clean PASS too - 11/11, inv 9, inv 10, schema pass-through. Security fixes all look right to me and the comments explaining why are solid.

Three things I'd want before merge, left inline. The schema one is the main one, I don't think the byte-for-byte claim holds against the real server. The other two are that the conformance gate has never actually run in CI - missing secret, and a pnpm setup break sitting behind it.

I'd do the fixture fix in StackOneHQ/sdk-conformance#2 first so the schema regression fails the harness rather than being fixed on trust.

Comment thread stackone_ai/types.py
"""

type: str = Field(description="JSON Schema type")
properties: JsonDict = Field(description="JSON Schema properties")

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.

This only carries type and properties, so to_openai_function() rebuilds the root as {type, properties, required} and anything else at the root goes. UCA emits $schema: 2020-12 at the root of every non-empty tool's inputSchema (input-json-schema.util.ts:264), so we're dropping that on every tool today.

Worse - I fed it a schema with $defs and a property using $ref: "#/$defs/Money". The $ref survives and $defs doesn't, so the model gets a dangling ref that OpenAI strict mode won't take.

Can we keep the served root here and only swap required? Also want to double check what the 139-tool byte-for-byte comparison was actually comparing, feels like it must have been normalising the root away 🤔

Comment thread .github/workflows/ci.yaml
# missing when a job is renamed or a matrix entry is added, and nothing notices.
ci-ok:
if: always()
needs: [gitleaks, ci, conformance]

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.

Dependabot and fork PRs never get repo secrets, so conformance exits 1 on the token check and this needs takes ci-ok down with it. If ci-ok is the single required check then every dependabot PR is blocked forever, which is a shame given we just moved the python ecosystem to uv so they could actually pass. Can we treat "this context can't have the secret" as not-run rather than a failure?

Comment thread .github/workflows/ci.yaml
- name: Setup pnpm
uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0
with:
package_json_file: sdk-conformance/package.json

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.

There's no version: here and sdk-conformance's package.json has no packageManager or devEngines, so action-setup's readTargetVersion throws No pnpm version is specified. We haven't seen it because the token check fails first. Worth fixing on the conformance side (StackOneHQ/sdk-conformance#2) or pinning version: here, otherwise this job still won't run once the secret is set.

Comment thread stackone_ai/tools.py
X-Api-Key, ...) and is wrong the moment one is missed. The previous two-name
list let all of those through.

The allowlist is the served schema itself, so this needs no maintenance: today

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.

On a *_execute_action meta tool self.parameters.properties is action_id/path/query/body/headers, so the headers_* allowlist is always empty and every header gets dropped. Checked it - meta tool keeps {}, an rpc tool with a declared headers_x-custom-tenant keeps it.

Fine as a default. But the comment says it'll work with no SDK release the day an action needs a header, and on the search/execute path it won't. Correct the comment, or build the allowlist from the target action's schema?

Comment thread stackone_ai/tools.py
_account_id: str | None = PrivateAttr(default=None)

@property
def connector(self) -> str:

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.

This still splits on the first _, so browser_linkedin_search_people comes out as browser - the same thing _filter_by_provider and _connector_of got fixed for. get_connectors() feeds off it. Worth pulling the three into one helper?

Comment thread stackone_ai/tools.py
# ALL, not any: under flat_prefixed every parameter is prefixed, so one bare
# name is proof the schema is not. `any` would be satisfied by the very key
# this exists to protect — a declared body field called `path_to_file`.
prefixed = not named or all(_FLAT_ENVELOPE_KEY_PATTERN.match(k) for k in named)

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.

If the server ever serves one property that isn't path|query|body|headers-prefixed, this flips off for the whole schema and every prefixed key falls into the body. Tried it with {path_id, raw_extra} and arg path_id, got body: {path_id: 'abc'} with path empty. Agree all is right over any, but should we log when it flips? Silent is rough on whoever's debugging a missing path param.

Comment thread stackone_ai/toolset.py
if not effective_account_ids and self.account_id:
effective_account_ids = [self.account_id]
if not effective_account_ids:
effective_account_ids = self._discover_account_ids()

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.

This is the headline "api key is enough" path, and it's one MCP listing per active account, 10 at a time. /accounts has no limit on the bare-array branch so we'll see all of them - an org with 50 linked accounts pays 50 round trips on the first fetch_tools() and gets 50x the catalog into the model. Do we want a cap, or at least a note in the README?

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