Skip to content

fix(ci): clear review label when issues close#5701

Closed
RaresKeY wants to merge 1936 commits into
odysseus-dev:devfrom
RaresKeY:fix/issue-review-label-close
Closed

fix(ci): clear review label when issues close#5701
RaresKeY wants to merge 1936 commits into
odysseus-dev:devfrom
RaresKeY:fix/issue-review-label-close

Conversation

@RaresKeY

Copy link
Copy Markdown
Member

Summary

The issue-description workflow marks complete issues as ready for review, but it does not run when an issue closes and has no closed-state guard. As a result, a closed issue can retain that status, and a later edit can run the normal validation path and restore it.

This adds closed to the existing workflow trigger and makes the existing checker remove ready for review and return for any closed issue. Reopening still uses the existing validation path, so the appropriate status label is restored automatically. Focused regressions cover both the close event and a later edit to a closed issue. This does not remove needs more info on closure or perform a historical label backfill; those remain separate policy/maintenance choices.

PR #5486 is related because it refactors shared helpers in the same description-check scripts, but it intentionally preserves behavior and does not implement this lifecycle fix.

Target branch

  • This PR targets dev, not main. All PRs land in dev; main is curated by the maintainer at each release. If your PR is on main by accident, click "Edit" on this PR and change the base.

Linked Issue

Fixes #5700

Type of Change

  • Bug fix (non-breaking - fixes a confirmed issue)
  • New feature (non-breaking - adds new behaviour)
  • Breaking change (changes or removes existing behaviour)
  • Refactor / cleanup (behaviour unchanged)
  • Documentation only
  • CI / tooling / configuration

Checklist

  • I searched open issues and open PRs - this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above - no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.

The app was not run because this change is isolated to a repository GitHub Actions workflow. The checker was exercised through a mocked Octokit event harness instead.

How to Test

  1. Run pytest -q tests/test_issue_description_check.py; the workflow-trigger assertion and both closed-issue event scenarios should pass.
  2. Run node --check .github/scripts/check-issue-description.js.
  3. In GitHub, close an issue carrying ready for review and confirm the label is removed. Edit the issue while it remains closed and confirm the label stays removed. Reopen it and confirm the existing validation flow restores either ready for review or needs more info from the current description.

Visual / UI changes - REQUIRED if you touched anything that renders

N/A - repository automation only; no UI rendering changed.

  • Screenshot or short clip of the change in the running app, attached below. Mobile screenshot too if the change affects mobile.
  • Style match: the change uses Odysseus's existing visual language.
  • No new component patterns. If a similar widget already exists in the app, extend it instead of writing a parallel one.
  • I am not an LLM agent submitting a bulk PR. If you are, please open an issue describing the problem first - bulk auto-generated PRs that don't match the project's visual style are closed on sight, even when the underlying fix is correct.

Screenshots / clips

N/A - no visual changes.

pewdiepie-archdaemon and others added 30 commits June 27, 2026 21:25
* fix(security): prevent ReDoS in XML and args tool-call parsers

Four py/polynomial-redos sinks in tool_parsing.py ran lazy/greedy regexes over
untrusted model output (tool-call markup is attacker-influenced via prompt
injection). When the closing delimiter was absent, each rescanned to
end-of-string from every opener -> O(n^2):

  - args => { ... } in _parse_tool_call_block: greedy \{([\s\S]*)\} restarted
    from every `args:{` opener. Now finds the opener once and takes through the
    last `}` (rfind) — equivalent capture, O(n).
  - _XML_INVOKE_RE: lazy <invoke ...>([\s\S]*?)</invoke>. Now _iter_xml_invoke
    pairs each opener with the first reachable </invoke> and stops when none is.
  - _XML_DIRECT_TOOL_RE and the <tag>([\s\S]*?)</\1> param scan in
    _parse_tool_code_block: lazy backreference patterns. Now _iter_backref_blocks
    pairs each opener with the nearest matching closer and memoizes tag names
    with no remaining closer, so an opener flood stays O(n).

All four are output-equivalent to the originals on well-formed tool-call markup;
the lazy patterns remain defined (still re-exported via agent_tools) but no
longer drive a finditer over untrusted text. Adds tests/test_redos_xml_tool_parsers.py
pinning correctness and bounding the opener-flood inputs (old paths took 4-15s).

* fix(security): harden invoke-parameter and distinct-name tag scans

Forward-only the two residual ReDoS paths in the XML/tool parsers that the
outer-delimiter fix left quadratic:

- _parse_xml_invoke parsed <parameter> with _XML_PARAM_RE.finditer, so a
  closed <invoke> body full of unclosed <parameter> openers rescanned the
  body from every opener (O(n^2), ~11s at 8k openers). Now scans forward-only
  via _iter_named_blocks, factored out of _iter_xml_invoke.
- _iter_backref_blocks only memoized repeated missing tag names; a flood of
  distinct unclosed names searched the suffix once per name (O(n^2)). It now
  indexes every closer by name in one linear pass and binary-searches per
  opener (O(n log n)). Covers the direct and tool_code backref scans.

Output-equivalent to the prior scanners (200k randomized trials match the
memoized version for both the direct ci=True and tool_code ci=False configs).
Adds regressions for the closed-invoke parameter flood and the distinct-name
floods (45k openers now run in ~0.05s, were 5-6s).
bitboody and others added 27 commits July 13, 2026 08:56
…mporter (#5261)

* Harden skill importer against SSRF: block private targets + revalidate redirects per hop

The skill importer validated only the initial URL with the lenient SSRF guard
(block_private=False) and then fetched with follow_redirects=True, so a 3xx to
an internal/metadata address (169.254.169.254, 127.0.0.1, RFC-1918) was still
connected to — inconsistent with the hardened services/search/content.py
:_get_public_url path.

Add a _get_checked() helper that follows redirects manually and re-runs the
SSRF guard with block_private=True on every hop, and route all three fetch
sites (skills.sh unwrap, _fetch_bytes, _list_github_dir) through it. GitHub's
own redirects and the final-host _assert_github_url checks are preserved.

Adds hermetic regression tests (IP-literal hosts, faked HTTP layer) and updates
the existing mock signature for the new block_private kwarg.

Defense-in-depth: the endpoint is admin-gated (require_admin) and admins are
trusted per THREAT_MODEL.md, so this is not a cross-boundary vulnerability.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: enforce follow_redirects=False invariant in mock client

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Add Arch-specific package installation and NVIDIA runtime configuration
to the Docker setup guide. Cover passthrough verification, the NVIDIA
Compose overlay, and the distinction between GPU passthrough and
CUDA-backed model serving

Refs #831
chore: sync upstream changes from odysseus-dev/odysseus
fix(docker): bump Docker CLI to a patched release
fix(docker): bump Docker CLI to a patched release
* feat(models): define capability schema and readers

* fix(models): harden Google catalog probing

Restrict native catalog probing to the Gemini host, keep provider keys out of request URLs, filter non-chat model resources, and preserve the manual refresh default in the built-in Google add flow.
…(#5474)

* security(url-safety): reject RFC 6598 shared address space in strict mode

Strict mode (block_private=True) is a full SSRF lockdown, but it only
rejected is_private and is_loopback targets. CPython does not classify RFC
6598 shared/CGNAT space (100.64.0.0/10) as is_private (it is "shared", not
"private"), so a public redirect into 100.64.0.1 passed the per-hop guard
and still issued the request to a potentially internal CGNAT service.

not is_global would also exclude it, but only on CPython 3.11.10+/3.12.4+/
3.13+; the CI matrix runs 3.11/3.12, so reject the range explicitly to stay
correct across patch levels and the 3.14 runtime image. Default local-first
mode is unchanged. Adds strict-mode coverage for shared, non-global, and
public targets.

* docs(url-safety): correct CGNAT is_global rationale in strict-mode comment

The prior comment claimed `not is_global` catches 100.64.0.0/10 only on
CPython 3.11.10+/3.12.4+/3.13+. That is inaccurate for CGNAT: is_global
is False for 100.64.0.1 on every supported version (verified 3.10-3.14).
The version-fragility applies to other ranges gh-113171 touched, not CGNAT.
The explicit range reject is still the right choice; restate the reason as
is_private not covering shared space, and not coupling strict mode to
is_global's broader, cross-version definition. No behavior change.
…… (#5491)

* fix(llm): enhance fallback logic to handle empty completions and improve metadata handling

* fix(llm): stream tool call deltas immediately
Slice 2f of the route-domain reorganization (#4082/#4071, per
specs/architecture-runtime-inventory.md §6.3). Moves note_routes.py into
routes/note/, leaving a backward-compat sys.modules shim at the old path.
Pure file reorganization, no behavior change.

The shim uses sys.modules replacement (same pattern as the merged gallery
#4903, research #4975, memory #5007, history #5090, and contacts #5227
slices) so that `import routes.note_routes`, `from routes.note_routes import
X`, `importlib.import_module(...)`, and the `import ... as note_routes` +
`monkeypatch.setattr(note_routes, "SessionLocal", ...)` pattern used by
test_note_reminder_fire_scope.py / test_notes_fail_closed_auth.py all
operate on the same module object the application uses.

The canonical module does NOT depend on the shim — routes/note/note_routes.py
imports only from core/, src/, and stdlib. The outbound email cross-domain
imports (routes.email_routes._get_email_config, routes.email_helpers.
_send_smtp_message) are function-local lazy imports that keep resolving
through the email module's own path (email is not yet migrated).

One source-introspection test site repointed to the new canonical path:
- test_model_helper_owner_scope.py (shared with history; history entry
  already repointed in #5090, note entry repointed here)

Adds tests/test_note_routes_shim.py to pin the sys.modules shim contract
(legacy and canonical paths resolve to the same module object; monkeypatch
via legacy alias reaches the canonical module).

Verified: compileall clean; full suite 4487 passed, 3 skipped.
… (#5658)

Slice 2g of the route-domain reorganization (#4082/#4071). Moves
cleanup_routes.py into routes/cleanup/, leaving a backward-compat
sys.modules shim at the old path. Pure file reorganization, no behavior
change.

The shim uses sys.modules replacement so string-targeted
monkeypatch.setattr("routes.cleanup_routes.*", ...) in
test_cleanup_owner_scope.py reaches the canonical module.

Canonical module imports only from src/ and stdlib (zero internal
routes/ coupling). Zero source-introspection landmines.

Adds tests/test_cleanup_routes_shim.py to pin the sys.modules shim
contract. Verified: compileall clean; targeted tests pass.
…ackage (#5659)

Slice 2h of the route-domain reorganization (#4082/#4071). Moves
admin_wipe_routes.py into routes/admin_wipe/, leaving a backward-compat
sys.modules shim at the old path. Pure file reorganization, no behavior
change.

The shim uses sys.modules replacement so the `import ... as
admin_wipe_routes` + `monkeypatch.setattr(admin_wipe_routes, "SessionLocal",
...)` / `"require_admin"` pattern in test_admin_wipe_gallery.py reaches
the canonical module.

Canonical module imports only from core/, src/, and stdlib (zero internal
routes/ coupling). Zero source-introspection landmines.

Adds tests/test_admin_wipe_routes_shim.py to pin the sys.modules shim
contract. Verified: compileall clean; targeted tests pass.
… (#5660)

Slice 2i of the route-domain reorganization (#4082/#4071). Moves
compare_routes.py into routes/compare/, leaving a backward-compat
sys.modules shim at the old path. Pure file reorganization, no behavior
change.

The shim uses sys.modules replacement so the `import ... as cr` +
`monkeypatch.setattr(cr, "SessionLocal", ...)` / `"_owned_endpoint_by_url"`
/ `"_owned_endpoint_by_id"` pattern in test_endpoint_owner_scope_followup.py
reaches the canonical module.

Canonical module imports only from core/, src/, and routes.session_routes
(zero dependency on the legacy shim). One source-introspection test site
repointed: test_endpoint_owner_scope_followup.py (shared with other domains;
only the compare entry repointed here).

Adds tests/test_compare_routes_shim.py to pin the sys.modules shim
contract. Verified: compileall clean; targeted tests pass.
fix(docker): detect snap+WSL2 GPU passthrough incompatibility
* ci: add CodeQL advanced setup to scan pull requests before merge

* ci(codeql): preserve scheduled scans

Add a weekly advanced-setup scan and update the security CI guide so default setup remains disabled.

---------

Co-authored-by: RaresKeY <158580472+RaresKeY@users.noreply.github.com>
* fix(config): forward Google OAuth env vars into Docker container and document setup

GOOGLE_OAUTH_CLIENT_ID, GOOGLE_OAUTH_CLIENT_SECRET, and GOOGLE_OAUTH_REDIRECT_URI
were read by the app but never forwarded through docker-compose.yml's explicit
environment allowlist, causing the "not set" error even when the vars existed in .env.

Also adds a documented section to .env.example with step-by-step GCP setup instructions
so users know where to get the credentials.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(config): cover OAuth in standalone compose files

* test(config): parse OAuth compose service env

* test(config): keep checkout skip wording neutral

---------

Co-authored-by: TNTBA <trynottobreakanything@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(email): use XOAUTH2 in test-connection for Google OAuth accounts

The test-connection endpoint was password-only and had no awareness of
OAuth accounts. For Google-connected accounts this caused two failures:
- IMAP: "Need IMAP host, username, and password" because imap_pass is
  empty (no password is stored for OAuth accounts)
- SMTP: 535 BadCredentials because smtp.login() was called with an
  empty password instead of an XOAUTH2 token

Fix: include oauth_provider and token fields in saved_body when hydrating
from the DB, then use conn.authenticate("XOAUTH2") / smtp.auth("XOAUTH2")
for Google accounts in both the IMAP and SMTP test paths, mirroring what
_send_smtp_message already does for real sends.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(email): add OAuth2 tests for test-connection endpoint

Covers the XOAUTH2 changes made to routes/email_routes.py:
- Google OAuth accounts must not be rejected with 'Need IMAP host,
  username, and password' (no stored password for OAuth accounts)
- IMAP and SMTP test paths must use conn.authenticate('XOAUTH2')
  for Google accounts
- Password accounts must still use conn.login() / smtp.login()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(email): bind OAuth account tests to Google transport

---------

Co-authored-by: TNTBA <trynottobreakanything@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Closed issues keep the ready for review label