docs: publish authentication and function guides - #170
Conversation
|
@codex review. Please review code and security on the current head. This adds published authentication and function guides using the existing native facade and verified README examples. Docs lint and every Python/Ruby code block parse locally. Pay particular attention to session ownership/revocation, OAuth state validation, invocation credentials, and error classification. No wire, runtime, or shared-scenario changes. Hosting #1127 remains the integration PR and merges last. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b834f998f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ```python | ||
| result = client.auth.sign_up( | ||
| email="new-user@example.com", password="secret", sign_in_when_allowed=True |
There was a problem hiding this comment.
Replace this password and the new-secret literal in the reset example with values of at least 15 characters. The API schema enforces a minimum configurable password length of 15 (openapi/openapi.yaml lines 11490-11495), so the documented signup always fails validation with secret, and the reset workflow likewise fails with its 10-character example.
Useful? React with 👍 / 👎.
| Invoke a deployed function by its name. | ||
| This example assumes a public function named `hello` that accepts a JSON object. |
There was a problem hiding this comment.
When VOLCANO_ANON_KEY has the default auth-only permissions, making hello public is not enough for this example to run: anonymous invocation additionally requires the key's explicit functions.invoke permission (openapi/openapi.yaml lines 2781-2784 and 9652-9655), otherwise resolution or invocation returns 403. The standalone guide should tell users to enable that permission or construct the example client with a user/service credential.
Useful? React with 👍 / 👎.
| On success, `refresh_session()` replaces the in-memory session and returns the immutable new | ||
| snapshot. An authentication failure clears the session that initiated the request. Server and | ||
| transport failures preserve it, and a late response never replaces a newer session. The SDK does |
There was a problem hiding this comment.
This guarantee does not hold for every AuthenticationError: when returned refresh credentials fail the user/session continuity checks, or supplied credentials lack a usable session identifier, refresh_session() raises AuthenticationError while preserving the current session (src/volcano_sdk/auth.py lines 1129-1146). Callers may reasonably use the documented error classification to decide whether local credentials were cleared, so narrow this statement to an authentication rejection from the refresh endpoint and document that local continuity-validation failures preserve the session.
Useful? React with 👍 / 👎.
|
@codex review. Please rerun code and security reviews on the new head. Examples now use passwords longer than the 15-character minimum, explicitly require anonymous functions.invoke permission, consume stored OAuth and hosted-auth callback state once before validation, distinguish server-rejected refreshes from local continuity-validation failures, and limit typed pre-dispatch errors to platform HTTP failures. Matching README guidance was corrected too. Docs lint and all Python/Ruby code blocks parse. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 188a64762f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| session = client.auth.adopt_hosted_auth_session( | ||
| returned_session, | ||
| state=returned_state, | ||
| expected_state=hosted_state, |
There was a problem hiding this comment.
When the redirect and callback are handled by separate requests, this example still passes the generation-time hosted_state rather than the value atomically fetched and deleted from the server-side session. A reader following the code can therefore validate without proving that the stored nonce existed or was consumed, allowing a hosted-auth callback to be replayed despite the preceding guidance. Mirror the OAuth example by assigning the consumed value (for example, stored_hosted_state) and passing that as expected_state.
Useful? React with 👍 / 👎.
| Successful `get_user()`, `update_user()`, `convert_anonymous()`, and | ||
| `confirm_email_change()` calls update that local snapshot without changing tokens | ||
| or emitting an auth-state event. Previously returned sessions remain immutable. |
There was a problem hiding this comment.
When any of these profile operations receives HTTP 401 while a usable refresh token exists, Auth._session_request() refreshes the session before replaying the operation; this replaces both tokens and emits TOKEN_REFRESHED. A successful get_user(), update_user(), convert_anonymous(), or confirm_email_change() can therefore do exactly what this paragraph says it never does, which can cause callers to miss persisting or reacting to refreshed credentials. Qualify this guarantee as applying only when no automatic refresh is needed, or document the refresh side effect.
Useful? React with 👍 / 👎.
|
|
||
| ## Use hosted authentication | ||
|
|
||
| Build a managed hosted-auth URL: |
There was a problem hiding this comment.
For a project using the default configuration, managed_auth_enabled is false and the hosted page returned by this helper responds with HTTP 404. The guide currently moves directly from ordinary authentication setup to constructing the URL, so the hosted-auth workflow fails for readers who have not separately enabled managed hosted pages. State that managed authentication must be enabled before using this section.
Useful? React with 👍 / 👎.
|
@codex review. Please rerun code and security reviews on the new head. Examples now use passwords longer than the 15-character minimum, explicitly require anonymous functions.invoke permission, consume stored OAuth and hosted-auth callback state once before validation, distinguish server-rejected refreshes from local continuity-validation failures, and limit typed pre-dispatch errors to platform HTTP failures. Matching README guidance was corrected too. Docs lint and all Python/Ruby code blocks parse. |
|
@codex review. Please review code and security on the current head. The guides and matching README now qualify credential changes during automatic refresh, document readable session-ID requirements and opaque-token sign-out fallback, use a longer anonymous-conversion password, and state Ruby callback exception behavior precisely. Python Markdown examples also pass Ruff formatting. No runtime or wire changes. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d819bc8675
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| in transition order after the state lock is released, and callback failures cannot interrupt auth | ||
| operations. Unsubscribing prevents queued and future delivery; a callback already selected for |
There was a problem hiding this comment.
Limit this guarantee to ordinary Exception subclasses. _notify_auth_state_change() deliberately re-raises non-Exception BaseException values after dispatch, and test_auth_state_dispatch_recovers_after_a_base_exception confirms that a callback raising KeyboardInterrupt causes sign_in() to raise even though the session transition completed. As written, callers may incorrectly assume that every callback failure is isolated from the initiating auth call.
Useful? React with 👍 / 👎.
| The request uses the current access token. Deleting that token's own session clears local | ||
| credentials, including when the request outcome is uncertain; deleting another session preserves | ||
| them. If another authentication operation replaces the session before deletion finishes, the method |
There was a problem hiding this comment.
When the active access token is opaque or its JWT lacks a readable UUID session_id, delete_session() cannot recognize that the supplied ID is current: session_id_from_access_token() returns None, deletes_current remains false, and a successful server deletion preserves the now-revoked local credentials. This can occur when callers select the is_current entry returned by list_sessions(), so qualify the clearing guarantee—and its matching README text—with the readable-session-ID requirement.
Useful? React with 👍 / 👎.
|
@codex review. Please rerun code and security on the current head. The guides and README now require readable session IDs for supplied-credential refresh and self-deletion recognition, qualify other-session deletion during automatic refresh, specify supplied-token precedence for function calls, and distinguish ordinary callback exceptions from process-control exceptions. Matched these claims against the native auth/session and function implementations; docs lint, code-block syntax, and Python formatting pass. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42ad4e7235
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Without that identifier, the SDK cannot recognize self-deletion. Other deletions retain the | ||
| local session, though automatic HTTP 401 recovery can rotate credentials and emit `TOKEN_REFRESHED`. If another authentication operation replaces the session before deletion finishes, the method |
There was a problem hiding this comment.
When deletion of another session receives HTTP 401 and the refresh endpoint rejects the refresh token, Auth._perform_refresh() clears the captured local session before delete_session() raises the original authentication error. The new qualification covers successful refresh rotation only, so the unconditional statement that other deletions retain the local session remains false for this recovery-failure path; limit it to requests not followed by a rejected refresh or document that rejection clears credentials.
Useful? React with 👍 / 👎.
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review. Please rerun code and security on the current head. Clarified the remaining other-session deletion edge: the deletion itself does not clear local state, while automatic recovery can rotate credentials or clear the captured session when the server rejects refresh. README and guide agree. Formatting and docs lint pass. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex security review. Please rerun security review on current head 0b7c9d0 and report the result. The summary marks the previous security review completed but says finding details are still loading; there is no current-head security result or inline finding visible through the complete GitHub comment/review APIs. Code review and CI are clean. We are holding the merge until the current-head security result is clear. |
|
@codex review. Please review code and security on the new head after rebasing onto merged function recovery #173. The authentication corrections are unchanged; the function guide now documents pre-dispatch 401 recovery, captured session/payload, and non-retry boundaries. The README retains the merged runtime PR guidance. Docs lint, code-block syntax and Ruff formatting pass. |
0b7c9d0 to
e0ab0ca
Compare
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Add published authentication and function guides for the Python SDK. The authentication guide covers the existing facade's account, email, OAuth, profile, session adoption, refresh, and revocation behavior. The function guide explains invocation credentials, endpoint resolution, response values, and platform-versus-function errors. Link both from the quickstart and correct the stale README claim that a version header establishes dispatch.
Docs lint and all 30 native-language code blocks pass syntax validation. Examples were checked against the current handwritten facades; no API, wire contract, or shared scenario changes. Hosting #1127 remains the cumulative parity integration PR and merges last.