Skip to content

Fix session-context and authorization refactor regressions#687

Open
badMade wants to merge 6 commits into
mainfrom
fix/session-context-refactor-regressions-12448265710704761371
Open

Fix session-context and authorization refactor regressions#687
badMade wants to merge 6 commits into
mainfrom
fix/session-context-refactor-regressions-12448265710704761371

Conversation

@badMade
Copy link
Copy Markdown
Owner

@badMade badMade commented Jun 1, 2026

This PR fixes a set of regressions introduced by a recent refactor of session context and authorization helpers.

Key changes:

  1. Session Context Compatibility: Restored the expected API for gateway/session_context.py, including keyword-only arguments for set_session_vars and explicit clearing to empty strings in clear_session_vars to suppress environment fallback.
  2. Authorization Fixes: Resolved a NameError: name 'team_id' is not defined in gateway/run.py by safely fetching team_id from the session source.
  3. Webhook Validation: Added a check in gateway/platforms/webhook.py to reject HMAC secrets that appear to be unresolved environment placeholders (e.g., ${WEBHOOK_SECRET}).
  4. Web Tools / DDGS: Restored the _ddgs_package_available alias and updated backend selection logic to prevent DuckDuckGo from being an auto-detected fallback, while maintaining its functionality when explicitly configured.
  5. SSRF Message Normalization: Standardized SSRF-related error strings to "Blocked: ... private or internal network address" across web_tools.py and browser_tool.py, and updated corresponding tests.

Verified with the following test suites:

  • tests/gateway/test_session_env.py
  • tests/gateway/test_unauthorized_dm_behavior.py
  • tests/gateway/test_webhook_adapter.py
  • tests/tools/test_web_providers_ddgs.py
  • tests/tools/test_browser_ssrf_local.py

Fixes #684


PR created automatically by Jules for task 12448265710704761371 started by @badMade

- Restored gateway.session_context APIs (set_session_vars, clear_session_vars, get_terminal_cwd) and ContextVars to fix TypeError and NameError in gateway tests.
- Fixed team_id NameError in gateway/run.py by safely retrieving it from source.
- Implemented unresolved placeholder secret validation in gateway/platforms/webhook.py.
- Restored ddgs tool wiring and package availability checks in tools/web_tools.py.
- Normalized SSRF error messages across web_tools.py and browser_tool.py.
- Updated tests/tools/test_browser_ssrf_local.py to match the normalized SSRF message.
- Verified all fixes with relevant test suites.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@google-labs-jules google-labs-jules Bot mentioned this pull request Jun 1, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

🔎 Lint report: fix/session-context-refactor-regressions-12448265710704761371 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8260 on HEAD, 8262 on base (✅ -2)

🆕 New issues (33):

Rule Count
invalid-argument-type 19
unresolved-attribute 11
unsupported-operator 3
First entries
run_agent.py:4285: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
run_agent.py:9977: [unresolved-attribute] unresolved-attribute: Attribute `lower` is not defined on `dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy`, `int & ~AlwaysFalsy`, `dict[Unknown, Unknown] & ~AlwaysFalsy` in union `(str & ~AlwaysFalsy) | (Unknown & ~AlwaysFalsy) | (dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy) | ... omitted 4 union elements`
tests/conftest.py:433: [unresolved-attribute] unresolved-attribute: Module `gateway.session_context` has no member `_SESSION_USER_NAME`
tests/run_agent/test_provider_attribution_headers.py:90: [unresolved-attribute] unresolved-attribute: Attribute `startswith` is not defined on `dict[str, str]` in union `Unknown | str | Divergent | dict[str, str]`
tests/conftest.py:432: [unresolved-attribute] unresolved-attribute: Module `gateway.session_context` has no member `_SESSION_USER_ID`
run_agent.py:5423: [invalid-argument-type] invalid-argument-type: Argument to function `parse_rate_limit_headers` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
tests/agent/test_codex_cloudflare_headers.py:163: [unresolved-attribute] unresolved-attribute: Attribute `startswith` is not defined on `dict[str, str]` in union `Unknown | str | Divergent | dict[str, str]`
run_agent.py:7763: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_request_timeout` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
tests/agent/test_codex_cloudflare_headers.py:181: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["originator"]` and `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 4 union elements`
run_agent.py:9552: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_profile` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
run_agent.py:13108: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.update_token_counts` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
tests/agent/test_codex_cloudflare_headers.py:163: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str & ~AlwaysFalsy`, `int & ~AlwaysFalsy` in union `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 4 union elements`
tests/run_agent/test_provider_attribution_headers.py:155: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["X-OpenRouter-Cache"]` and `Unknown | str | dict[str, str] | ... omitted 4 union elements`
run_agent.py:9114: [invalid-argument-type] invalid-argument-type: Argument to function `get_transport` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
run_agent.py:14028: [invalid-argument-type] invalid-argument-type: Argument to bound method `ContextCompressor.update_model` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
run_agent.py:3397: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_stale_timeout` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
run_agent.py:2377: [invalid-argument-type] invalid-argument-type: Argument to function `query_ollama_num_ctx` is incorrect: Expected `str`, found `(str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 5 union elements`
tests/run_agent/test_provider_attribution_headers.py:156: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["X-OpenRouter-Cache-TTL"]` and `Unknown | str | dict[str, str] | ... omitted 4 union elements`
run_agent.py:2690: [invalid-argument-type] invalid-argument-type: Argument to function `get_model_context_length` is incorrect: Expected `str`, found `str | dict[str, str] | Any | ... omitted 4 union elements`
tests/conftest.py:431: [unresolved-attribute] unresolved-attribute: Module `gateway.session_context` has no member `_SESSION_THREAD_ID`
run_agent.py:13805: [invalid-argument-type] invalid-argument-type: Argument to function `_pool_may_recover_from_rate_limit` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | ... omitted 3 union elements`
tests/conftest.py:428: [unresolved-attribute] unresolved-attribute: Module `gateway.session_context` has no member `_SESSION_PLATFORM`
tests/conftest.py:429: [unresolved-attribute] unresolved-attribute: Module `gateway.session_context` has no member `_SESSION_CHAT_ID`
run_agent.py:7317: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 4 union elements`
run_agent.py:13571: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 4 union elements`
... and 8 more

✅ Fixed issues (24):

Rule Count
invalid-argument-type 16
unresolved-attribute 3
unsupported-operator 3
unresolved-reference 1
unresolved-import 1
First entries
tests/run_agent/test_provider_attribution_headers.py:90: [unresolved-attribute] unresolved-attribute: Attribute `startswith` is not defined on `dict[str, str]` in union `Unknown | str | dict[str, str]`
run_agent.py:3397: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_stale_timeout` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:7317: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:9114: [invalid-argument-type] invalid-argument-type: Argument to function `get_transport` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/agent/test_codex_cloudflare_headers.py:163: [unresolved-attribute] unresolved-attribute: Attribute `startswith` is not defined on `dict[str, str]` in union `Unknown | str | dict[str, str]`
run_agent.py:2377: [invalid-argument-type] invalid-argument-type: Argument to function `query_ollama_num_ctx` is incorrect: Expected `str`, found `(str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 4 union elements`
run_agent.py:13071: [invalid-argument-type] invalid-argument-type: Argument to function `estimate_usage_cost` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:13574: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown, Unknown] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
gateway/run.py:5544: [unresolved-reference] unresolved-reference: Name `team_id` used when not defined
run_agent.py:11105: [unresolved-import] unresolved-import: Module `gateway.session_context` has no member `get_terminal_cwd`
run_agent.py:2690: [invalid-argument-type] invalid-argument-type: Argument to function `get_model_context_length` is incorrect: Expected `str`, found `str | dict[str, str] | Any | ... omitted 3 union elements`
run_agent.py:13805: [invalid-argument-type] invalid-argument-type: Argument to function `_pool_may_recover_from_rate_limit` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:2633: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 4 union elements`
run_agent.py:9552: [invalid-argument-type] invalid-argument-type: Argument to function `get_provider_profile` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:13024: [invalid-argument-type] invalid-argument-type: Argument to function `normalize_usage` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/run_agent/test_provider_attribution_headers.py:156: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["X-OpenRouter-Cache-TTL"]` and `Unknown | str | dict[str, str] | ... omitted 3 union elements`
tests/agent/test_codex_cloudflare_headers.py:163: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str & ~AlwaysFalsy`, `int & ~AlwaysFalsy` in union `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:13571: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:7146: [invalid-argument-type] invalid-argument-type: Argument to function `_codex_cloudflare_headers` is incorrect: Expected `str`, found `Unknown | str | dict[str, str] | ... omitted 3 union elements`
tests/run_agent/test_provider_attribution_headers.py:155: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["X-OpenRouter-Cache"]` and `Unknown | str | dict[str, str] | ... omitted 3 union elements`
run_agent.py:5423: [invalid-argument-type] invalid-argument-type: Argument to function `parse_rate_limit_headers` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
tests/agent/test_codex_cloudflare_headers.py:181: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["originator"]` and `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:13108: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.update_token_counts` is incorrect: Expected `str | None`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
run_agent.py:2630: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (dict[str, str] & ~AlwaysFalsy) | ... omitted 4 union elements`

Unchanged: 4338 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several updates across the gateway and tools modules. Key changes include adding a check to prevent signature validation using unresolved environment variable placeholders in webhooks, introducing a session-scoped TERMINAL_CWD context variable, refining error messages in the browser tool, and ensuring the DuckDuckGo search backend is only invoked if the ddgs package is available. Feedback on the changes highlights two issues: first, the regular expression in _looks_unresolved_secret does not match placeholders with default values (e.g., ${VAR_NAME:-default}) as described in its docstring; second, directly calling str() on potentially None values in set_session_vars converts them to the literal string "None", which is truthy and could lead to logical errors. Suggestions are provided to address both issues.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread gateway/platforms/webhook.py Outdated
Comment thread gateway/session_context.py Outdated
badMade and others added 2 commits June 1, 2026 18:35
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@badMade badMade marked this pull request as ready for review June 1, 2026 22:35
Copilot AI review requested due to automatic review settings June 1, 2026 22:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses regressions from a recent session-context and authorization refactor by restoring expected session helper behavior, fixing an authorization NameError, hardening webhook secret validation, and re-aligning DDGS backend wiring and SSRF error messaging.

Changes:

  • Restore/extend gateway.session_context compatibility and add TERMINAL_CWD support via ContextVars.
  • Fix authorization pairing logic by safely reading team_id from the session source.
  • Add webhook HMAC secret placeholder detection and adjust DDGS backend selection/availability checks; normalize one SSRF error string and update tests accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
gateway/session_context.py Restores/extends ContextVar-backed session env API and adds get_terminal_cwd.
gateway/run.py Fixes team_id scoping in authorization pairing checks.
gateway/platforms/webhook.py Rejects unresolved placeholder secrets (e.g. ${WEBHOOK_SECRET}) during signature validation.
tools/web_tools.py Prevents DDGS from being auto-detected as fallback, restores _ddgs_package_available alias, and adds ddgs-package guard in search.
tools/browser_tool.py Normalizes one SSRF redirect-block error string.
tests/tools/test_browser_ssrf_local.py Updates assertions to match the normalized browser SSRF error string.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 88 to +92
user_id: str = "",
user_name: str = "",
session_key: str = "",
) -> list:
terminal_cwd: Optional[str] = None,
) -> List:
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@jules fix:

set_session_vars is described in the PR/issue as having keyword-only arguments, but the current signature allows positional arguments. Making the parameters keyword-only would match the intended API and prevent accidental positional misuse.

Comment on lines 93 to 97
"""Set all session context variables and return reset tokens.

Call ``clear_session_vars(tokens)`` in a ``finally`` block to restore
the previous values when the handler exits.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@jules fix:
The set_session_vars docstring says clear_session_vars(tokens) will "restore the previous values", but clear_session_vars actually sets the ContextVars to "" (explicitly cleared) to suppress env fallback. This is misleading for callers and should be updated to reflect the real behavior.

Comment thread gateway/session_context.py Outdated
Comment thread tools/web_tools.py
Comment on lines +210 to +214
def _ddgs_package_available() -> bool:
"""Backward-compatible alias for _ddgs_package_importable."""
return _ddgs_package_importable()


Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@jules fix:

_ddgs_package_importable() currently checks availability by importing ddgs. Import-based availability checks can execute top-level code and can be shadowed by a local ddgs.py on sys.path. Since tools.web_providers.ddgs.ddgs_package_available() already implements a metadata-based, shadowing-safe check, it would be safer to delegate to that instead of importing the module here.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@badMade
Copy link
Copy Markdown
Owner Author

badMade commented Jun 2, 2026

@jules resolve the merge conflicts in this pull request

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

Auto-merge: checks failing

The following checks did not pass:

  • copilot (failure)

Please fix the failing checks before this PR can be merged.

View workflow run

@badMade
Copy link
Copy Markdown
Owner Author

badMade commented Jun 2, 2026

@jules fix:
failing checks
auto-merge
auto-merge — 5 CI check(s) failing.
Tests / test (pull_request)
Tests / test (pull_request)Failing after 13m

- Restored gateway.session_context APIs with keyword-only arguments and improved None handling.
- Restored internal ContextVar names (_SESSION_PLATFORM, etc.) for test compatibility.
- Implemented robust unresolved placeholder secret validation in webhook adapter.
- Switched to shadowing-safe ddgs package check in web tools.
- Preserved <memory-context> tags in assistant stored content while stripping think blocks.
- Added missing _FakeProviderMemoryManager to tests/run_agent/test_run_agent.py.
- Normalized SSRF error messages across tools and updated relevant tests.
- Verified all fixes with full test suite.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@badMade
Copy link
Copy Markdown
Owner Author

badMade commented Jun 2, 2026

@jules resolve the merge conflicts in this pull request

@badMade
Copy link
Copy Markdown
Owner Author

badMade commented Jun 2, 2026

@copilot resolve the merge conflicts in this pull request

This commit restores backward-compatible session and authorization helpers
to resolve NameErrors and TypeErrors introduced by a recent refactor.

Key changes:
- gateway/session_context.py: Restored `set_session_vars` and `clear_session_vars`
  APIs with support for legacy parameters (user_id, user_name, terminal_cwd).
  Ensured `ContextVar` internal names match expectations in `tests/conftest.py`.
- gateway/run.py: Fixed `team_id` NameError by safely retrieving it from source.
- gateway/platforms/webhook.py: Added validation to reject unresolved
  environment placeholders (e.g., `${WEBHOOK_SECRET}`) in HMAC secrets.
- tools/web_tools.py: Restored `ddgs` wiring and normalized SSRF error strings.
- run_agent.py: Fixed regression where `<memory-context>` tags were incorrectly
  persisted in stored transcripts.
- tests/run_agent/test_run_agent.py: Restored missing `_FakeProviderMemoryManager`.

Verified with tests/gateway/test_session_env.py and other relevant suites.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@badMade
Copy link
Copy Markdown
Owner Author

badMade commented Jun 3, 2026

@copilot resolve the merge conflicts in this pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@jules fix:

2 participants