feat(sdk): allow graph creation without explicit secrets - #861
Open
breken-ai wants to merge 1 commit into
Open
Conversation
`upsert_graph` required a `secrets` mapping even for a graph that uses none, so every
caller passed `{}` (FailproofAI#631). The parameter is now optional (`dict[str, str] | None = None`).
The wire contract is unchanged, on purpose: the state manager's upsert request requires
the `secrets` field and `GraphTemplate.set_secrets` replaces the stored secrets on every
upsert, so an omitted argument is sent as an empty mapping and the docstring says that
omitting it when updating a graph that already has secrets clears them. Preserving them
from the SDK is not possible (the API never returns secret values), so that would be a
server contract change and is not made here. The body carries a copy of the caller's
mapping, never the mapping itself; there is no shared mutable default.
Regression: tests/test_statemanager_optional_secrets.py — omission sends `{}` and
succeeds; an explicit mapping is sent unchanged and not mutated; repeated calls share no
state; positional binding is unchanged. 3 of 4 fail before the change (TypeError on
omission; the body carried the caller's own dict), all pass after.
Closes FailproofAI#631
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #631.
What changes
StateManager.upsert_graph(graph_name, graph_nodes, secrets=None, …): thesecretsmapping is optional. A graph that needs none is created without passing{}.What deliberately does not change
The wire contract. The state manager's
UpsertGraphTemplateRequestrequiressecrets, andGraphTemplate.set_secretsreplaces a graph's stored secrets on every upsert. So an omitted argument is sent as an empty mapping, exactly what callers send today, and the docstring and README say in one sentence that omitting it when updating a graph that already has secrets clears them. Preserving them client-side is not possible (the API returns only secret names), so that would be a server change and is out of scope here. The body carries a copy of the caller's mapping (it was passed by reference before), and there is no shared mutable default.Tests
python-sdk/tests/test_statemanager_optional_secrets.py, against a mockedaiohttpsession that captures the PUT body:"secrets": {}on the wire, call succeedsBefore: 3 failed, 1 passed (
TypeError: missing 1 required positional argument: 'secrets'; body was the caller's own dict). After: 4 passed; full SDK suite 178 passed;ruff check .clean.Evidence record