From ac449612af9aa0aa1844caf2845fe49a0c52025a Mon Sep 17 00:00:00 2001 From: PrinceThummar011 Date: Thu, 1 Oct 2026 17:57:58 +0530 Subject: [PATCH 01/36] fix(mcp): handle expired/unknown OAuth state in callback with dedicated expired page (#5598) --- routes/mcp/mcp_routes.py | 34 ++++++- tests/test_mcp_oauth.py | 186 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 4 deletions(-) diff --git a/routes/mcp/mcp_routes.py b/routes/mcp/mcp_routes.py index 0e2b30276..385cf09e3 100644 --- a/routes/mcp/mcp_routes.py +++ b/routes/mcp/mcp_routes.py @@ -487,19 +487,32 @@ def setup_mcp_routes(mcp_manager: McpManager): db.close() @router.get("/oauth/callback") - async def oauth_callback(code: str, state: str, request: Request): + async def oauth_callback(code: str = "", state: str = "", request: Request = None): """Handle OAuth callback. Generic MCP OAuth flows resolve via the pending-state registry; Google flows fall through to the legacy path.""" - require_admin(request) from src.mcp_oauth import resolve_pending - if resolve_pending(state, code): + if state and code and resolve_pending(state, code): return HTMLResponse(_oauth_result_page( "Authorization Successful", "The MCP server is connecting. You can close this window and return to Odysseus.", success=True, )) + # Legacy Google path: state is the server_id - return await _exchange_and_connect(state, code, request) + if state: + db = SessionLocal() + try: + srv = db.query(McpServer).filter(McpServer.id == state).first() + if srv and srv.oauth_config: + require_admin(request) + return await _exchange_and_connect(state, code, request) + finally: + db.close() + + return HTMLResponse(_oauth_result_page( + "Authorization Expired", + "This authorization round has expired. Press Reconnect in Odysseus and try again.", + ), status_code=400) @router.post("/oauth/exchange/{server_id}") async def oauth_exchange(server_id: str, request: Request, callback_url: str = Form(...)): @@ -525,6 +538,19 @@ def setup_mcp_routes(mcp_manager: McpManager): success=True, )) + db = SessionLocal() + try: + srv = db.query(McpServer).filter(McpServer.id == server_id).first() + if not srv: + return HTMLResponse(_oauth_result_page("Error", "Server not found."), status_code=404) + if not srv.oauth_config: + return HTMLResponse(_oauth_result_page( + "Authorization Expired", + "This authorization round has expired. Press Reconnect in Odysseus and try again.", + ), status_code=400) + finally: + db.close() + return await _exchange_and_connect(server_id, code, request) async def _exchange_and_connect(server_id: str, code: str, request: Request): diff --git a/tests/test_mcp_oauth.py b/tests/test_mcp_oauth.py index 6a8e21876..24f52820e 100644 --- a/tests/test_mcp_oauth.py +++ b/tests/test_mcp_oauth.py @@ -264,3 +264,189 @@ def test_launchers_export_the_port_they_serve_on(): text = (_repo_root() / name).read_text(encoding="utf-8") assert uvicorn_flag in text, f"{name}: launcher no longer passes {uvicorn_flag}" assert export in text, f"{name}: serves on a port the app cannot read back" + + +# ── Callback & Exchange Expired State Handling ─────────────────── + +def test_oauth_callback_expired_state_returns_dedicated_expired_page(monkeypatch): + """An unknown or expired generic MCP OAuth state in the callback must return + the dedicated 'Authorization Expired' page instead of a bare 403 'Admin only' + or a 404 'Server not found'.""" + from unittest.mock import MagicMock + from fastapi import FastAPI, HTTPException + from fastapi.testclient import TestClient + from routes.mcp import mcp_routes + + # Simulate unauthenticated request that would fail admin check if invoked + def _deny_admin(request): + raise HTTPException(403, "Admin only") + + monkeypatch.setattr(mcp_routes, "require_admin", _deny_admin) + + # Empty DB / no matching server + class FakeQuery: + def filter(self, *a): + return self + + def first(self): + return None + + class FakeSession: + def query(self, *a): + return FakeQuery() + + def close(self): + pass + + monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: FakeSession()) + + app = FastAPI() + manager = MagicMock() + router = mcp_routes.setup_mcp_routes(manager) + app.include_router(router) + client = TestClient(app) + + # Both code and state are provided, but state is not in _pending (expired/unknown) + resp = client.get("/api/mcp/oauth/callback?code=some-code&state=expired-state-123") + assert resp.status_code == 400 + assert "Authorization Expired" in resp.text + assert ( + "This authorization round has expired. Press Reconnect in Odysseus and try again." + in resp.text + ) + # Ensure it did not return the bare 403 "Admin only" + assert "Admin only" not in resp.text + + +def test_oauth_callback_active_pending_state_succeeds_without_admin(monkeypatch): + """An active pending generic MCP OAuth state resolves without requiring admin auth.""" + from unittest.mock import MagicMock + from fastapi import FastAPI, HTTPException + from fastapi.testclient import TestClient + from routes.mcp import mcp_routes + + # Admin check fails if invoked + def _deny_admin(request): + raise HTTPException(403, "Admin only") + + monkeypatch.setattr(mcp_routes, "require_admin", _deny_admin) + + app = FastAPI() + manager = MagicMock() + router = mcp_routes.setup_mcp_routes(manager) + app.include_router(router) + client = TestClient(app) + + async def go(): + fut = mcp_oauth.register_pending("pending-round-1") + resp = client.get("/api/mcp/oauth/callback?code=auth-code-xyz&state=pending-round-1") + assert resp.status_code == 200 + assert "Authorization Successful" in resp.text + code, state = await asyncio.wait_for(fut, timeout=1) + assert code == "auth-code-xyz" + assert state == "pending-round-1" + + asyncio.run(go()) + + +def test_oauth_callback_legacy_google_server_requires_admin(monkeypatch): + """Legacy Google OAuth callbacks (where state is server_id of a server with + oauth_config) still enforce require_admin.""" + import json + from unittest.mock import MagicMock + from fastapi import FastAPI, HTTPException + from fastapi.testclient import TestClient + from routes.mcp import mcp_routes + from core.database import McpServer + + # Server exists in DB with Google oauth_config + google_srv = McpServer( + id="google-srv-1", + name="Google Workspace", + oauth_config=json.dumps({"provider": "google", "keys_file": "k.json", "token_file": "t.json"}), + ) + + class FakeQuery: + def filter(self, *a): + return self + + def first(self): + return google_srv + + class FakeSession: + def query(self, *a): + return FakeQuery() + + def close(self): + pass + + monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: FakeSession()) + + # When require_admin denies, 403 is raised for the legacy Google path + def _deny_admin(request): + raise HTTPException(403, "Admin only") + + monkeypatch.setattr(mcp_routes, "require_admin", _deny_admin) + + app = FastAPI() + manager = MagicMock() + router = mcp_routes.setup_mcp_routes(manager) + app.include_router(router) + client = TestClient(app) + + resp = client.get("/api/mcp/oauth/callback?code=code-123&state=google-srv-1") + assert resp.status_code == 403 + assert "Admin only" in resp.text + + +def test_oauth_exchange_expired_generic_state_returns_expired_page(monkeypatch): + """Remote paste-back flow for a generic MCP server returns the dedicated expired + page when the state is no longer pending, instead of 'No OAuth config.'.""" + from unittest.mock import MagicMock + from fastapi import FastAPI + from fastapi.testclient import TestClient + from routes.mcp import mcp_routes + from core.database import McpServer + + monkeypatch.setattr(mcp_routes, "require_admin", lambda r: None) + + # Server exists in DB without oauth_config (generic MCP server) + generic_srv = McpServer( + id="http-srv-1", + name="Remote HTTP MCP", + oauth_config=None, + ) + + class FakeQuery: + def filter(self, *a): + return self + + def first(self): + return generic_srv + + class FakeSession: + def query(self, *a): + return FakeQuery() + + def close(self): + pass + + monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: FakeSession()) + + app = FastAPI() + manager = MagicMock() + router = mcp_routes.setup_mcp_routes(manager) + app.include_router(router) + client = TestClient(app) + + resp = client.post( + "/api/mcp/oauth/exchange/http-srv-1", + data={"callback_url": "http://localhost:7000/api/mcp/oauth/callback?code=code-1&state=expired-state"}, + ) + assert resp.status_code == 400 + assert "Authorization Expired" in resp.text + assert ( + "This authorization round has expired. Press Reconnect in Odysseus and try again." + in resp.text + ) + assert "No OAuth config." not in resp.text From 35e3b298d8f84a68f147053e8367121b06768b09 Mon Sep 17 00:00:00 2001 From: ShahabAhmed01 <192491541+ShahabAhmed01@users.noreply.github.com> Date: Sat, 3 Oct 2026 23:23:36 +0500 Subject: [PATCH 02/36] fix(scheduler): fall back to defaults for non-numeric research settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scheduled-research task reads research_max_tokens, research_extraction_timeout_seconds, and research_extraction_concurrency with bare int() casts and no guard. A hand-edited or agent-written data/settings.json value like {"research_max_tokens": "invalid"} raised ValueError and killed every scheduled research run until settings.json was fixed by hand (#5049). src/research_handler.py already guards the same reads defensively (_bounded_int); this mirrors that for the scheduler. Wrap the three reads in try/except (TypeError, ValueError) falling back to the documented defaults 8192 / 90 / 3. Valid values (numeric strings like "4096") still parse; the change is limited to the scheduler's reads (agent_loop sites are tracked separately in #5051). Child of maintainer subtracker #5335 (RaresKeY) whose acceptance criteria state: "Non-numeric agent/scheduler settings fall back safely instead of crashing or silently degrading." Prior PR #5050 attempted the same fix but was closed unmerged in the 2026-07-23 repo-transfer bulk close and never resubmitted — redoing it with credit. Fixes #5049. --- src/task_scheduler.py | 36 ++++++++- .../test_scheduler_research_settings_guard.py | 79 +++++++++++++++++++ 2 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 tests/test_scheduler_research_settings_guard.py diff --git a/src/task_scheduler.py b/src/task_scheduler.py index f2f59d65e..89c9217fd 100644 --- a/src/task_scheduler.py +++ b/src/task_scheduler.py @@ -342,6 +342,36 @@ def _normalize_chat_endpoint(url: str) -> str: return url +def _resolve_research_settings(get_setting_fn): + """Resolve the scheduled-research settings with crash-proof fallbacks. + + A hand-edited or agent-written data/settings.json can hold non-numeric + values for the research_* keys; the bare int() reads previously raised + inside TaskScheduler._execute_research_task and killed every scheduled + research run until settings.json was fixed by hand (#5049). + src/research_handler.py already guards the same reads defensively — + this mirrors that for the scheduler. Valid values pass through + unchanged (numeric strings like "4096" still parse). + """ + try: + max_tokens = int(get_setting_fn("research_max_tokens", 8192)) + except (TypeError, ValueError): + max_tokens = 8192 + try: + extraction_timeout = int( + get_setting_fn("research_extraction_timeout_seconds", 90) or 90 + ) + except (TypeError, ValueError): + extraction_timeout = 90 + try: + extraction_concurrency = int( + get_setting_fn("research_extraction_concurrency", 3) or 3 + ) + except (TypeError, ValueError): + extraction_concurrency = 3 + return max_tokens, extraction_timeout, extraction_concurrency + + class TaskScheduler: def __init__(self, session_manager): self._session_manager = session_manager @@ -2068,9 +2098,9 @@ class TaskScheduler: except Exception: pass - max_tokens = int(get_setting("research_max_tokens", 8192)) - extraction_timeout = int(get_setting("research_extraction_timeout_seconds", 90) or 90) - extraction_concurrency = int(get_setting("research_extraction_concurrency", 3) or 3) + max_tokens, extraction_timeout, extraction_concurrency = ( + _resolve_research_settings(get_setting) + ) researcher = DeepResearcher( llm_endpoint=endpoint_url, diff --git a/tests/test_scheduler_research_settings_guard.py b/tests/test_scheduler_research_settings_guard.py new file mode 100644 index 000000000..0cdfa2b87 --- /dev/null +++ b/tests/test_scheduler_research_settings_guard.py @@ -0,0 +1,79 @@ +"""Guarded resolution of scheduled-research settings (#5049). + +The scheduler reads ``research_max_tokens`` / +``research_extraction_timeout_seconds`` / ``research_extraction_concurrency`` +from settings.json with bare ``int()`` casts. A hand-edited value like +``{"research_max_tokens": "invalid"}`` used to raise inside +``TaskScheduler._execute_research_task`` and kill every scheduled research +run until settings.json was fixed by hand. + +``_resolve_research_settings`` must fall back to the documented defaults +(8192 / 90 / 3) for non-numeric input while preserving valid-value behavior +(numeric strings still parse). ``src/research_handler.py`` already guards the +same reads defensively; this is the scheduler's equivalent. +""" + +from __future__ import annotations + +from src.task_scheduler import _resolve_research_settings + + +def _fake_get_setting(values): + """A get_setting stand-in backed by a plain dict.""" + + def get(key, default=None): + return values.get(key, default) + + return get + + +def test_non_numeric_values_fall_back_to_documented_defaults(): + get = _fake_get_setting( + { + "research_max_tokens": "invalid", + "research_extraction_timeout_seconds": "bogus", + "research_extraction_concurrency": "not-a-number", + } + ) + + assert _resolve_research_settings(get) == (8192, 90, 3) + + +def test_none_values_fall_back_to_documented_defaults(): + get = _fake_get_setting( + { + "research_max_tokens": None, + "research_extraction_timeout_seconds": None, + "research_extraction_concurrency": None, + } + ) + + assert _resolve_research_settings(get) == (8192, 90, 3) + + +def test_numeric_strings_still_parse(): + get = _fake_get_setting( + { + "research_max_tokens": "4096", + "research_extraction_timeout_seconds": "45", + "research_extraction_concurrency": "2", + } + ) + + assert _resolve_research_settings(get) == (4096, 45, 2) + + +def test_missing_keys_keep_documented_defaults(): + assert _resolve_research_settings(_fake_get_setting({})) == (8192, 90, 3) + + +def test_valid_ints_pass_through(): + get = _fake_get_setting( + { + "research_max_tokens": 2048, + "research_extraction_timeout_seconds": 120, + "research_extraction_concurrency": 6, + } + ) + + assert _resolve_research_settings(get) == (2048, 120, 6) From a0137b47d16ffa4288860903ddbb160346f8c7d9 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Thu, 15 Oct 2026 11:16:52 +0100 Subject: [PATCH 03/36] docs(maintainers): publish governance handbook draft and review playbook Adapt the agreed maintainer responsibilities and risk-based review policy for the public repository. Add evidence-driven security review, CodeQL triage, release guidance and a discoverable contributing link. --- CODE_OF_CONDUCT.md | 33 ++++++ CONTRIBUTING.md | 4 + docs/maintainers/README.md | 40 ++++++++ docs/maintainers/changes-and-review.md | 62 ++++++++++++ docs/maintainers/documentation.md | 31 ++++++ docs/maintainers/product-and-features.md | 42 ++++++++ docs/maintainers/review-playbook.md | 124 +++++++++++++++++++++++ docs/maintainers/roles-and-access.md | 40 ++++++++ docs/maintainers/testing-and-releases.md | 46 +++++++++ docs/maintainers/triage.md | 48 +++++++++ 10 files changed, 470 insertions(+) create mode 100644 CODE_OF_CONDUCT.md create mode 100644 docs/maintainers/README.md create mode 100644 docs/maintainers/changes-and-review.md create mode 100644 docs/maintainers/documentation.md create mode 100644 docs/maintainers/product-and-features.md create mode 100644 docs/maintainers/review-playbook.md create mode 100644 docs/maintainers/roles-and-access.md create mode 100644 docs/maintainers/testing-and-releases.md create mode 100644 docs/maintainers/triage.md diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 000000000..f79c0be5b --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,33 @@ +# Code of Conduct + +Odysseus project spaces should remain respectful, useful, and focused on the project. Technical disagreement is welcome; personal hostility is not. + +## Expected conduct + +- Be professional and constructive. Critique ideas, evidence, and implementations, not people. +- Treat people fairly regardless of experience, background, identity, or role. +- Explain disagreement with reproducible evidence when possible, and acknowledge uncertainty. +- Keep project spaces relevant to Odysseus, software engineering, AI, and project work. +- Help de-escalate discussions that stop being productive, and respect moderation decisions while they are reviewed. + +## Unacceptable conduct + +- Harassment, discrimination, threats, intimidation, or personal attacks. +- Trolling, baiting, sustained disruption, or targeted hostility. +- Doxxing, publishing private correspondence or personal information without permission. +- Political campaigning, ideological or culture-war disputes, and repeated highly polarizing arguments unrelated to project work. +- Retaliation against someone who reports a concern in good faith. + +These expectations apply to everyone, including administrators and maintainers. Disagreeing about project decisions or criticizing code in good faith is not a violation. + +## Scope + +This applies across official Odysseus spaces, including GitHub, Matrix, Plane, maintainer channels, and other project-managed community spaces, and to representatives acting on the project's behalf. + +## Reporting and enforcement + +Report conduct concerns to a Repository Admin or Organization Owner using an appropriate project communication channel. For sensitive concerns, ask for a private channel rather than posting personal details publicly. Security vulnerabilities follow [SECURITY.md](SECURITY.md), not the public conduct-reporting process. + +Maintainers may redirect or pause unproductive conversations. Repository Admins handle serious moderation or access decisions, consulting others when appropriate and avoiding conflicts of interest. + +Actions may include a reminder, warning, temporary restriction, removal from project spaces, or removal of elevated access. Responses should be proportionate to the behavior and circumstances, with privacy respected and an opportunity to provide context where practical. Retaliation is prohibited. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 38586845f..2934094d2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,6 +62,10 @@ docker compose logs --tail=120 odysseus Mention what you ran in the pull request description. If you could not run a check, say so. +## Maintainer governance and review guidance + +For repository roles, product decisions, risk-based approvals, issue triage, independent security reviews, CodeQL findings, and release handoffs, see the [Odysseus maintainer handbook](docs/maintainers/README.md). This complements the contributor requirements below and does not change the branch model or current PR template. + ## Pull Requests Good pull requests usually include: diff --git a/docs/maintainers/README.md b/docs/maintainers/README.md new file mode 100644 index 000000000..aae60d4df --- /dev/null +++ b/docs/maintainers/README.md @@ -0,0 +1,40 @@ +# Odysseus maintainer handbook + +Odysseus maintainers are trusted to act, not to wait for permission. The default is individual responsibility; additional review is required when the **risk** warrants it. + +This handbook records the project's maintainer governance and gives reviewers a reusable process. It complements [CONTRIBUTING.md](../../CONTRIBUTING.md) and [SECURITY.md](../../SECURITY.md); it does not replace the PR template, repository protections, or product decisions. + +## At a glance + +| Area | Working rule | +|---|---| +| Product direction | Pewds is the Product Owner | +| Ordinary, reversible maintainer PR | Self-merge allowed after appropriate validation and applicable repository checks | +| External contribution | Maintainer review required | +| High-risk change | At least one **independent, qualified** maintainer review | +| Access | Need-to-have, least privilege, separately granted per system | +| Release | A designated Release Maintainer owns preparation and publish | +| Inactivity | Access review after 2 months without activity, notice, or response | +| Documentation | Repository Markdown is canonical | + +These are governance rules, not guarantees that a GitHub permission or branch-protection setting permits a particular action. Respect the stronger applicable automated gate. + +## Start here + +- [Roles and access](roles-and-access.md): responsibility, permissions, and continuity. +- [Product and feature proposals](product-and-features.md): product approval versus technical ownership. +- [Changes and review](changes-and-review.md): review thresholds, self-merge, contributor corrections. +- [Practical review playbook](review-playbook.md): risk, provenance, CI, CodeQL, independent audit, and handoff evidence. +- [Triage](triage.md): turning incoming reports into actionable work. +- [Testing and releases](testing-and-releases.md): evidence, platform validation, promotion to main. +- [Documentation](documentation.md): canonical sources, updates, and traceability. + +## Accountability + +Maintainers own the changes they take on, including validation, regression prevention, follow-up, and rollback when needed. Roles describe responsibilities, not a hierarchy of technical approval. + +Important product-direction decisions go through the Product Owner. Maintainers retain responsibility for feasibility, implementation, maintainability, and security. + +AI coding or review agents can help collect evidence, test hypotheses, and prepare changes, but they are not the qualified human reviewer required for high-risk changes. A maintainer must inspect and accept the evidence. + +Project spaces follow the [Code of Conduct](../../CODE_OF_CONDUCT.md). diff --git a/docs/maintainers/changes-and-review.md b/docs/maintainers/changes-and-review.md new file mode 100644 index 000000000..fdad31d23 --- /dev/null +++ b/docs/maintainers/changes-and-review.md @@ -0,0 +1,62 @@ +# Changes and review + +Review is **risk-based**. Trusted maintainers should be able to ship ordinary work without creating a queue of ceremonial approvals. + +## Required review + +| Change | Minimum review rule | +|---|---| +| Ordinary, reversible maintainer change | Independent approval optional; self-merge permitted after appropriate validation | +| External contribution | Maintainer review required | +| High-risk change | At least one independent, qualified maintainer review required | + +Repository branch protection and required checks also apply. Never treat this document as permission to bypass a live protection rule. + +## Ordinary maintainer changes + +1. Open a PR against `dev`, scoped to one coherent change. +2. Explain intent, expected behavior, validation, and rollback if relevant. +3. Run suitable tests and inspect the actual result. +4. Ensure applicable required checks pass, address feedback, and merge when ready. + +Self-merge is allowed for ordinary reversible work. The author remains responsible for follow-up, correction, and rollback. + +## External contributions and small corrections + +External contributions receive maintainer review. + +A maintainer may directly fix a **small, obvious** problem in a contributor PR when the intended behavior is clear and the repair is faster than a separate round trip. Document what changed, preserve contributor attribution and history, avoid force-pushing, and re-run affected checks. + +Substantive or ambiguous corrections require discussion with the contributor or a separately scoped correction. A maintainer must not silently turn a contributor's PR into unrelated work. Any corrective commit changes the SHA being reviewed; update validation and audit evidence accordingly. + +## High-risk changes + +High-risk work requires **at least one independent qualified review**. The author cannot be the only approver. Examples: + +- Authentication, authorization, tenancy, sandboxing, agent tool permissions, or secrets. +- Outbound network and SSRF defenses, sensitive integrations, and deployment access. +- Destructive operations, deletion, filesystem ownership, and data migrations. +- Persistence and concurrency invariants with possible data loss. +- Major architectural contracts, release infrastructure, or changes hard to roll back. + +A high-risk review should verify the threat or failure model, exact final commit, reproducer or regression evidence, failure and rollback behavior, and the relevant CI/security findings. Use the [review playbook](review-playbook.md). + +## Cross-area changes and CODEOWNERS + +Touching multiple areas does **not** automatically require multiple approvals. Consult domain expertise when needed. CODEOWNERS helps identify reviewers and accountable contacts; it is not a blanket approval requirement by itself. See [issue #593](https://github.com/odysseus-dev/odysseus/issues/593) for the ongoing ownership map discussion. + +## Validation and UI evidence + +Automated tests count as runtime evidence when they truly exercise the changed behavior. Use manual validation when coverage is insufficient. + +For any change to visible UI behavior, follow the current [contributor UI checklist](../../CONTRIBUTING.md#style-and-visual-changes) and the repository PR template. Screenshots or clips are expected when they materially demonstrate the rendered result; screenshots alone do not replace behavioral tests. + +## Disagreement and escalation + +The maintainer doing the work owns ordinary implementation decisions within approved product direction and existing technical contracts. For substantive disagreements: + +1. Involve a qualified maintainer with relevant expertise. +2. Escalate unresolved cross-cutting technical issues to trusted project-wide maintainers. +3. Refer product-direction disagreements to the Product Owner. + +Security findings should be handled via [SECURITY.md](../../SECURITY.md), without disclosing exploit details in public issues. Repository protection rules should support these risk-based policies and should not accidentally force independent approval for every ordinary maintainer PR. diff --git a/docs/maintainers/documentation.md b/docs/maintainers/documentation.md new file mode 100644 index 000000000..0057305f0 --- /dev/null +++ b/docs/maintainers/documentation.md @@ -0,0 +1,31 @@ +# Documentation + +Repository Markdown is the canonical source for project documentation. A public docs site may be generated from it; avoid maintaining an independent wiki copy unless there is a deliberate future decision to do so. + +## Where information belongs + +| Location | Purpose | +|---|---| +| `/design` | Product intent, proposals, and accepted decisions (when those paths are used) | +| `/specs` | Technical contracts and invariants (when those paths are used) | +| `/docs` | User, developer, architecture, security, deployment, and maintainer guidance | +| Root Markdown | Discoverable entry points and policies | + +Keep root documents easy to find: + +- `README.md` +- `CONTRIBUTING.md` +- `SECURITY.md` +- `CODE_OF_CONDUCT.md` + +Not every named directory exists today; do not invent a parallel documentation store just to match this map. + +## Updating documentation + +When behavior, architecture, a workflow, or its verification requirements change, update the canonical documentation in the same PR when appropriate. + +Separate **policy** from **examples**: examples may evolve with tooling; changes to role authority, review requirements, or product ownership require an explicit governance decision. Do not introduce a new mandatory approval rule through a checklist alone. + +Documentation PRs should link the relevant issue, record what was reviewed (links, commands, or affected features), and avoid publishing private channels, account names, local paths, secrets, or confidential incident details. + +For practical examples of review evidence, see the [review playbook](review-playbook.md). For contributor setup and PR requirements, use [CONTRIBUTING.md](../../CONTRIBUTING.md). diff --git a/docs/maintainers/product-and-features.md b/docs/maintainers/product-and-features.md new file mode 100644 index 000000000..71b6cf686 --- /dev/null +++ b/docs/maintainers/product-and-features.md @@ -0,0 +1,42 @@ +# Product and feature proposals + +Pewds is the Product Owner for Odysseus and has final authority over product direction. Maintainers own technical feasibility, implementation, and safety. + +## Which changes require product approval? + +**Product Owner approval before implementation:** new features and material changes to product direction or behavior. + +**No separate product approval by default:** bug fixes, security fixes, maintenance, tests, CI, documentation, and internal refactors, unless they materially change product behavior or direction. + +A security or reliability fix that changes behavior should state the user-visible impact and seek product input when its direction is genuinely ambiguous. Urgent containment need not wait if it preserves existing product intent. + +## Product Owner initiated + +```text +Product Owner proposal or implementation + -> technical feasibility + -> plan and implementation + -> risk-based review and validation +``` + +A Product Owner proposal is already product-approved; it is not automatically technically approved or exempt from high-risk review. + +## Community or maintainer initiated + +```text +Feature proposal + -> Product Owner accepts / rejects + -> technical feasibility + -> plan and implementation + -> risk-based review and validation +``` + +Record the decision in an issue, proposal, work item, or PR so future maintainers can discover it. Do not rely on a chat-only decision. + +## Decision boundaries + +The Product Owner decides **what belongs in Odysseus**. Maintainers decide **how approved work is built safely and maintainably**. + +A rejected or out-of-scope feature can be maintained independently in a contributor's fork. + +If the Product Owner is unavailable, new product-direction decisions wait unless that authority was explicitly delegated. Urgent security and reliability corrections that do not redefine product direction may continue under normal review rules. diff --git a/docs/maintainers/review-playbook.md b/docs/maintainers/review-playbook.md new file mode 100644 index 000000000..77c4357e7 --- /dev/null +++ b/docs/maintainers/review-playbook.md @@ -0,0 +1,124 @@ +# Practical PR review playbook + +Use this as a **repeatable review method**, not as an extra organizational approval ladder. The binding minimums remain in [changes and review](changes-and-review.md): ordinary maintainer changes may be self-merged, external PRs require maintainer review, and high-risk work requires at least one independent qualified maintainer review. + +No single checklist replaces engineering judgment. Increase depth with risk. + +## 1. Establish provenance and scope + +- Confirm repository, source fork, target branch (`dev`), exact **HEAD SHA**, and current base. +- Review the full effective diff, commit history, issue, prior feedback, and related upstream changes. +- Confirm the branch is mergeable against the **current** `dev`, not an older local snapshot. +- Identify the user-facing contract, affected data and users, security boundaries, and rollback plan. +- Separate pre-existing upstream problems from regressions introduced or made reachable by the PR. + +Example read-only commands: + +```bash +REPO="odysseus-dev/odysseus" +PR=1234 +gh pr view "$PR" --repo "$REPO" --json title,state,isDraft,headRefOid,baseRefName,mergeable +gh pr diff "$PR" --repo "$REPO" +gh pr checks "$PR" --repo "$REPO" +``` + +A green check on an old commit does not verify the revised branch. + +## 2. Review the failure and threat model + +For the relevant change, ask what happens with malformed input, missing auth state, wrong owner, concurrent requests, stale caches, partial failure, retries, cancelled jobs, unavailable dependencies, and restart/recovery. + +Prioritize boundaries that can lose data or authority: identity, delegated API tokens, filesystem confinement, email deletion, persistence versioning, network/SSRF destinations, secrets, and agent tool execution. + +For security claims, demonstrate the source-to-sink path and the *actual* guard. Tests must represent production middleware or transport where a unit mock could conceal a bypass. + +## 3. Correct without obscuring contributor work + +For small, unambiguous issues, maintainers may add focused corrections directly to a contributor PR under [the contribution review policy](changes-and-review.md#external-contributions-and-small-corrections). Preserve provenance, explain the change, prefer normal commits and fast-forward pushes, and avoid unrelated rewrites. + +For substantive or ambiguous work, ask the contributor or prepare a clearly separated patch. Any code change means validating **the new SHA**, not approving based solely on the previous audit. + +## 4. Build an evidence record + +Useful evidence includes: + +- Pre-fix reproduction, expected outcome, and actual outcome. +- Focused regression tests for the fix and broad suites relevant to cross-cutting effects. +- Actual runtime checks (browser/UI, backend, Docker, supported OS) where necessary. +- Exact executed commands, result counts, skipped cases, and environmental limitations. +- GitHub required checks and conflict-free state on the exact final SHA. +- Relevant operational consequences (data migration, rollback, compatibility). + +Keep audits reproducible. A screenshot alone is not a functional test, and a passing mocked test is not proof that a production integration path is safe. + +## 5. Independent review for high-risk changes + +A separate **qualified maintainer** must inspect high-risk changes; the implementing author cannot be the sole reviewer. Independent coding agents may be useful for adversarial tests and cross-checks, but **an AI agent is not itself a GitHub approver** and does not replace human accountability. + +To avoid confirmation bias, give the independent reviewer the current SHA, changed files, acceptance criteria, known reproduction, and explicit permission to challenge the proposed solution. Preserve unresolved findings and require a new audit when meaningful fixes change the diff. + +## CodeQL and security scanning + +A successful CodeQL workflow means analysis ran, not that the security dashboard is empty. Likewise, resolving a PR discussion does not necessarily resolve its code-scanning alert. + +For each relevant alert: + +1. Record the alert number, rule, severity, file/line, state, and source-to-sink trace if applicable. +2. Compare with the current `dev` baseline; decide whether the PR introduced, exposed, or did not affect it. +3. Reproduce or reason through the actual production behavior, including validation and authorization. +4. **Fix** credible vulnerabilities, preferably with a regression test. Do not hide them by broad suppressions. +5. **Recommend dismissal** only for a demonstrable false positive or a stale/invalid finding, with a precise technical explanation; only authorized maintainers should dismiss alerts. +6. Document genuine but intentionally accepted risk separately. A true positive is not a false positive merely because it is deliberate. +7. Rerun or inspect updated analysis on the final commit and confirm no blocking issues remain. + +Never publish exploit payloads, secrets, or credentials in public review comments. See [SECURITY.md](../../SECURITY.md). + +## 6. Decide and integrate + +Before approval or self-merge, verify: + +- Current branch and reviewed SHA are still the ones to be merged. +- All applicable required checks are complete and successful. +- High-risk independent review is present, where required. +- No unresolved, credible P0/P1/P2 security or data-integrity defects remain in the change. Resolve or explicitly document P3 observations with a tracked owner and priority. +- Review discussions, compatibility risks, and operational handoffs have been addressed. +- The correct target is `dev`; `main` is curated for releases. + +This P0-P3 classification is [triage guidance](triage.md#practical-priority-language), not a replacement for branch protection or an excuse to self-approve high-risk work. Maintainers may apply a stricter bar where warranted. + +For a final manual merge, GitHub CLI supports an exact-head guard: + +```bash +gh pr merge 1234 --repo odysseus-dev/odysseus --squash \ + --match-head-commit +``` + +Use `--delete-branch` only when removing the source branch is appropriate and does not disrupt another workstream. Do not force a merge past review or security requirements. + +## 7. Post-merge follow-up + +Verify the PR actually merged and reference its integration commit. Record remaining operational work, such as schema migrations, backup/restore verification, service restarts, or compatibility documentation. A code change to a container mount does **not** by itself migrate existing runtime data. + +Do not silently mark a deferred data migration complete; track its owner, evidence, rollback, and execution separately. + +## Reusable reviewer handoff + +```text +PR / target: +Reviewed source HEAD SHA: +Current target branch SHA: +Scope and changed files: +Impact and risk classification: +Security / correctness invariants challenged: +Pre-fix reproducer and post-fix evidence: +Tests run, skipped, and environment: +CodeQL / scanning alert dispositions: +Required GitHub checks (exact SHA): +Mergeability and remaining conflicts: +Unresolved P0 / P1 / P2 / P3 findings: +Rollback / operational follow-ups: +Independent qualified review required? Completed? +Verdict: APPROVED / CHANGES REQUIRED / BLOCKED +``` + +State what was actually verified. Do not write "all checks green" if only a subset of workflows was inspected. diff --git a/docs/maintainers/roles-and-access.md b/docs/maintainers/roles-and-access.md new file mode 100644 index 000000000..a98fd3258 --- /dev/null +++ b/docs/maintainers/roles-and-access.md @@ -0,0 +1,40 @@ +# Roles and access + +Roles clarify who is accountable. They do not create a management chain or automatically grant privileges. + +## Responsibility and typical access + +| Responsibility | Typical GitHub access | Purpose | +|---|---|---| +| Product Owner | No automatic repository access implied | Product direction | +| Organization Owner | Organization Owner | Organization administration and continuity | +| Repository Admin | Admin | Repository settings, permissions, and protection rules | +| Maintainer | Write by default | Implement, review, merge, and maintain | +| Triage | Triage | Organize intake, de-duplicate, and prepare work | +| Contributor / Platform Tester | Public / Read as appropriate | Contribute code or provide validation | + +Maintain access may be granted when repository-management duties require it. + +## Maintainer focus + +- **Core Maintainer:** project-wide judgment, difficult technical decisions, governance support. +- **Technical Maintainer:** implementation, architecture, validation, technical health. +- **Project Maintainer:** review, contributor coordination, documentation, releases, organization. + +These are responsibility labels. No label automatically changes GitHub permissions or overrides the [review rules](changes-and-review.md). + +## Functional responsibilities + +Maintainers may coordinate Release, Security, Infrastructure and CI, Community, or Test and Validation. These are duties, not ranks. For each release, designate one Release Maintainer and record the handoff if ownership changes. + +## Least privilege and continuity + +Grant access because it is necessary for current work, not because of a title. GitHub, Matrix, Plane, infrastructure, SSH, deployment systems, secrets, and external services are separate access surfaces. + +For elevated or sensitive access, document a responsible owner and recovery route in an appropriate **private** administrative system. Do not publish tokens, SSH details, personal addresses, or credential locations in this handbook. Remove access promptly when it is no longer needed. + +## Inactivity + +Tell an admin when you expect to be unavailable; a communicated absence is fine. + +If there is no meaningful maintainer or triage activity for **two months**, no prior notice, and no response to reasonable follow-up, elevated access may be reviewed and removed. This is access hygiene, not punishment. Returning members may regain access appropriate to their active responsibilities. diff --git a/docs/maintainers/testing-and-releases.md b/docs/maintainers/testing-and-releases.md new file mode 100644 index 000000000..ce7c1b59b --- /dev/null +++ b/docs/maintainers/testing-and-releases.md @@ -0,0 +1,46 @@ +# Testing and releases + +Validation must match the actual change, and evidence must state what was **run**, **observed**, and **not tested**. + +`dev` is the integration and stabilization branch. `main` is the curated release destination. Changes land in `dev`; a release promotes a verified `dev` revision to `main`. + +## Change validation + +- Run focused tests that cover the modified behavior and failure cases. +- Run broader regressions where risk crosses module boundaries. +- For authentication, data loss, or similar high-risk work, include adversarial and rollback tests. +- Reproduce the issue on the pre-fix version when safe and practical, then prove the correction. +- For visual changes, run the application and attach the screenshots or clips required by [CONTRIBUTING.md](../../CONTRIBUTING.md). +- Record exact commands, environment, result, skipped tests, and blockers. Never claim execution from a command you did not run. + +GitHub Actions success means a workflow finished successfully; it does **not** mean CodeQL found no alerts. Use [review playbook](review-playbook.md#codeql-and-security-scanning) to assess findings separately. A review must tie required CI and the final diff to the **same commit SHA**. + +## Supported-environment evidence + +Platform Testers may validate without maintainer permissions. Useful coverage includes: + +- Windows native and WSL. +- macOS Apple Silicon. +- macOS Intel while supported. +- Linux native on the reference distribution. +- Docker on supported hosts. + +Record the environment and configuration for each result. Do not imply that a platform was tested when it was not. + +## Release ownership + +A designated **Release Maintainer** owns preparation and the publish decision once the release criteria are met. Other maintainers and testers supply the evidence needed for the change set. If the Release Maintainer becomes unavailable, another maintainer explicitly takes over. + +## Release checklist + +Before promoting `dev` to `main`: + +1. Identify and freeze the release candidate SHA; check the actual content that will ship. +2. Verify required checks and relevant supported-environment validation. +3. Confirm high-risk and data-migration findings are resolved or addressed through an explicit approved plan. +4. Document skipped coverage, known issues, compatibility and upgrade implications. +5. Prepare the version, tag, artifacts, and release notes. +6. Confirm rollback/recovery, including any manual persistence or data migration steps. +7. Publish and record what was promoted, the verification evidence, and operational follow-ups. + +Publishing does not require ceremonial Core or domain sign-off when the criteria are met. Exceptional or disputed releases follow the normal high-risk escalation path. diff --git a/docs/maintainers/triage.md b/docs/maintainers/triage.md new file mode 100644 index 000000000..d2f47f8ab --- /dev/null +++ b/docs/maintainers/triage.md @@ -0,0 +1,48 @@ +# Triage + +Triage turns reports into evidence-backed work that a maintainer can pick up without guessing. + +## Intake flow + +```text +Report + -> reproduce / clarify + -> identify duplicates and affected versions + -> classify impact, urgency, and risk + -> prepare actionable work item + -> maintainer takes ownership + -> PR, verification, and follow-up +``` + +## A useful work item + +Record what is known and distinguish it from what remains unverified: + +- Original report and reproducible steps; installation method, OS, version/commit, and relevant environment. +- Expected versus actual behavior, including user-visible impact. +- Evidence: sanitized logs, screenshots, failing tests, or controlled reproduction. +- Related issues, PRs, duplicates, or a known prior regression. +- Scope, acceptance criteria, and non-goals. +- Priority, ownership, and relevant domain expertise. +- Remaining unknowns, risks, and safe recovery or workaround when available. + +Small and obvious fixes may go straight to a focused PR, consistent with [CONTRIBUTING.md](../../CONTRIBUTING.md) and [changes and review](changes-and-review.md). + +## Practical priority language + +Use P0-P3 as **triage shorthand**, not as a substitute for verified exploitability or the project's actual GitHub labels: + +| Priority | Typical impact | Expected response | +|---|---|---| +| P0 | Active exploitation, broad irreversible loss, catastrophic outage | Immediate containment and escalation | +| P1 | Credible severe security or data-integrity vulnerability | Urgent owner assignment and validated fix | +| P2 | Significant correctness, security, or operational regression | Prioritize and resolve before approving affected high-risk changes | +| P3 | Limited impact, maintainability, or low-risk polish | Fix or explicitly track with rationale | + +Do not call a concern harmless merely because an automated check is green. Escalate privately when reproduction would disclose a vulnerability. + +## Where decisions live + +Matrix is for coordination; durable decisions belong in GitHub, Plane, a specification, or the relevant PR. Keep confidential security and access details in appropriate private channels. Do not leave important project decisions only in chat history. + +See the [review playbook](review-playbook.md) for converting triage into review evidence and handling contested or uncertain findings. From eb940f24a3f867220804a644d128ea023ddf25d1 Mon Sep 17 00:00:00 2001 From: pewdiepie-archdaemon Date: Tue, 13 Oct 2026 13:37:51 +0000 Subject: [PATCH 04/36] feat(photo-editor): NL editor planner, image ops, email/gallery server Server-side photo editor: natural-language editor agent route (/api/editor/agent), photo_editor tool family + style guide, image-op provider proxy with progress, email server hardening, gallery/hwfit routes, and graceful shutdown. Source: vendor/photocraft (ad1cff44) editor-server files, merged onto upstream/dev (a8c147b2). Conflict resolution documented in pr_work/B_conflicts.md. Co-Authored-By: Claude Opus 5.5 --- app.py | 83 +- docs/EDITOR_AGENT_API.md | 201 ++ routes/editor_agent_routes.py | 1811 +++++++++++++++++ routes/email/email_helpers.py | 169 +- routes/email/email_routes.py | 449 ++-- routes/gallery/gallery_routes.py | 821 +++++++- routes/hwfit_routes.py | 14 +- routes/model_routes.py | 29 +- src/activity.py | 61 + src/agent_tools/editor_tools.py | 646 ++++++ src/editor_agent_model.py | 498 +++++ src/editor_style_guide.py | 1105 ++++++++++ src/fixture_email.py | 65 +- src/shutdown.py | 34 + src/tools/image.py | 16 +- src/upload_handler.py | 79 +- tests/test_activity.py | 36 + tests/test_agent_editor_routes.py | 1492 ++++++++++++++ tests/test_docs_no_orphan_images.py | 14 +- tests/test_edit_image_tool.py | 30 +- tests/test_editor_agent_model_resolution.py | 71 + tests/test_editor_planner_routing.py | 171 ++ tests/test_editor_writing_action_routing.py | 11 + tests/test_email_imap_timeout.py | 12 +- .../test_email_list_stale_while_revalidate.py | 79 + tests/test_email_send_only_no_inbox.py | 2 +- tests/test_email_smtp_security.py | 18 +- tests/test_email_test_connection_oauth.py | 24 +- tests/test_gallery_cloud_inpaint.py | 155 ++ tests/test_gallery_edit_progress.py | 120 ++ tests/test_gallery_heavy_image_ops.py | 201 ++ tests/test_gallery_image_endpoint_fallback.py | 124 ++ tests/test_gallery_image_model_picker.py | 27 + tests/test_gallery_landing_import_button.py | 25 + tests/test_gallery_seneca_image_ops.py | 187 ++ tests/test_hwfit_gemma4_12b.py | 77 +- tests/test_hwfit_gpu_count_nonnumeric.py | 99 +- tests/test_hwfit_macos.py | 27 +- tests/test_hwfit_quant_formats.py | 14 +- tests/test_image_creation_routing.py | 6 +- tests/test_image_op_offload.py | 54 + tests/test_image_provider_transport.py | 32 + tests/test_imap_leak_fixes.py | 2 +- tests/test_photo_editor_family.py | 503 +++++ tests/test_photo_editor_style_guide.py | 642 ++++++ tests/test_shutdown_signal.py | 51 + 46 files changed, 9662 insertions(+), 725 deletions(-) create mode 100644 docs/EDITOR_AGENT_API.md create mode 100644 routes/editor_agent_routes.py create mode 100644 src/activity.py create mode 100644 src/agent_tools/editor_tools.py create mode 100644 src/editor_agent_model.py create mode 100644 src/editor_style_guide.py create mode 100644 src/shutdown.py create mode 100644 tests/test_activity.py create mode 100644 tests/test_agent_editor_routes.py create mode 100644 tests/test_editor_agent_model_resolution.py create mode 100644 tests/test_editor_planner_routing.py create mode 100644 tests/test_email_list_stale_while_revalidate.py create mode 100644 tests/test_gallery_edit_progress.py create mode 100644 tests/test_gallery_heavy_image_ops.py create mode 100644 tests/test_gallery_image_endpoint_fallback.py create mode 100644 tests/test_gallery_image_model_picker.py create mode 100644 tests/test_gallery_landing_import_button.py create mode 100644 tests/test_gallery_seneca_image_ops.py create mode 100644 tests/test_image_op_offload.py create mode 100644 tests/test_photo_editor_family.py create mode 100644 tests/test_photo_editor_style_guide.py create mode 100644 tests/test_shutdown_signal.py diff --git a/app.py b/app.py index 60441baf1..6c3974c65 100644 --- a/app.py +++ b/app.py @@ -178,6 +178,14 @@ app.add_middleware(SecurityHeadersMiddleware) import asyncio as _asyncio from starlette.middleware.base import BaseHTTPMiddleware as _BaseHTTPMiddleware from starlette.responses import JSONResponse as _JSONResponse +import anyio as _anyio +from starlette.requests import ClientDisconnect as _ClientDisconnect + +# SSE endpoints (e.g. /api/editor/bridge/events) end/abort mid-stream when the +# browser disconnects; Starlette's BaseHTTPMiddleware surfaces that as +# anyio.EndOfStream (and sometimes ClientDisconnect). Those are client-side +# events, not server errors, so we map them to a 499 instead of a 500 traceback. +_STREAM_CLIENT_EXC = (_anyio.EndOfStream, _ClientDisconnect) REQUEST_HARD_TIMEOUT = float(os.getenv("REQUEST_HARD_TIMEOUT", "45")) _TIMEOUT_EXEMPT_PREFIXES = ( @@ -190,6 +198,7 @@ _TIMEOUT_EXEMPT_PREFIXES = ( "/api/cookbook/setup", # remote pacman/apt installs "/api/upload", # large files "/api/image", # diffusion proxies (inpaint/harmonize/upscale/etc.) — own 120s httpx timeout + "/api/editor", # editor agent/bridge: generative steps can take 30-110s (SAM + local diffusion) "/api/memory/audit", # retains own 120s LLM inactivity timeout ) @@ -225,15 +234,43 @@ class _InteractiveActivityMiddleware(_BaseHTTPMiddleware): return await call_next(request) +# Heavy image ops that can run 30-150 s; a deploy restart must not cut these off. +_HEAVY_IMAGE_OP_PREFIXES = ( + "/api/image/inpaint", + "/api/image/replace-background", + "/api/image/outpaint", + "/api/image/extend", + "/api/image/remove-bg", + "/api/image/mask", + "/api/image/upscale-local", + "/api/image/denoise", + "/api/image/enhance-face", + "/api/gallery/ai-upscale", + "/api/gallery/style-transfer", + "/api/editor/agent", +) + + class _SlowRequestLogMiddleware(_BaseHTTPMiddleware): async def dispatch(self, request, call_next): + from src import activity + path = request.url.path or "" + heavy = any(path.startswith(p) for p in _HEAVY_IMAGE_OP_PREFIXES) + if heavy: + activity.begin_image_op(path) start = time.perf_counter() status = 500 try: response = await call_next(request) status = getattr(response, "status_code", 0) or 0 return response + except _STREAM_CLIENT_EXC: + # Client disconnected / SSE stream ended — not a server error. + status = 499 + return _JSONResponse({"detail": "client disconnected"}, status_code=499) finally: + if heavy: + activity.end_image_op(path) elapsed = time.perf_counter() - start try: threshold = float(os.getenv("ODYSSEUS_SLOW_REQUEST_LOG_SECONDS", "0.75") or "0.75") @@ -821,6 +858,10 @@ app.include_router(setup_gallery_routes()) from routes.editor_draft_routes import setup_editor_draft_routes app.include_router(setup_editor_draft_routes()) +# Natural-language editor planning (planner wired to the configured chat model) +from routes.editor_agent_routes import setup_editor_agent_routes +app.include_router(setup_editor_agent_routes()) + # Scheduled tasks + event bus from src.task_scheduler import TaskScheduler task_scheduler = TaskScheduler(session_manager) @@ -992,12 +1033,21 @@ async def serve_login(request: Request): @app.get("/api/version") async def get_version(): from core.constants import APP_BUILD_VERSION, APP_SOURCE_COMMIT, APP_VERSION + from src import activity return { "version": APP_VERSION, "build": APP_BUILD_VERSION, "source_commit": APP_SOURCE_COMMIT, + "busy_image_ops": activity.heavy_image_ops_active(), } + +@app.get("/api/busy") +async def get_busy(): + """Report in-flight heavy work so a deploy can wait instead of dropping it.""" + from src import activity + return activity.activity() + @app.get("/api/health") async def health_check() -> Dict[str, str]: return {"status": "healthy", "timestamp": datetime.now(timezone.utc).isoformat()} @@ -1436,4 +1486,35 @@ if __name__ == "__main__": bind_host = os.getenv("APP_BIND", "127.0.0.1") bind_port = int(os.getenv("APP_PORT", "7011")) - uvicorn.run(app, host=bind_host, port=bind_port, log_level="info") + # Give in-flight requests time to finish on SIGTERM instead of being cut + # off mid-flight. Diffusion edits run 60-120 s; outpaint/extend can run up + # to ~330 s (provider cap 900 s), so default the graceful window to 900 s. + # systemd's TimeoutStopSec must exceed this (see the graceful-stop drop-in). + # + # We hook uvicorn's signal handler so a SIGTERM also flips src.shutdown: + # SSE / streaming generators check it and end promptly, so an open browser + # stream cannot stall a restart for the whole window. The long window stays + # a safety net for genuine in-flight image ops. + from src import shutdown as _shutdown + + config = uvicorn.Config( + app, + host=bind_host, + port=bind_port, + log_level="info", + timeout_graceful_shutdown=int(os.getenv("ODYSSEUS_GRACEFUL_SHUTDOWN_TIMEOUT", "900") or 900), + ) + server = uvicorn.Server(config) + + # uvicorn's capture_signals() wires SIGINT/SIGTERM to self.handle_exit, so + # wrapping that method lets us set the shutdown flag before the normal exit + # path runs (this uvicorn version has no install_signal_handlers kwarg). + _uvicorn_handle_exit = server.handle_exit + + def _handle_shutdown_signal(signum, frame): + _shutdown.request_shutdown() + _uvicorn_handle_exit(signum, frame) + + server.handle_exit = _handle_shutdown_signal + + server.run() diff --git a/docs/EDITOR_AGENT_API.md b/docs/EDITOR_AGENT_API.md new file mode 100644 index 000000000..c873d28d9 --- /dev/null +++ b/docs/EDITOR_AGENT_API.md @@ -0,0 +1,201 @@ +# Editor Agent API + +How an agent (LLM or script) drives the PhotoCraft editor. There are three layers: + +1. **Control API** — `window.geEditorControl`, a stable in-page command surface. +2. **NL planner** — `window.geEditorAI`, turns a prompt into control-API steps (LLM or basic mode). +3. **Bridge** — server-mediated SSE channel so a *server-side* agent can drive an **open** editor. + +All three are owner-scoped by the app session; nothing is exposed to unauthenticated users. + +--- + +## 1. Control API (`window.geEditorControl`) + +Installed when the editor opens. Version `EDITOR_CONTROL_VERSION`. + +```js +await window.geEditorControl.invoke(op, params) // { ok, op, result } | { ok:false, op, error } +await window.geEditorControl.run([{op, params}]) // sequential, stops on first failure +window.geEditorControl.list() // [{op, description}] +``` + +`invoke` is async. Every op that mutates the document pushes **one** undo entry via +`_saveState`, so a single op is one undo step. + +### Op catalog + +| Op | Params | Notes | +| --- | --- | --- | +| `document.info` | – | size, layers, mode | +| `document.importImage` | `{dataUrl,name,raster}` | load a raster image as a new layer | +| `layers.list` | – | top-most first | +| `layers.select` | `{id\|index\|name}` | index is top-most-first | +| `layers.addRaster` | – | new empty pixel layer | +| `layers.addAdjustment` | `{type,params}` | adjustment layer | +| `layers.duplicate` / `layers.delete` | – | on the active/selected layer(s) | +| `layers.setOpacity` | `{id?,opacity 0-1}` | | +| `layers.setBlend` | `{id?,blendMode\|mode}` | | +| `layers.group` / `layers.ungroup` | – | | +| `selection.selectAll` / `deselect` / `invert` | – | | +| `selection.rectangle` / `selection.ellipse` | `{x,y,w,h}` | document space | +| `selection.subject` | `{label}` | SAM text grounding (falls back to a centred rect) | +| `selection.face` | – | SAM "face" (falls back to an upper-centre ellipse) | +| `selection.sky` | – | SAM "sky" (falls back to a top-band colour range) | +| `selection.background` | `{fuzziness}` | border colour union | +| `selection.samText` | `{text}` | SAM text grounding only (throws without SAM) | +| `selection.colorRange` | `{color,fuzziness}` | | +| `selection.hasMask` | – | `{active, space}` | +| `adjustments.apply` | `{type,params}` | adjustment layer; camelCase types accepted | +| `adjustments.applyMasked` | `{brightness,contrast,saturation,warmth,invert}` | pixel pass, masked if a selection exists else whole layer | +| `filters.apply` | `{id}` | opens the slider dialog and applies | +| `filters.applyMasked` | `{id,radius,amount,invert}` | `id`: blur-gaussian, sharpen, unsharp-mask | +| `transform.start` / `transform.apply` / `transform.cancel` | – | | +| `text.add` | `{text,x,y,color,size,font,align}` | | +| `history.undo` / `history.redo` | – | | +| `history.mark` | – | returns current undo depth (for grouping) | +| `history.collapse` | `{mark,label}` | collapses entries above `mark` into one | +| `export.png` | – | `{dataUrl}` of the flattened document | +| `export.gallery` | – | saves a copy back to the gallery | +| `screenshot` | – | `{dataUrl,width,height}` of the composited canvas | + +Param conventions the planner must respect: + +- `adjustments.applyMasked`: `brightness`/`contrast`/`warmth` are **-100..100** (0 neutral); + `saturation` is a **multiplier** (1 neutral; values >3 are treated as percentages). +- `adjustments.apply` `type` aliases are normalised (`blackAndWhite` -> `black-white`). +- `color-balance` params are `{shadows:{r,g,b}, midtones:{r,g,b}, highlights:{r,g,b}}`; + a bare number is coerced to `{r:n,g:n,b:n}`. +- A masked op with **no** active selection degrades to a whole-layer edit (does not throw). + +--- + +## 2. NL planner (`window.geEditorAI`) + +```js +const res = await window.geEditorAI.execute('blur the background behind the person', { onStep }); +// exec: window.geEditorAI.execute('blur the background behind the person', { onStep }); +// res = { ok, prompt, reply, planner, basicMode, steps, trace, failed } +``` + +Flow (`static/js/editor/nl-edit.js`): + +1. Read document state + a **before** screenshot. +2. Ask the **server planner** (`POST /api/editor/plan`) with the prompt, state and + screenshot. The client passes its strong guidance via `state.planner_guidance`. + If the route is unavailable it falls back to `localPlan` (regex — labelled + **"basic mode"** in the UI). +3. `sanitizeSteps()`: drop no-op/export steps and, if a masked op has no preceding + `selection.*`, insert the region implied by the prompt (face/sky/background/subject). +4. `history.mark`, then run each step, screenshotting after each one. +5. **Self-check** (server plans only, max 3 rounds): send the after screenshot back in + `mode:"check"` with the original intent. If not satisfied, **revert** the previous + attempt (undo its history entries) and apply the model's **full corrected** plan. +6. `history.collapse` — the whole request becomes a single undoable step. + +UI: the Quick Edit box routes multi-clause / "and ..." prompts to this executor and +streams `Step i/n: ` into its status line; a `(basic mode)` suffix marks the offline planner. + +### Server route + +`POST /api/editor/plan` (HARNESS, `routes/editor_agent_routes.py`) + +``` +{ prompt, state, mode?: "plan"|"check", screenshot?: "data:image/..." } +-> { reply, planner, steps: [{op, params}], done? } +``` + +Owner-scoped, authenticated. Uses the configured default/utility model with vision. +If the model call is unavailable it returns a deterministic `planner:"fallback"` plan. + +--- + +## 3. Bridge (server agent drives an open editor) + +`static/js/editor/editor-bridge.js` subscribes the open editor to the server queue. +Enabled automatically when the editor opens (`window.__geEditorBridge`). + +- Subscribe: `GET /api/editor/bridge/events?session=` — SSE, `data: {id,op,params}` (keepalive every 15s). +- The page runs the op via the control API and replies: + `POST /api/editor/bridge/result {id,ok,result,error,screenshot,ms}`. +- Enqueue a command (server side): `POST /api/editor/bridge/command {session, op|steps, wait, timeout}`. + With `wait:true` the call blocks and returns the editor's result. + +Round-trip example (authenticated; `` = `window.__geEditorBridge.session`): + +```bash +curl -s -X POST http://127.0.0.1:7011/api/editor/bridge/command \ + -H 'Content-Type: application/json' -b "odysseus_session=$TOKEN" \ + -d '{"session":"default","op":"layers.addRaster","wait":true,"timeout":8000}' +# -> {"id":"...","ok":true,"result":{...},"screenshot":"data:image/png;base64,..."} +``` + +Session scoping is `owner:session`, so an agent can only drive editors owned by the +same user. The bridge is a **browser-only** flow: it requires an editor page to be open. + +--- + +## 4. SAM dependency + +`selection.subject` / `selection.face` / `selection.sky` / `selection.samText` use the +server SAM mask tool (`POST /api/image/mask`). When the SAM tools are **not installed** +the editor raises *"Object mask tools are not installed"* and the control op falls back +to a **coarse approximation** (centred rect / upper-centre ellipse / top-band colour range). +On a plain background the approximation is acceptable; on real photos it leaks, producing +visible rectangular/elliptical artifacts. Install the Cookbook SAM mask dependencies for +true subject/face/sky segmentation. + +`selection.colorRange` and `selection.background` never need SAM (pure colour matching), +but they are also coarse on busy scenes. + +--- + +## 5. Two ways for an agent to use this — pick one + +### Option A — built-in tool (in-process) + +Register an editor-control tool inside the app (same process as the model) that calls the +bridge/control API directly. Uses the app's normal login; nothing new is exposed. This is +the rejected stdio-MCP design's replacement: a server-side tool that talks to the bridge +needs either an internal token (auth-bypass) or to run in-process. **Decision pending (user).** + +### Option B — browser-only flow (works today) + +No new server tool. An external agent drives an open editor purely through the bridge +endpoints using the logged-in owner's session. Restricted to the editor the user has open. +This is the flow demonstrated above and requires **no** auth-bypass. + +A stdio `photo_editor` MCP server was prototyped and **rejected** because the built-in MCP +subprocess has no internal token; wiring it would require expanding an auth bypass. +The two viable options above are left for the user to choose. + +--- + +## 6. Known limitations / honest grading + +Live run against real 7011 with the server LLM planner (7 prompts): all 7 executed +(`planner:"llm"`). Graded by eye: + +- PASS: black & white portrait; warm cinematic; sharper + more vivid; text title. +- FAIL (region-masked, SAM-bound): "dramatic sky" (global over-saturation instead of a + dramatic, masked sky), "blur the background" (coarse mask leaks; visible rectangle), + "brighten the face" (coarse ellipse/rect artifact). + +Whole-image edits are reliable; subject/face/sky region edits need SAM. The self-check +loop works (revert-and-replan) but the region cases spend all 3 iterations. + +## 7. Theming (control kit) + +The editor's interactive controls (ranges, number fields, selects, checkboxes) share a +single themed kit: `static/js/editor/control-kit.js` adds `.ge-ctl-*` classes scoped to +the editor roots (`#gallery-editor-container`, `.ge-adj-popup`, `.ge-filter-overlay`, +`.ge-canvas-size-overlay`, `.ge-float-window`, `.ge-transform-popup`), and +`static/css/13-image-editor.css` (section "Editor control kit") styles them from the app +design tokens (`static/css/00-tokens.css`) to match the app sliders/selects. It keeps +range fills in sync and attaches a **scrub-on-label** affordance to numeric fields +(drag the field's label to change the value; arrow keys work, Shift = x10). + +**The PhotoCraft editor is dark-only.** `:root.light` flips the global tokens, but +`13-image-editor.css` has no `.light` handling and the editor relies on dark surfaces, +so light mode renders a garbled mix. Unlike the rest of Odysseus, the editor follows +Photoshop and is not offered in a light theme. diff --git a/routes/editor_agent_routes.py b/routes/editor_agent_routes.py new file mode 100644 index 000000000..3fe3b7901 --- /dev/null +++ b/routes/editor_agent_routes.py @@ -0,0 +1,1811 @@ +"""Editor agent routes: natural-language planning for the image editor. + +Provides: + POST /api/editor/plan -> {reply, planner, steps:[{op,params}], done?} + POST /api/editor/bridge/command -> enqueue validated ops for the open editor + GET /api/editor/bridge/events -> SSE stream of queued commands (browser subscriber) + POST /api/editor/bridge/result -> browser posts results/screenshots back + POST /api/editor/bridge/await -> wait for a queued command's result + +The browser editor (static/js/editor/nl-edit.js) posts {prompt, state, mode, screenshot} +and consumes {reply, planner, steps} or {done:true}. When mode == "check" and the model +reports completion, the route returns {"done": true} to stop the client's self-check loop. + +The planner is wired to the app's configured chat model (vision-capable when a screenshot +is supplied) via set_editor_planner(); a deterministic keyword planner is the fallback so +the editor always gets a usable plan. +""" +from __future__ import annotations + +import asyncio +import inspect +import json +import logging +import re +import uuid +from typing import Any, Awaitable, Callable, Dict, List, Optional, Tuple + +from fastapi import APIRouter, Request +from fastapi.responses import JSONResponse, StreamingResponse + +from src import editor_style_guide as style_guide +from src import editor_agent_model as editor_agent_model +from src import shutdown as _shutdown + +logger = logging.getLogger(__name__) + +# ── Op catalog (kept in sync with static/js/editor/editor-control.js) ── +OP_CATALOG = [ + "document.info", "document.importImage", + "document.crop", "document.resizeCanvas", "document.savePsd", + "layers.list", "layers.select", "layers.addRaster", "layers.placeImage", "layers.addAdjustment", + "layers.duplicate", "layers.delete", "layers.setOpacity", "layers.setBlend", + "layers.group", "layers.ungroup", "layers.addStyle", + "ai.generativeExpand", + "selection.selectAll", "selection.deselect", "selection.invert", + "selection.rectangle", "selection.ellipse", + "selection.subject", "selection.sky", "selection.background", "selection.samText", + "selection.colorRange", "selection.maskBounds", "selection.exportMask", + "adjustments.apply", "adjustments.applyMasked", + "filters.apply", "filters.applyMasked", + "transform.start", "transform.apply", "transform.cancel", + "text.add", + "history.mark", "history.collapse", "history.undo", "history.redo", + "export.png", "export.gallery", "screenshot", +] +# Retouch/selection ops are defined once in the shared style guide so the +# server planner and the browser fallback cannot drift; a plan naming one of +# these would otherwise be filtered out of the catalog. +for _extra_op in style_guide.NEW_OP_NAMES: + if _extra_op not in OP_CATALOG: + OP_CATALOG.append(_extra_op) +del _extra_op +_OP_SET = frozenset(OP_CATALOG) + +# Canonical argument shapes advertised to the model so it stops guessing. The +# browser client tolerates loose forms as a safety net, but the model should +# emit exactly these. +_ADJUSTMENT_TYPES = ( + "brightness-contrast {brightness,contrast} (multipliers, 1=neutral), " + "exposure {exposure,offset,gamma}, white-balance {temperature,tint}, " + "hue-saturation {hue,saturation,lightness}, vibrance {vibrance}, black-white, " + "shadows-highlights {shadows,highlights}, levels, curves, color-balance, " + "photo-filter {color,density,preserve_luminosity}, selective-color, gradient-map, " + "invert, posterize, threshold, channel-mixer, color-lookup" +) +_FILTER_IDS = ( + "blur-gaussian, blur-motion, blur-zoom, sharpen, sharpen-edges, unsharp-mask, " + "noise-add, noise-despeckle, pixelate-mosaic, pixelate-crystallize, stylize-emboss, " + "stylize-find-edges, stylize-solarize, distort-twirl, distort-pinch, distort-spherize, " + "distort-ripple, render-clouds, clarity {amount,radius}, vignette {amount,feather}" +) +_CANONICAL_GUIDANCE = ( + "Canonical op shapes (use exactly these, never invent keys or names): " + "adjustments.apply / adjustments.applyMasked params = " + "{\"type\": , \"params\": {...}}; " + "filters.apply / filters.applyMasked params = {\"id\": }. " + "Never nest a numeric bag under {\"adjustments\": {...}} with no type, and never invent " + "adjustment types or filter ids. \"clarity\" (params amount,radius) and \"vignette\" " + "(params amount,feather) are real filters — use them for local-contrast and framing. " + "Canvas: document.crop {x,y,width,height}, document.resizeCanvas {width,height,anchor}, " + "ai.generativeExpand {width,height,anchor,prompt,mode:'instruction',feather}. Text/styles: " + "text.add {text,x,y,color,size}, layers.addStyle {type,params}. Export: document.savePsd. " + "ai_edit_image takes a mode: 'instruction' (default; whole image, no mask, prompt names " + "what stays unchanged) for background/scene/season/style changes, adding an object to an " + "existing photo, and removing diffuse distractions; 'masked' (tight selection first) ONLY " + "for explicit regional removal of a clearly identified object. generate_image is ONLY for " + "creating a brand-new image." +) +# The shared style guide is the single source of truth for intent routing, +# calibrated strengths and the low-key/temperature/portrait rules. Both the +# server planner and the browser fallback read it (the client fetches +# GET /api/editor/style-guide). +_STYLE_GUIDANCE = style_guide.guidance() + +# EDITOR_PLANNER: async (messages: list[dict], owner: Optional[str]) -> str | dict. Set by app.py. +_EDITOR_PLANNER: Optional[Callable[[List[dict], Optional[str]], Any]] = None +_MAX_STEPS = 32 +_QUEUE_MAX = 64 +# Generative editor steps (SAM mask + local diffusion) take 30-110s each, so +# bridge waits and the agent's per-step wait must outlast the provider. +_BRIDGE_DEFAULT_TIMEOUT = 330.0 + +# ── Strict JSON ops schema (structured outputs / grammar) ── +# The planner (the local Qwen3.5-9B at :8097 and every fallback) is constrained +# to this schema so it can only emit valid editor ops. Allowed ops: +# * the deterministic editor ops (OP_CATALOG), +# * generate_edit -> image model (instruction, optional mask) [=ai_edit_image] +# * select_object -> SAM mask by description (seneca /v1/images/mask) +# [=selection.samText] +# * ask_user -> ask 2-3 concrete choices (never a silent tone default). +# The model-facing aliases are translated to the internal execution ops below, +# so the browser client and the executor keep their existing op vocabulary. +_MODEL_PLAN_OPS = tuple(dict.fromkeys( + list(OP_CATALOG) + ["generate_edit", "select_object", "ai_edit_image", + "generate_image", "ask_user"] +)) +_ALLOWED_PLAN_OPS = frozenset(OP_CATALOG) | {"ai_edit_image", "generate_image"} +_PLAN_OP_ALIASES = { + "generate_edit": "ai_edit_image", # image model: instruction + optional mask + "select_object": "selection.samText", # SAM mask by description +} +# Inner op values the ai_edit_image tool accepts (src/agent_tools/editor_tools.py +# `_AI_OPS`). A model-authored ai_edit_image step frequently omits the inner op or +# invents one (e.g. "restyle"), which the tool then rejects. Normalise it so +# instruction/restyle edits become op "generative" and masked region edits become +# "inpaint". +_AI_IMAGE_INNER_OPS = frozenset( + {"remove-bg", "upscale", "denoise", "enhance-face", "inpaint", "generative"} +) + + +def _normalize_ai_step(params: Any) -> Dict[str, Any]: + """Repair an ai_edit_image step's inner op so it always runs (restyle -> + generative, masked -> inpaint).""" + params = dict(params) if isinstance(params, dict) else {} + mode = str(params.get("mode") or "").strip().lower() + inner = str(params.get("op") or "").strip().lower() + if inner not in _AI_IMAGE_INNER_OPS: + inner = "inpaint" if mode == "masked" else "generative" + params["op"] = inner + if inner in ("inpaint", "generative") and not mode: + params["mode"] = "masked" if inner == "inpaint" else "instruction" + return params +PLAN_SCHEMA: Dict[str, Any] = { + "type": "object", + "properties": { + "reply": {"type": "string"}, + "steps": { + "type": "array", + "items": { + "type": "object", + "properties": { + "op": { + "type": "string", + "enum": list(_MODEL_PLAN_OPS), + "description": ( + "generate_edit = AI image model edit (params {prompt, " + "mode:'instruction'|'masked'}; do NOT add an inner 'op' — " + "instruction/restyle edits run as the image model's " + "'generative' op, masked region edits as 'inpaint'). " + "select_object = SAM mask by text (params {text}). " + "ask_user = ask with 2-3 choices (params {question, options})." + ), + }, + "params": {"type": "object"}, + }, + "required": ["op"], + }, + }, + "ask_user": { + "type": "object", + "properties": { + "question": {"type": "string"}, + "options": {"type": "array", "items": {"type": "string"}}, + }, + "required": ["question", "options"], + }, + "done": {"type": "boolean"}, + }, + "required": ["reply", "steps"], +} +PLAN_RESPONSE_FORMAT: Dict[str, Any] = { + "type": "json_schema", + "json_schema": {"name": "editor_plan", "schema": PLAN_SCHEMA}, +} + + +def _normalize_ask_user(payload: Any) -> Optional[Dict[str, Any]]: + """Validate an ask_user payload: one question and 2-3 concrete choices.""" + if not isinstance(payload, dict): + return None + question = str(payload.get("question") or "").strip() + raw = payload.get("options") + labels: List[str] = [] + for opt in (raw or []): + if isinstance(opt, str): + lbl = opt.strip() + elif isinstance(opt, dict): + lbl = str(opt.get("label") or "").strip() + else: + lbl = "" + if lbl: + labels.append(lbl) + labels = labels[:3] + if not question or len(labels) < 2: + return None + # Match the shape the ask_user tool + the editor client expect: option + # objects with a label (and an optional description). + return { + "question": question, + "options": [{"label": lbl, "description": ""} for lbl in labels], + "multi": False, + } + + +def _validate_plan(data: Any) -> Optional[Dict[str, Any]]: + """Validate a raw planner plan against the strict ops schema. + + Returns a normalized plan (aliases translated, ops checked against the + allowed set, ask_user normalized to 2-3 choices) or None when the plan is + invalid or empty. Callers turn None into an ask_user plan — never a silent + tone default. + """ + if not isinstance(data, dict): + return None + reply = str(data.get("reply") or "") + ask_user = _normalize_ask_user(data.get("ask_user")) + raw_steps = data.get("steps") + if raw_steps is None: + raw_steps = [] + if not isinstance(raw_steps, list): + return None + steps: List[Dict[str, Any]] = [] + for s in raw_steps[:_MAX_STEPS]: + if not isinstance(s, dict): + continue # drop a malformed step rather than rejecting the whole plan + op = str(s.get("op") or "") + if op == "ask_user": + au = _normalize_ask_user(s.get("params")) + if au is not None and ask_user is None: + ask_user = au + continue + op = _PLAN_OP_ALIASES.get(op, op) + if op not in _ALLOWED_PLAN_OPS: + continue # drop a single unknown/hallucinated op; keep the valid steps + params = s.get("params") if isinstance(s.get("params"), dict) else {} + if op == "ai_edit_image": + params = _normalize_ai_step(params) + steps.append({"op": op, "params": params}) + # Guardrails apply to model plans too (no stacked lifts, no brighten after + # retouch.skin, no global adjustment after a masked recolour). + steps = style_guide.enforce_guards(steps) + done = bool(data.get("done")) + if not steps and not done and ask_user is None: + return None + out: Dict[str, Any] = {"reply": reply, "planner": "llm", "steps": steps} + if ask_user is not None: + out["ask_user"] = ask_user + if done: + out["done"] = True + return out + +# Generative edits (remove/add/replace content) require an image provider. When +# none is available the agent must stop and say so plainly rather than apply a +# deterministic approximation that silently does the wrong thing. +try: + from src.ai_interaction import IMAGE_PROVIDER_REQUIRED_MESSAGE as _PROVIDER_REQUIRED_MESSAGE +except Exception: # pragma: no cover - defensive + _PROVIDER_REQUIRED_MESSAGE = ( + "Removing/adding objects needs an image generation provider; enable one in " + "Settings > Models." + ) + +_AI_CONTENT_OPS = frozenset({"ai_edit_image", "generate_image"}) + +# Model ids that are editing/inpainting models rather than text-to-image ones. +_EDIT_MODEL_RE = re.compile(r"edit|inpaint|fill|kontext|controlnet|removal|qwen-image-edit", re.I) + +# Content nouns that signal a generative object edit rather than a tone/filter +# tweak. Kept narrow so "make it black and white" / "add a vignette" stay +# deterministic. +_GEN_OBJECTS = ( + r"(?:persons?|people|men|man|women|woman|guys?|girls?|boys?|someone|somebody|" + r"objects?|car|cars|sign|signs|logos?|watermarks?|trees?|buildings?|" + r"reflections?|animals?|dogs?|cats?|birds?|hands?|fingers?|moons?|suns?|clouds?|" + r"stars?|fire|smoke|snow|rain|flowers?|mountains?|planets?|balloons?|rockets?|hats?|" + r"glasses|sunglasses|shades|beard|beards|stickers?|frames?|borders?|" + r"rainbows?|wildflowers?|sunflowers?|tulips?|daisies?|leaves|pumpkins?|lanterns?|" + r"butterflies?|bees?|boats?|sailboats?|kites?|umbrellas?)" +) +_REMOVE_RE = re.compile( + r"\b(?:remove|delete|erase|eliminate|get\s+rid\s+of|take\s+out|photoshop\s+out|paint\s+out)\b" + r"[^.!?\n]{0,60}?\b" + _GEN_OBJECTS + r"\b", + re.I, +) +_ADD_RE = re.compile( + r"\b(?:add|insert|put|place|draw|paint|create|generate)\b" + r"[^.!?\n]{0,40}?\b" + _GEN_OBJECTS + r"\b", + re.I, +) +_REPLACE_RE = re.compile( + r"\b(?:replace|swap|substitute)\b|\b(?:change|turn)\b[^.!?\n]{0,40}?\b(?:to|into)\b", + re.I, +) +# Removing diffuse clutter ("remove the distractions") is generative too, but +# the specific-noun _REMOVE_RE above does not catch it. +_REMOVE_CLEANUP_RE = re.compile( + r"\b(?:remove|delete|erase|eliminate|get\s+rid\s+of|take\s+out|clean\s+up|declutter|clear)\b" + r"[^.!?\n]{0,40}?\b(?:distractions?|clutter|junk|mess|noise|" + r"unwanted\s+(?:objects?|items?|things?)|stuff|things|blemishes?|spots?)\b", + re.I, +) + +# Changing the whole background/scene (or placing the subject somewhere new) +# is a generative replacement, not the local remove-bg op. "make the +# background nicer/better" is included; plain "blur the background" is not. +_BG_RE = re.compile( + r"\b(?:replace|change|swap|put|place|set|make|improve|enhance|beautify|upgrade)\b" + r"[^.!?\n]{0,40}?\b(?:background|backdrop|scene)\b" + r"|\b(?:put|place)\b[^.!?\n]{0,30}?\b(?:on|in|at)\s+(?:a|an|the)\s+" + r"(?:beach|street|mountains?|forest|city|park|ocean|sea|desert|room|stage|field|snow|" + r"night|moon|mars|space|island|jungle)\b", + re.I, +) +# Season/weather changes and an explicit "generative fill" change scene +# content (not just tone), so they must be a generative edit, never a +# colour adjustment/filter step. +_SEASON_WEATHER_RE = re.compile( + r"\b(?:snow(?:y|ing|fall)?|winter|autumn(?:al)?|fall\s+(?:leaves|foliage)|spring|" + r"rain(?:y|ing)?|fog(?:gy)?|mist(?:y)?|storm(?:y)?|overcast)\b", + re.I, +) +_GENERATIVE_FILL_RE = re.compile(r"\bgenerative(?:\s+fill)?\b", re.I) +# Whole-image artistic RESTYLE requests (turn the photo into a painting, anime, +# pixel art, ...). These are a model instruction edit over the whole frame, not a +# local adjustment. The client has a short fast-path regex; the server must +# generalise so unknown restyle phrasings still reach the model. +_RESTYLE_RE = re.compile( + r"\b(?:re[\s-]?style|painting|painterly|water[\s-]?colou?r|oil\s+paint(?:ing)?|" + r"acrylic|gouache|pastel|sketch|pencil|charcoal|ink\s+drawing|line\s+art|comic|" + r"cartoon|anime|manga|illustration|3d\s+render|cgi|claymation|clay|pixel\s+art|" + r"voxel|low[\s-]?poly|mosaic|stained\s+glass|pop\s+art|art\s+nouveau|ukiyo[\s-]?e|" + r"surreal|fantasy\s+art|steampunk|cyberpunk|vaporwave|synthwave|graffiti|woodcut|" + r"linocut|etching|lithograph|impressionist|cubist|abstract|doodle|vector\s+art|" + r"neon\s+sign|psychedelic|gothic|comic[\s-]?book|storybook)\b", + re.I, +) + +# Text/title/PSD/format requests are deterministic edits, never generative. +_TEXT_RE = re.compile( + r"\b(?:title|caption|subtitle|label|heading|watermark|text|word(?:ing)?)\b", re.I, +) +_TEXT_ADD_RE = re.compile(r"\b(?:add|insert|put|place|write|overlay|create|set)\b", re.I) +_STYLE_RE = re.compile( + r"\b(drop[\s-]?shadow|outer[\s-]?glow|glow|stroke|outline|emboss|bevel)\b", re.I, +) +_PSD_RE = re.compile(r"\b(?:psd|photoshop file|layered file)\b", re.I) + +# Aspect -> exact output size. Instagram/format intents crop to the aspect and +# then resize to this size (then optionally add a title). +_ASPECT_TARGETS = ( + (re.compile(r"\b(?:1080\s*[x×]\s*1350|4\s*:\s*5|four[\s-]?fifths?|instagram)\b", re.I), (1080, 1350)), + (re.compile(r"\b(?:1080\s*[x×]\s*1080|1\s*:\s*1|square)\b", re.I), (1080, 1080)), + (re.compile(r"\b(?:1080\s*[x×]\s*1920|9\s*:\s*16|story|reel|tiktok|vertical)\b", re.I), (1080, 1920)), + (re.compile(r"\b(?:1920\s*[x×]\s*1080|16\s*:\s*9|widescreen|landscape video)\b", re.I), (1920, 1080)), +) +_DIM_RE = re.compile(r"\b(\d{3,5})\s*[x×]\s*(\d{3,5})\b") + + +def _is_text_request(text: str) -> bool: + return bool(_TEXT_RE.search(text) and _TEXT_ADD_RE.search(text)) + + +def _format_target(text: str) -> Optional[Tuple[int, int]]: + for pattern, dims in _ASPECT_TARGETS: + if pattern.search(text): + return dims + m = _DIM_RE.search(text) + if m: + try: + return (int(m.group(1)), int(m.group(2))) + except Exception: + return None + return None + + +def _extend_requested(text: str) -> bool: + if re.search(r"\b(?:extend|expand|wider|taller|letterbox)\b", text, re.I): + return True + return bool(re.search( + r"\badd\b[^.!?\n]{0,25}?\b(?:canvas|border|space|margin)\b", text, re.I, + )) + + +def _generative_kind(prompt: str) -> Optional[str]: + """Classify a prompt as a generative content edit: 'remove'/'add'/'replace'. + + Returns None for tone/colour/filter/text/selection edits that deterministic + editor ops handle correctly. 'remove the background' is deliberately not + matched (that is the local remove-bg op). + """ + text = str(prompt or "") + if not text.strip(): + return None + # Text/title, PSD and format requests are deterministic — never generative. + if _is_text_request(text) or _PSD_RE.search(text) or _format_target(text): + return None + # An explicit brand-new image ("create/generate/make a new image/picture/logo + # ...") is a text-to-image generation, never an edit of the current photo. + if _is_new_image_request(text): + return "add" + # Replacing the background/scene (or putting the subject somewhere new) is + # a generative replacement even though it is phrased like a background op. + if _BG_RE.search(text): + return "replace" + if re.search( + r"\b(?:remove|delete|erase|eliminate|get\s+rid\s+of)\s+(?:the\s+)?background\b", + text, + re.I, + ): + return None + # Season/weather changes and explicit "generative fill" change content, not + # tone. Only treat them as generative when the user is not asking to REMOVE + # that element ("remove the rain" stays a removal). + if _GENERATIVE_FILL_RE.search(text) or _SEASON_WEATHER_RE.search(text): + if not re.search( + r"\b(?:remove|delete|erase|eliminate|get\s+rid\s+of|take\s+out)\b", text, re.I + ): + return "replace" + if _RESTYLE_RE.search(text) and not _is_new_image_request(text): + # A whole-image artistic restyle is a model instruction edit (keep the + # subject/composition), never a local tone/colour adjustment. + return "restyle" + if _REPLACE_RE.search(text): + return "replace" + if _ADD_RE.search(text): + return "add" + if _REMOVE_RE.search(text) or _REMOVE_CLEANUP_RE.search(text): + return "remove" + return None + + +_NEW_IMAGE_RE = re.compile( + r"\b(?:create|generate|make|produce|draw|design)\b[^.!?\n]{0,30}?\b" + r"(?:new\s+)?(?:image|picture|photo|illustration|artwork|graphic|logo|icon|wallpaper|poster)\b", + re.I, +) +_EDIT_CONTEXT_RE = re.compile( + r"\b(?:this|that|these|it|the\s+(?:photo|image|picture)|attached|from\s+this|" + r"of\s+(?:this|the\s+photo)|edit|in\s+(?:this|the)\s+photo)\b", + re.I, +) +_KEEP_CLAUSE_RE = re.compile(r"\bkeep\b|\bunchanged\b|\bpreserve\b|\bsame\b|\bidentical\b", re.I) + + +def _is_new_image_request(text: str) -> bool: + """True only for a from-scratch image generation (not an edit of an existing photo).""" + if not _NEW_IMAGE_RE.search(text): + return False + return not _EDIT_CONTEXT_RE.search(text) + + +def _instruction_prompt(text: str) -> str: + """Append a keep-unchanged clause so instruction edits preserve the subject.""" + t = str(text or "").strip() + if not t: + return t + if _KEEP_CLAUSE_RE.search(t): + return t + return ( + t.rstrip(".") + + ". Keep the main subject and everything else in the image exactly the same." + ) + + +def _restyle_prompt(text: str) -> str: + """Whole-image artistic restyle instruction (subject/composition preserved).""" + t = str(text or "").strip() + if t: + base = ( + "Restyle the entire image as requested, applying the artistic style " + "across the whole frame: " + t + ) + else: + base = "Restyle the entire image in a new artistic style." + return _instruction_prompt(base) + + +def _generative_mode(kind: str, prompt: str) -> str: + """'masked' only for an explicit regional removal of a clearly identified object. + + Everything else generative (background/season/style replacement, adding an + object to the scene, removing diffuse distractions) uses 'instruction'. + """ + if kind == "remove": + text = str(prompt or "") + if _REMOVE_RE.search(text) and not _REMOVE_CLEANUP_RE.search(text): + return "masked" + return "instruction" + + +def _removal_prompt(text: str) -> str: + """A more explicit diffuse-removal instruction that names common clutter.""" + t = str(text or "").strip() or "Remove the distracting clutter from the photo" + return ( + t.rstrip(". ") + + ". Remove any distracting clutter such as roads, guardrails, utility poles, " + "signs, wires and debris, and fill the cleared area with the surrounding " + "landscape so it looks natural." + ) + + +# Rich, scene-specific descriptions so a background replacement produces a +# convincing scene (Qwen-Image-Edit is an instruction model and a bare "a beach" +# tends to yield a flat blue backdrop). Keyed by keywords in the request. +_SCENE_HINTS = ( + (r"beach|shore|seaside|coast|tropical", + "a sunny tropical beach", + "golden sand, turquoise sea with gentle waves, a clear blue sky with soft " + "clouds, and natural daylight"), + (r"mountain|alps|peak|valley", + "a scenic mountain landscape", + "distant peaks, natural daylight and soft haze"), + (r"forest|woods|jungle", + "a lush green forest", + "dappled natural daylight and depth"), + (r"city|street|urban|downtown", + "a realistic city street", + "recognisable city buildings and depth of field with natural perspective"), + (r"field|meadow|grass", + "an open grassy field", + "natural daylight and a distant horizon"), + (r"snow|winter", + "a snow-covered landscape", + "cold, soft daylight and a pale sky"), + (r"desert|dunes", + "a warm desert", + "sand dunes and a clear sky"), + (r"studio|plain|neutral|solid", + "a clean neutral studio backdrop", + "soft, even lighting"), +) +_SCENE_DEFAULT = ( + "a realistic natural background", + "matching the subject's lighting, perspective and shadows", +) +# Season/weather scene descriptions ("make it snowy/autumn"). +_SEASON_HINTS = ( + (r"snow|winter|snowy", "a snowy winter scene", + "fresh snow covering the ground and surfaces, soft cold daylight, a pale overcast sky"), + (r"autumn|autumnal|fall", "an autumn scene", + "vivid orange and golden foliage, warm low sunlight and fallen leaves"), + (r"spring", "a bright spring scene", + "fresh green foliage and blossoms under bright natural daylight"), + (r"rain|storm|overcast", "a rainy overcast scene", + "wet reflective surfaces, raindrops and a dramatic overcast sky"), + (r"fog|mist", "a foggy scene", + "soft drifting mist and diffuse light"), + (r"sky|clouds", "a new sky", + "realistic clouds and natural lighting matching the scene"), +) + + +def _scene_prompt(text: str) -> str: + """Deterministic, templated instruction for a background/scene change. + + Qwen-Image-Edit is an instruction model and a bare scene noun tends to yield + a flat backdrop, so the scene is expanded into concrete detail. The result is + passed to the provider unmodified. + """ + t = str(text or "").strip() + if not t: + return t + if _BG_RE.search(t): + scene, details = _SCENE_DEFAULT + for pattern, scene_name, description in _SCENE_HINTS: + if re.search(pattern, t, re.I): + scene, details = scene_name, description + break + return ( + f"Replace the entire background behind the person with {scene}: {details}. " + "Keep the person exactly the same." + ) + if _SEASON_WEATHER_RE.search(t) or _GENERATIVE_FILL_RE.search(t): + scene, details = _SCENE_DEFAULT + for pattern, scene_name, description in _SEASON_HINTS: + if re.search(pattern, t, re.I): + scene, details = scene_name, description + break + return ( + f"Change the scene to {scene}: {details}. " + "Keep the main subject and everything else in the image exactly the same." + ) + return t + + +def _generative_step(kind: str, prompt: str) -> Dict[str, Any]: + """The AI step a generative intent must use instead of deterministic ops.""" + instruction = str(prompt or "").strip() + if kind == "restyle": + return { + "op": "ai_edit_image", + "params": {"op": "generative", "mode": "instruction", + "prompt": _restyle_prompt(instruction), "steps": 8}, + } + if kind == "add": + # A brand-new image is the only case for generate_image; adding an object + # to an existing photo is an instruction edit of the whole image. + if _is_new_image_request(prompt): + return {"op": "generate_image", "params": {"prompt": instruction}} + return { + "op": "ai_edit_image", + "params": { + "op": "generative", + "mode": "instruction", + "prompt": _instruction_prompt(instruction), + "steps": 8, + }, + } + mode = _generative_mode(kind, prompt) + op = "inpaint" if kind == "remove" else "generative" + if mode == "instruction": + if kind == "remove": + base = _removal_prompt(instruction) + else: + base = _scene_prompt(instruction) + params: Dict[str, Any] = {"op": op, "mode": "instruction", "prompt": _instruction_prompt(base)} + else: + params = {"op": op, "mode": "masked", "prompt": instruction} + if kind in ("remove", "replace"): + # Removal and background-replacement edits need more sampler steps than + # the server's default of 4. + params["steps"] = 8 + return {"op": "ai_edit_image", "params": params} + + +_SKY_ADD_RE = re.compile( + r"\b(?:sky|moons?|suns?|sunset|sunrise|clouds?|stars?|night|aurora|stars?)\b", + re.I, +) +_REMOVE_NOUN_RE = re.compile( + r"\b(?:remove|delete|erase|eliminate|get\s+rid\s+of|take\s+out|clean\s+up|declutter|clear)\b" + r"[^.!?\n]{0,60}?\b(" + + _GEN_OBJECTS + + r"|distractions?|clutter|junk|mess|noise|stuff|things)\b", + re.I, +) + + +def _selection_step_for(kind: str, prompt: str) -> Optional[Dict[str, Any]]: + """A deterministic selection step that yields the mask a MASKED ai_edit_image needs. + + Instruction-mode edits operate on the whole image and need no mask, so this + returns None for them. Masked mode is only used for an explicit regional + object removal; the selection targets that object so ``selection.exportMask`` + has a tight region to export. + """ + text = str(prompt or "") + if kind == "remove" and _generative_mode(kind, text) == "masked": + m = _REMOVE_NOUN_RE.search(text) + label = (m.group(1).lower() if m else "person") + if label.endswith("s"): + label = label[:-1] + if label in ("people",): + label = "person" + return {"op": "selection.subject", "params": {"label": label}} + return None + + +def _generative_steps(kind: str, prompt: str) -> List[Dict[str, Any]]: + """The selection step (when needed) followed by the AI content step.""" + steps: List[Dict[str, Any]] = [] + selection = _selection_step_for(kind, prompt) + if selection: + steps.append(selection) + steps.append(_generative_step(kind, prompt)) + return steps + + +def _plan_uses_ai_content(steps: Any) -> bool: + return any(isinstance(s, dict) and s.get("op") in _AI_CONTENT_OPS for s in (steps or [])) + + +def _ensure_generative(prompt: str, steps: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + """Force a generative intent onto an AI content op, never a substitute. + + If the prompt is a generative content edit (remove/add/replace) but the + plan has no ai_edit_image/generate_image step, replace the plan with the + correct AI step so the agent cannot silently do an unrelated deterministic + edit. Non-generative prompts are returned unchanged. + """ + kind = _generative_kind(prompt) + if not kind: + return steps + # Background/scene replacement is deterministic: always use the templated + # scene prompt so a model rewrite cannot drop the scene details (a model + # ai_edit_image step would otherwise pass with its own vague prompt). + if kind == "replace" and ( + _BG_RE.search(str(prompt or "")) + or _SEASON_WEATHER_RE.search(str(prompt or "")) + or _GENERATIVE_FILL_RE.search(str(prompt or "")) + ): + return _generative_steps(kind, prompt) + # For a restyle (and any other generative intent) the planner is expected to + # choose generate_edit itself; the _RESTYLE_RE keyword list is only a fast + # path used when the model did not already pick an AI content op. + if not _plan_uses_ai_content(steps): + return _generative_steps(kind, prompt) + return steps + + +def _image_provider_available(owner: Optional[str]) -> bool: + try: + from src.ai_interaction import image_editing_provider_available + + return bool(image_editing_provider_available(owner)) + except Exception: + return True # unknown: let the tool attempt and report its own error + + +def _state_dims(state: Any) -> Optional[Tuple[int, int]]: + """Current canvas size from the client-sent state (flat or nested).""" + if not isinstance(state, dict): + return None + candidates = [state] + for key in ("document", "document_info", "info"): + if isinstance(state.get(key), dict): + candidates.append(state[key]) + for src in candidates: + w = src.get("width") or src.get("imgWidth") + h = src.get("height") or src.get("imgHeight") + try: + if w and h: + return (int(round(float(w))), int(round(float(h)))) + except Exception: + continue + return None + + +def _title_text(prompt: str) -> Optional[str]: + quoted = re.search(r"[\"'\u201c\u201d\u2018\u2019]([^\"'\u201c\u201d\u2018\u2019]{1,40})[\"'\u201c\u201d\u2018\u2019]", prompt) + if quoted and quoted.group(1).strip(): + return quoted.group(1).strip() + caps = re.search(r"\b([A-Z][A-Z0-9]{1,20})\b", prompt) + if caps and caps.group(1).lower() not in {"add", "title", "text", "psd"}: + return caps.group(1) + return None + + +def _crop_rect_for_aspect(cur: Tuple[int, int], target: Tuple[int, int]) -> Optional[Dict[str, int]]: + cw, ch = cur + tw, th = target + if cw <= 0 or ch <= 0 or tw <= 0 or th <= 0: + return None + target_ratio = tw / th + cur_ratio = cw / ch + if abs(cur_ratio - target_ratio) < 0.02: + return None # already the right aspect + if cur_ratio > target_ratio: + # too wide -> trim the sides + new_w = int(round(ch * target_ratio)) + new_w = min(cw, max(1, new_w)) + return {"x": (cw - new_w) // 2, "y": 0, "width": new_w, "height": ch} + # too tall -> trim top/bottom + new_h = int(round(cw / target_ratio)) + new_h = min(ch, max(1, new_h)) + return {"x": 0, "y": (ch - new_h) // 2, "width": cw, "height": new_h} + + +def _extend_dims(cur: Tuple[int, int], target: Tuple[int, int]) -> Tuple[int, int]: + """Target canvas that grows (never crops) toward the target aspect.""" + cw, ch = cur + target_ratio = target[0] / target[1] + cur_ratio = cw / ch + if abs(cur_ratio - target_ratio) < 0.02: + return (cw, ch) + if cur_ratio < target_ratio: + return (int(round(ch * target_ratio)), ch) # add width + return (cw, int(round(cw / target_ratio))) # add height + + +def _plan_text_steps(prompt: str) -> List[Dict[str, Any]]: + title = _title_text(prompt) or "Title" + steps: List[Dict[str, Any]] = [{ + "op": "text.add", + "params": {"text": title, "x": 40, "y": 40, "color": "#ffffff", "size": 64}, + }] + if _STYLE_RE.search(prompt): + steps.append({"op": "layers.addStyle", "params": {"type": "drop-shadow", "params": {}}}) + return steps + + +def _plan_format_steps(prompt: str, state: Any, provider_available: bool) -> Optional[List[Dict[str, Any]]]: + target = _format_target(prompt) + if not target: + return None + cur = _state_dims(state) + if not cur: + # No canvas size known: ask the client to crop to the aspect itself. + return None + steps: List[Dict[str, Any]] = [] + if _extend_requested(prompt): + new_w, new_h = _extend_dims(cur, target) + if (new_w, new_h) == cur: + return None + if provider_available: + steps.append({"op": "ai.generativeExpand", "params": { + "width": new_w, "height": new_h, + "anchor": "center", "mode": "instruction", "feather": True, + "steps": 8, + "prompt": "extend the surrounding scene and sky naturally into the new border, " + "keep the original photo unchanged"}}) + else: + steps.append({"op": "document.resizeCanvas", "params": { + "width": new_w, "height": new_h, "anchor": "center"}}) + else: + rect = _crop_rect_for_aspect(cur, target) + if rect: + steps.append({"op": "document.crop", "params": rect}) + # Only resize when an exact output size was requested. + if abs(target[0] / target[1] - cur[0] / cur[1]) >= 0.02 or target in ((1080, 1350), (1080, 1080), (1080, 1920), (1920, 1080)): + steps.append({"op": "document.resizeCanvas", "params": { + "width": target[0], "height": target[1], "anchor": "center"}}) + if not steps: + return None + # Optional caption/title that came with the format request. + if _is_text_request(prompt): + steps.extend(_plan_text_steps(prompt)) + return steps + + +def _plan_psd_steps(prompt: str) -> Optional[List[Dict[str, Any]]]: + if _PSD_RE.search(prompt): + return [{"op": "document.savePsd", "params": {}}] + return None + + +def _plan_special( + prompt: str, state: Any, provider_available: bool, +) -> Optional[Tuple[List[Dict[str, Any]], str, str]]: + """Deterministic overrides for intents the model reliably botches. + + Canvas format/crop/extend, text + layer styles and PSD export must not be + left to the model. Returns (steps, reply, note) or None. These never need an + image provider except generative border fill, used only when one exists. + """ + # Targeted recolour is fully deterministic (object mask + masked hue shift) + # and must never be left to the model, which sometimes substitutes a + # generative placeImage. Highest priority so it can never vary. + recolor = style_guide.recolor_plan(prompt) + if recolor: + return list(recolor["steps"]), str(recolor["reply"]), "" + if _format_target(prompt): + steps = _plan_format_steps(prompt, state, provider_available) + if not steps: + return None + target = _format_target(prompt) + reply = f"Setting the canvas to {target[0]}\u00d7{target[1]}." + note = "" + if _extend_requested(prompt) and not provider_available: + note = ("No image generation provider is configured, so the new border " + "was extended with edge/transparent pixels.") + return steps, reply, note + if _PSD_RE.search(prompt): + steps = _plan_psd_steps(prompt) + return (steps, "Saving a layered PSD.", "") if steps else None + if _is_text_request(prompt): + steps = _plan_text_steps(prompt) + if steps: + reply = ("Adding the title with a drop shadow." + if _STYLE_RE.search(prompt) else "Adding the title text.") + return steps, reply, "" + # Structural ops (fill, text edit, align, rotate/flip, copy/paste, flatten, + # merge-down, colour overlay) — the guide's deterministic routing so the + # model never substitutes a generic adjustment/transform for them. + structural = style_guide.structural_plan(prompt) + if structural: + return list(structural["steps"]), str(structural["reply"]), "" + return None + + +_DIMS_CLAIM_RE = re.compile(r"(\d{2,5})\s*[x\u00d7]\s*(\d{2,5})") +_CANVAS_CHANGE_OPS = frozenset({ + "document.crop", "document.resizeCanvas", "ai.generativeExpand", "document.importImage", +}) +_TEXT_OPS = frozenset({"text.add", "layers.addStyle", "document.savePsd"}) + + +def _step_op_signature(step: Any) -> str: + """Identify an executed step by op plus its adjustment type / filter id.""" + if not isinstance(step, dict): + return "" + op = str(step.get("op") or "") + params = step.get("params") or {} + if op in ("adjustments.apply", "adjustments.applyMasked"): + return f"{op}:{params.get('type') or ''}" + if op in ("filters.apply", "filters.applyMasked"): + return f"{op}:{params.get('id') or ''}" + return op + + +# Phrases that name a specific edit. If the reply claims one of these but the +# executed steps do not contain it, the reply is rewritten from the real steps. +_OP_CLAIM_KEYWORDS: List[Tuple[str, Tuple[str, ...]]] = [ + ("photo.autoTone", ("auto tone", "auto-tone", "autotone", "auto-tune")), + ("adjustments.apply:shadows-highlights", ("shadow", "highlight")), + ("adjustments.apply:white-balance", ("white balance", "white-balance", + "temperature", "colour cast", "color cast")), + ("adjustments.apply:vibrance", ("vibrance",)), + ("adjustments.apply:brightness-contrast", ("brightness", "contrast")), + ("adjustments.apply:black-white", ("black and white", "black & white", + "grayscale", "greyscale", "monochrome")), + ("filters.apply:clarity", ("clarity", "local contrast")), + ("filters.apply:vignette", ("vignette",)), + ("filters.apply:sharpen", ("sharpen", "crisper")), + ("retouch.skin", ("skin", "complexion")), + ("retouch.teeth", ("teeth", "smile")), +] + +_OP_LABELS: Dict[str, str] = { + "photo.autoTone": "auto tone", + "adjustments.apply:shadows-highlights": "a shadow lift (highlights protected)", + "adjustments.apply:white-balance": "white balance", + "adjustments.apply:vibrance": "vibrance", + "adjustments.apply:brightness-contrast": "brightness/contrast", + "adjustments.apply:black-white": "black and white", + "filters.apply:clarity": "local contrast (clarity)", + "filters.apply:vignette": "a light vignette", + "filters.apply:sharpen": "sharpening", + "selection.face": "a face selection", + "selection.background": "a background selection", + "retouch.skin": "gentle skin retouching", + "retouch.teeth": "teeth whitening", +} + + +def _reply_from_ops(steps: List[Dict[str, Any]]) -> str: + """Describe what was actually executed, never what was planned.""" + labels: List[str] = [] + for step in steps or []: + sig = _step_op_signature(step) + label = _OP_LABELS.get(sig) + if label is None: + label = sig.split(":")[0] or "an edit" + if label not in labels: + labels.append(label) + if not labels: + return "Done." + return "Done \u2014 I applied " + ", ".join(labels) + "." + + +def _reply_matches_ops(planned_reply: str, steps: List[Dict[str, Any]]) -> bool: + """True when the reply does not claim an edit that was not executed.""" + text = (planned_reply or "").lower() + if not text: + return True + # "drop shadow" is a layer style, not a shadows/highlights adjustment. + text = text.replace("drop shadow", "drop-shadow-style") + executed = {_step_op_signature(step) for step in (steps or [])} + for sig, keywords in _OP_CLAIM_KEYWORDS: + if sig in executed: + continue + if any(keyword in text for keyword in keywords): + return False + return True + + + +def _verified_reply( + prompt: str, + steps: List[Dict[str, Any]], + results: List[Dict[str, Any]], + final_state: Any, + planned_reply: str, +) -> str: + """Build the final reply from ACTUAL executed results + final document state. + + Never let a plan's optimistic prose claim an edit that did not happen. + """ + failed = [ + r for r in (results or []) + if isinstance(r, dict) and r.get("ok") is False + ] + if failed: + op = str(failed[0].get("op") or "step") + err = failed[0].get("error") or "it did not complete" + return (f"I couldn't finish the {op} step ({err}). " + f"Nothing else was left changed — the image is back as it was.") + dims = _state_dims(final_state) + step_ops = [str(s.get("op") or "") for s in (steps or []) if isinstance(s, dict)] + claim = _DIMS_CLAIM_RE.search(planned_reply or "") + if claim and dims: + try: + claimed = (int(claim.group(1)), int(claim.group(2))) + except Exception: + claimed = None + if claimed and (abs(dims[0] - claimed[0]) > 2 or abs(dims[1] - claimed[1]) > 2): + if not any(op in _CANVAS_CHANGE_OPS for op in step_ops): + return (f"I planned to make it {claimed[0]}\u00d7{claimed[1]}, but the canvas is " + f"still {dims[0]}\u00d7{dims[1]} — the resize didn't apply. " + f"Please try again in the editor.") + if not steps and not results: + return planned_reply or "I didn't have a concrete edit to apply." + if not _is_text_request(prompt) and not _reply_matches_ops(planned_reply, steps): + return _reply_from_ops(steps) + if _is_text_request(prompt) and any(op in _TEXT_OPS for op in step_ops) and not failed: + return planned_reply or "Added the text." + if dims and any(op in _CANVAS_CHANGE_OPS for op in step_ops): + return (planned_reply or f"Done — the canvas is now {dims[0]}\u00d7{dims[1]}.") + return planned_reply + + +def set_editor_planner( + fn: Optional[Callable[[List[dict], Optional[str]], Any]], +) -> None: + global _EDITOR_PLANNER + _EDITOR_PLANNER = fn + + +_ROUTING_GUIDANCE = ( + "Routing rules (follow strictly): removing objects/people, adding new content " + "(e.g. a moon, an animal, a sign), or replacing/changing what is in the picture " + "are GENERATIVE edits -> use op \"ai_edit_image\" (op \"inpaint\" to remove, " + "\"generative\" to replace, with a descriptive prompt) or op \"generate_image\" " + "({prompt}) for added content. NEVER approximate these with selection.rectangle, " + "adjustments.apply, filters.apply, transform.* or any other deterministic op. " + "Use deterministic ops only for tone/colour/filter/crop/resize/text/selection " + "edits that do not invent or delete picture content. \"remove the background\" " + "is op \"ai_edit_image\" op \"remove-bg\". If a generative edit is requested and " + "no image provider is configured, do NOT substitute a deterministic edit: return " + "{\"reply\": \"" + _PROVIDER_REQUIRED_MESSAGE + "\", \"steps\": []}." +) + +# Strict ops vocabulary for the model (the schema is also sent as a structured +# output constraint, so the model can only emit these ops). +_SCHEMA_NOTE = ( + "Reply with ONLY a JSON object matching the ops schema. Valid step ops are the " + "deterministic editor ops above plus: \"generate_edit\" (a model image edit; " + "params {\"prompt\": str, \"mode\": \"instruction\"|\"masked\", \"mask\": optional " + "object description}) for any edit that invents, removes, replaces, recolours or " + "restyles image content; \"select_object\" (params {\"text\": object description}) " + "to select an object by description; and \"ask_user\" (params {\"question\": str, " + "\"options\": [2-3 short choices]}) when the request is ambiguous. For " + "\"generate_edit\" set params {\"prompt\": str, \"mode\": \"instruction\"|\"masked\"} " + "and do NOT set an inner \"op\" field: the server maps instruction/restyle edits to " + "the image model's \"generative\" op and masked region edits to \"inpaint\". Prefer " + "generate_edit over deterministic approximations for content changes, and ask_user " + "with 2-3 concrete choices rather than guessing a tone tweak." +) + +_SYSTEM_PROMPT = ( + "You are a photo-editor planning agent. Convert the user's request into a JSON plan of " + "editor operations. Return ONLY JSON: {\"reply\": str, \"steps\": [{\"op\": str, \"params\": {}}]}. " + "Available ops: " + ", ".join(OP_CATALOG) + ", plus ai_edit_image and generate_image. " + + _ROUTING_GUIDANCE + " " + + _CANONICAL_GUIDANCE + " " + _STYLE_GUIDANCE + " " + "Use selection.* before targeted edits when possible. Keep to <=6 steps. " + "If a screenshot is attached, use it to judge the current result. " + + _SCHEMA_NOTE +) + +_CHECK_SYSTEM_PROMPT = ( + "You are a photo-editor quality checker. A screenshot of the editor's CURRENT result is " + "attached along with the user's request and the iteration number. Decide whether the edit " + "satisfies the request. Return ONLY JSON: if it is complete, {\"done\": true, \"reply\": str}; " + "if more work is needed, {\"done\": false, \"reply\": str, \"steps\": [{\"op\": str, \"params\": {}}]}. " + "Available ops: " + ", ".join(OP_CATALOG) + ", plus ai_edit_image and generate_image. " + + _ROUTING_GUIDANCE + " " + + _CANONICAL_GUIDANCE + " " + _STYLE_GUIDANCE + " " + "Never repeat steps that already ran; <=4 steps. " + + _SCHEMA_NOTE +) + + +_GENERIC_TONE_OPS = frozenset({"photo.autoTone", "adjustments.apply", "filters.apply"}) + + +def _plan_is_generic_tone_only(steps: Any) -> bool: + """True when a plan is nothing but whole-image tone/colour/filter steps.""" + items = [s for s in (steps or []) if isinstance(s, dict)] + return bool(items) and all(s.get("op") in _GENERIC_TONE_OPS for s in items) + + +def _unsure_plan(prompt: str, plan: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]: + """Demote a bare whole-image tone plan to an ask when the prompt is unsure. + + When the planner produced ONLY generic tone ops (photo.autoTone / + adjustments.apply / filters.apply) for a prompt that matches no concrete + intent and names no concrete defect, the editor is guessing ("it just adds + vibrance and brightness"). Ask the user with 2-3 concrete choices instead. + """ + if not isinstance(plan, dict): + return plan + steps = plan.get("steps") or [] + if not _plan_is_generic_tone_only(steps): + return plan + if _generative_kind(prompt): + return plan + if style_guide.has_concrete_intent(prompt): + return plan + return style_guide.ask_user_plan() + + +def _fallback_plan(prompt: str) -> Dict[str, Any]: + kind = _generative_kind(prompt) + if kind: + label = {"remove": "object removal", "add": "content addition", + "replace": "content replacement", "restyle": "artistic restyle"}[kind] + return { + "reply": f"Planned an AI {label}.", + "planner": "fallback", + "steps": _generative_steps(kind, prompt), + } + guided = style_guide.fallback_plan(prompt) + if guided is not None: + guided["steps"] = style_guide.enforce_guards(guided.get("steps") or []) + return guided + # Nothing mapped to a concrete intent: ASK instead of silently applying a + # default tone tweak. The old fallback emitted a generic + # photo.autoTone/adjustments/filters stack here, which is the "it just adds + # vibrance and brightness" bug for unrecognized prompts. + return style_guide.ask_user_plan() + + +def _parse_plan(raw: Any) -> Optional[Dict[str, Any]]: + if isinstance(raw, dict): + data = raw + else: + text = raw if isinstance(raw, str) else "" + if not text: + return None + try: + data = json.loads(text) + except Exception: + start, end = text.find("{"), text.rfind("}") + if start < 0 or end <= start: + return None + try: + data = json.loads(text[start:end + 1]) + except Exception: + return None + # Every plan is validated against the strict ops schema; an invalid or empty + # plan returns None so the caller falls back to ask_user (never a tone + # default). See _validate_plan. + return _validate_plan(data) + + +def _normalize_steps(body: Dict[str, Any]) -> List[Dict[str, Any]]: + if isinstance(body.get("steps"), list): + raw = body["steps"] + elif body.get("op"): + raw = [{"op": body["op"], "params": body.get("params") or {}}] + else: + raw = [] + out: List[Dict[str, Any]] = [] + for step in raw[:_MAX_STEPS]: + if not isinstance(step, dict): + continue + op = str(step.get("op") or "") + if op not in _OP_SET: + continue + out.append({"op": op, "params": step.get("params") or {}}) + return out + + +def _user(request: Request) -> Optional[str]: + try: + from src.auth_helpers import get_current_user + return get_current_user(request) + except Exception: + return None + + +async def _default_editor_planner(messages: List[dict], owner: Optional[str]) -> Optional[str]: + """Plan with the local editor-agent model, falling back through the chain. + + Order: per-request ``agent_model`` override -> local Qwen3.5-9B (vision, + screenshot attached) -> the same model text-only with a VL caption of the + screenshot -> the hosted default/utility chat model. The model actually + used is recorded so the route can report it. + """ + result = await editor_agent_model.plan_with_editor_agent( + messages, owner, override=editor_agent_model.get_override(), + response_format=PLAN_RESPONSE_FORMAT, + ) + if result is None: + return None + raw, model_used, source = result + editor_agent_model.set_last_model(model_used, source) + return raw + + +def setup_editor_agent_routes() -> APIRouter: + router = APIRouter(tags=["editor-agent"]) + + if _EDITOR_PLANNER is None: + set_editor_planner(_default_editor_planner) + + # Probe the local editor-agent model server (non-blocking) so the planner + # can use it for vision planning when reachable. + editor_agent_model.startup_probe() + + # scoped session -> list of that session's live subscriber queues; id -> result future. + # A command is BROADCAST to every live subscriber (per-connection queue), never to a + # single shared queue: EventSource reconnects overlap, and with one shared queue a + # command could be consumed by a stale subscriber while the active page got nothing. + subscribers: Dict[str, List["asyncio.Queue[dict]"]] = {} + backlog: Dict[str, List[dict]] = {} + pending: Dict[str, asyncio.Future] = {} + + def _subscribe(session: str) -> "asyncio.Queue[dict]": + q: "asyncio.Queue[dict]" = asyncio.Queue(maxsize=_QUEUE_MAX) + subscribers.setdefault(session, []).append(q) + # Replay anything enqueued while this session had no live subscriber. + for cmd in backlog.pop(session, []) or []: + try: + q.put_nowait(cmd) + except asyncio.QueueFull: + break + return q + + def _unsubscribe(session: str, q: "asyncio.Queue[dict]") -> None: + subs = subscribers.get(session) + if subs and q in subs: + subs.remove(q) + if not subs: + subscribers.pop(session, None) + + def _broadcast(session: str, cmd: dict) -> int: + subs = subscribers.get(session) or [] + if subs: + for q in list(subs): + try: + q.put_nowait(cmd) + except asyncio.QueueFull: + pass + return len(subs) + # No live subscriber yet: hold it so a subscriber that connects shortly + # after still receives the command (bounded ring buffer). + bl = backlog.setdefault(session, []) + bl.append(cmd) + if len(bl) > _QUEUE_MAX: + del bl[0: len(bl) - _QUEUE_MAX] + return 0 + + def _scoped(session: Optional[str], owner: Optional[str]) -> str: + return f"{owner or 'anon'}:{session or 'default'}" + + @router.post("/api/editor/plan") + async def plan(request: Request) -> JSONResponse: + owner = _user(request) + try: + body = await request.json() + except Exception: + body = {} + prompt = str(body.get("prompt") or "").strip() + state = body.get("state") or {} + mode = str(body.get("mode") or "plan").strip().lower() + screenshot = body.get("screenshot") + agent_model = str(body.get("agent_model") or "").strip() + if not prompt and mode != "check": + return JSONResponse({"reply": "No prompt provided.", "planner": "none", "steps": [], + "agent_model": agent_model or editor_agent_model.local_model()}) + + system = _CHECK_SYSTEM_PROMPT if mode == "check" else _SYSTEM_PROMPT + text = json.dumps({"prompt": prompt, "state": state}, ensure_ascii=False) + content: Any = text + if isinstance(screenshot, str) and screenshot.startswith("data:image/"): + content = [ + {"type": "text", "text": text}, + {"type": "image_url", "image_url": {"url": screenshot}}, + ] + messages = [ + {"role": "system", "content": system}, + {"role": "user", "content": content}, + ] + + raw: Any = None + planner = _EDITOR_PLANNER + editor_agent_model.set_override(agent_model) + editor_agent_model.set_last_model("", "") + if planner is not None: + try: + raw = planner(messages, owner) + if inspect.isawaitable(raw): + raw = await raw + except Exception: + raw = None + _used = editor_agent_model.get_last_model() + model_used = (_used[0] if _used and _used[0] + else (agent_model or editor_agent_model.local_model())) + parsed = _parse_plan(raw) + if isinstance(parsed, dict): + parsed["agent_model"] = model_used + if mode != "check": + _special = _plan_special(prompt, state, _image_provider_available(owner)) + if _special: + _special_steps, _special_reply, _ = _special + return JSONResponse({ + "reply": _special_reply, + "planner": "deterministic", + "steps": style_guide.enforce_guards(_special_steps) or _special_steps, + "agent_model": model_used, + }) + if parsed and mode != "check": + _strengthened = style_guide.strengthen(prompt, list(parsed.get("steps") or [])) + if _strengthened != parsed.get("steps"): + parsed["steps"] = _strengthened + parsed["planner"] = str(parsed.get("planner") or "llm") + "+guide" + # Keep /plan and /agent consistent: a generative intent must never be + # left on a deterministic substitute (e.g. a model returning only + # selection/adjustment steps for 'remove the person on the left'). + _generative = _ensure_generative(prompt, list(parsed.get("steps") or [])) + if _generative != parsed.get("steps"): + parsed["steps"] = _generative + parsed["planner"] = str(parsed.get("planner") or "llm") + "+generative" + # Same safety guards the /agent path applies, so /plan never returns + # a stacked-lift or a global adjustment after a masked recolour. + parsed["steps"] = style_guide.enforce_guards(list(parsed.get("steps") or [])) + # A bare whole-image tone plan for an unspecified prompt is a guess: + # ask the user instead of silently tweaking. + _maybe = _unsure_plan(prompt, parsed) + if _maybe is not parsed and isinstance(_maybe, dict): + _maybe["agent_model"] = model_used + parsed = _maybe + + if mode == "check": + # Never let a self-check re-plan substitute a generative placeImage + # for a deterministic recolour; re-issue the masked-hue plan instead. + _recolor = style_guide.recolor_plan(prompt) + if _recolor and _plan_uses_ai_content(list((parsed or {}).get("steps") or [])): + return JSONResponse({ + "reply": str(_recolor["reply"]), + "planner": "deterministic", + "steps": style_guide.enforce_guards(list(_recolor["steps"])), + "agent_model": model_used, + }) + if parsed and (parsed.get("done") or parsed.get("steps")): + return JSONResponse(parsed) + return JSONResponse({ + "done": True, + "reply": (parsed or {}).get("reply") or "The edit looks complete.", + "planner": (parsed or {}).get("planner") or "check", + "steps": [], + "agent_model": model_used, + }) + if parsed: + return JSONResponse(parsed) + _fallback = _fallback_plan(prompt) + _fallback["agent_model"] = model_used + return JSONResponse(_fallback) + + @router.post("/api/editor/bridge/command") + async def bridge_command(request: Request) -> JSONResponse: + owner = _user(request) + try: + body = await request.json() + except Exception: + body = {} + session = _scoped(body.get("session"), owner) + steps = _normalize_steps(body) + wait = bool(body.get("wait")) + timeout = float(body.get("timeout") or _BRIDGE_DEFAULT_TIMEOUT) + ids: List[str] = [] + wait_ids: List[str] = [] + for step in steps: + cid = uuid.uuid4().hex + ids.append(cid) + if wait: + pending[cid] = asyncio.get_event_loop().create_future() + wait_ids.append(cid) + _broadcast(session, {"id": cid, "op": step["op"], "params": step["params"]}) + if wait and len(wait_ids) == 1: + try: + result = await asyncio.wait_for(pending[wait_ids[0]], timeout=timeout) + return JSONResponse(result) + except asyncio.TimeoutError: + pending.pop(wait_ids[0], None) + return JSONResponse({"ok": False, "error": "timeout", "id": wait_ids[0]}) + return JSONResponse({"queued": ids, "session": session}) + + @router.get("/api/editor/bridge/events") + async def bridge_events(request: Request, session: str = "default") -> StreamingResponse: + owner = _user(request) + scoped = _scoped(session, owner) + + async def gen(): + q = _subscribe(scoped) + try: + yield ": connected\n\n" + while not _shutdown.is_shutting_down(): + try: + cmd = await asyncio.wait_for(q.get(), timeout=15) + yield f"data: {json.dumps(cmd)}\n\n" + except asyncio.TimeoutError: + yield ": keepalive\n\n" + finally: + _unsubscribe(scoped, q) + + return StreamingResponse(gen(), media_type="text/event-stream", + headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}) + + @router.post("/api/editor/bridge/result") + async def bridge_result(request: Request) -> JSONResponse: + try: + body = await request.json() + except Exception: + body = {} + cid = str(body.get("id") or "") + fut = pending.pop(cid, None) + if fut is not None and not fut.done(): + fut.set_result({ + "id": cid, "ok": bool(body.get("ok")), + "result": body.get("result"), "error": body.get("error"), + "screenshot": body.get("screenshot"), + }) + return JSONResponse({"ok": True}) + + @router.post("/api/editor/bridge/await") + async def bridge_await(request: Request) -> JSONResponse: + try: + body = await request.json() + except Exception: + body = {} + cid = str(body.get("id") or "") + timeout = float(body.get("timeout") or _BRIDGE_DEFAULT_TIMEOUT) + fut = pending.get(cid) + if fut is None: + fut = asyncio.get_event_loop().create_future() + pending[cid] = fut + try: + result = await asyncio.wait_for(fut, timeout=timeout) + return JSONResponse(result) + except asyncio.TimeoutError: + pending.pop(cid, None) + return JSONResponse({"ok": False, "error": "timeout", "id": cid}) + + _AGENT_SYSTEM_PROMPT = ( + "You are the photo editor agent. You drive the open image editor and " + "may also run AI image operations. Available editor ops: " + + ", ".join(OP_CATALOG) + + ", plus ai_edit_image and generate_image. " + "Respond with ONLY a JSON object: {\"reply\": , " + "\"steps\": [{\"op\": , \"params\": {...}}]}. " + + _ROUTING_GUIDANCE + " " + + _CANONICAL_GUIDANCE + " " + _STYLE_GUIDANCE + " " + "Prefer AI content ops for generative requests: \"remove the person on the left\" " + "-> op \"ai_edit_image\" op \"inpaint\" with a prompt; \"add a full moon in the sky\" " + "-> op \"generate_image\" with a prompt. " + "Use deterministic editor ops (adjustments.apply, filters.apply, selection.*, " + "layers.placeImage, text.add, ...) only for non-generative, precise edits. " + "Keep steps minimal and ordered. " + + _SCHEMA_NOTE + ) + + def _scoped_editor_tools() -> List[str]: + """The exact tool list offered to the model for a photo_editor turn.""" + fallback = [ + "plan_edit", "run_editor_command", "get_editor_screenshot", + "ai_edit_image", "generate_image", "ask_user", + ] + try: + from src.tool_policy import ToolPolicy + from src.tool_schemas import FUNCTION_TOOL_SCHEMAS + from src.turn_contract import resolve_turn_contract + + contract = resolve_turn_contract( + capabilities={"photo_editor"}, + schemas=FUNCTION_TOOL_SCHEMAS, + policy=ToolPolicy(), + required_capabilities={"photo_editor"}, + ) + return sorted(contract.offered) or fallback + except Exception: + return fallback + + @router.post("/api/editor/agent") + async def editor_agent(request: Request) -> JSONResponse: + """Scoped photo_editor agent turn (Quick Edit). + + Offers ONLY the editor tool family plus ask_user, plans edits, and + drives the open editor through the owner-scoped bridge. AI edit and + generation actions land as new layers; the original is never + overwritten. + """ + owner = _user(request) + try: + body = await request.json() + except Exception: + body = {} + prompt = str(body.get("prompt") or "").strip() + session = str(body.get("editor_session") or body.get("session") or "default") + state = body.get("state") if isinstance(body.get("state"), dict) else {} + screenshot = body.get("screenshot") + # Optional user-picked image model (from the editor model selector). + model_choice = str(body.get("model") or body.get("image_model") or "").strip() + # Optional user-picked PLANNER model (the editor agent/planner model). + agent_model = str(body.get("agent_model") or "").strip() + tools_offered = _scoped_editor_tools() + scoped = _scoped(session, owner) + trace: List[Dict[str, Any]] = [ + {"type": "session", "session": scoped, "tools_offered": tools_offered}, + ] + if not prompt: + return JSONResponse({ + "ok": False, "reply": "No prompt provided.", + "tools_offered": tools_offered, "steps": [], "trace": trace, + "agent_model": agent_model or editor_agent_model.local_model(), + }) + text = json.dumps({"prompt": prompt, "state": state}, ensure_ascii=False) + content: Any = text + if isinstance(screenshot, str) and screenshot.startswith("data:image/"): + content = [ + {"type": "text", "text": text}, + {"type": "image_url", "image_url": {"url": screenshot}}, + ] + messages = [ + {"role": "system", "content": _AGENT_SYSTEM_PROMPT}, + {"role": "user", "content": content}, + ] + raw = None + planner = _EDITOR_PLANNER + editor_agent_model.set_override(agent_model) + editor_agent_model.set_last_model("", "") + if planner is not None: + try: + raw = planner(messages, owner) + if hasattr(raw, "__await__"): + raw = await raw + except Exception: + raw = None + _used = editor_agent_model.get_last_model() + model_used = (_used[0] if _used and _used[0] + else (agent_model or editor_agent_model.local_model())) + plan = _parse_plan(raw) or _fallback_plan(prompt) + _strengthened = style_guide.strengthen(prompt, list(plan.get("steps") or [])) + if _strengthened != plan.get("steps"): + plan = {**plan, "steps": _strengthened, + "planner": f"{plan.get('planner') or 'llm'}+guide"} + kind = _generative_kind(prompt) + provider_available = _image_provider_available(owner) + # Deterministic overrides for intents the model reliably botches: canvas + # format/crop/extend, text + layer styles, and PSD export. + _special = _plan_special(prompt, state, provider_available) + special_note = "" + if _special: + special_steps, special_reply, special_note = _special + steps = style_guide.enforce_guards(special_steps) or list(special_steps) + reply = special_reply + kind = None + plan = {**plan, "steps": steps, + "planner": f"{plan.get('planner') or 'llm'}+deterministic"} + else: + steps = _ensure_generative(prompt, list(plan.get("steps") or [])) + steps = style_guide.enforce_guards(steps) + if steps != (plan.get("steps") or []): + plan = {**plan, "steps": steps, + "planner": f"{plan.get('planner') or 'llm'}+generative"} + reply = str(plan.get("reply") or "") + # A bare whole-image tone plan for an unspecified prompt is a guess: + # ask the user instead of silently tweaking. + # A bare whole-image tone plan for an unspecified prompt is a guess: + # ask the user instead of silently tweaking. Only replace when the + # guard actually changed the plan (identity), so a model-supplied + # ask_user never wipes out a concrete generative edit. + _base_plan = {**plan, "steps": steps} + _maybe = _unsure_plan(prompt, _base_plan) + if _maybe is not _base_plan and isinstance(_maybe, dict) and _maybe.get("ask_user"): + plan = _maybe + steps = [] + reply = str(_maybe.get("reply") or "") + trace.append({ + "type": "plan", "tool": "plan_edit", + "planner": plan.get("planner"), "steps": steps, + "image_provider": provider_available, "agent_model": model_used, + }) + # A vague prompt with no concrete defect asks the user instead of + # guessing a token edit (2-3 concrete options). + ask_user_payload = plan.get("ask_user") + if isinstance(ask_user_payload, dict) and not steps and not _special: + trace.append({"type": "ask_user", + "question": ask_user_payload.get("question")}) + return JSONResponse({ + "ok": True, + "ask_user": ask_user_payload, + "ask_user_pending": True, + "reply": reply or str(ask_user_payload.get("question") or ""), + "tools_offered": tools_offered, + "steps": [], + "trace": trace, + "results": [], + "agent_model": model_used, + }) + # No image provider: stop before any edit runs and tell the user plainly + # (zero canvas changes; nothing to roll back). + if (kind or _plan_uses_ai_content(steps)) and not provider_available: + trace.append({"type": "blocked", "reason": "image_provider_unavailable"}) + return JSONResponse({ + "ok": False, + "blocked": True, + "needs_provider": True, + "reply": _PROVIDER_REQUIRED_MESSAGE, + "message": _PROVIDER_REQUIRED_MESSAGE, + "image_provider": False, + "tools_offered": tools_offered, + "steps": steps, + "trace": trace, + "results": [], + "agent_model": model_used, + }) + results: List[Dict[str, Any]] = [] + for step in steps: + op = step.get("op") + params = step.get("params") or {} + if op in ("ai_edit_image", "generate_image"): + try: + from src.agent_tools.editor_tools import AiEditImageTool, GenerateImageLayerTool + except Exception: + results.append({"op": op, "ok": False, "error": "editor tools unavailable"}) + break + tool_payload = dict(params) + tool_payload.setdefault("session", session) + # Route to the user-picked model when one is selected. + if model_choice: + tool_payload.setdefault("model", model_choice) + # Hand the AI step the canvas image the client already captured + # for this request. Without it the tool falls back to a separate + # bridge screenshot, which is not reliably answered right after a + # selection step (the masked path was losing the canvas image). + if isinstance(screenshot, str) and screenshot.startswith("data:image/"): + tool_payload.setdefault("image", screenshot) + tool_cls = AiEditImageTool if op == "ai_edit_image" else GenerateImageLayerTool + try: + _desc, res = await tool_cls().execute( + json.dumps(tool_payload), + {"owner": owner, "session_id": scoped}, + ) + except Exception as exc: + res = {"error": str(exc), "exit_code": 1} + res = res if isinstance(res, dict) else {} + ok = res.get("exit_code", 0) == 0 + step_result = { + k: res.get(k) for k in ("image_id", "image_url", "placed", "output", "model") + if k in res + } + results.append({ + "op": op, "ok": ok, "result": step_result, + # Surface the real provider/tool failure so the reply can + # say what went wrong instead of a bare ok:true/ok:false. + "error": None if ok else (res.get("error") or res.get("output") or "it did not complete"), + }) + trace.append({"type": "step", "op": op, "ok": ok, "result": step_result}) + if not ok: + break + continue + cid = uuid.uuid4().hex + pending[cid] = asyncio.get_event_loop().create_future() + _broadcast(scoped, {"id": cid, "op": op, "params": params}) + try: + res = await asyncio.wait_for(pending[cid], timeout=_BRIDGE_DEFAULT_TIMEOUT) + except asyncio.TimeoutError: + pending.pop(cid, None) + results.append({"op": op, "ok": False, "error": "timeout"}) + trace.append({"type": "step", "op": op, "ok": False, "error": "timeout"}) + break + results.append(res) + trace.append({"type": "step", "op": op, "ok": res.get("ok"), "result": res.get("result")}) + if res.get("ok") is False: + break + # Post-run verification: read the ACTUAL canvas back and base the reply + # on executed results + final document.info, never the plan. + final_state: Dict[str, Any] = {} + if steps and not any(r.get("ok") is False for r in results): + try: + _info_id = uuid.uuid4().hex + pending[_info_id] = asyncio.get_event_loop().create_future() + _broadcast(scoped, {"id": _info_id, "op": "document.info", "params": {}}) + _info = await asyncio.wait_for(pending[_info_id], timeout=60) + if isinstance(_info, dict) and isinstance(_info.get("result"), dict): + final_state = _info["result"] + except Exception: + final_state = {} + planned_reply = reply + reply = _verified_reply(prompt, steps, results, final_state, planned_reply) + if special_note and special_note.strip() not in reply: + reply = (reply + " " + special_note.strip()).strip() + return JSONResponse({ + "ok": not any(r.get("ok") is False for r in results), + "reply": reply or "Done.", + "planned_reply": planned_reply, + "image_provider": provider_available, + "final_state": final_state, + "tools_offered": tools_offered, + "tools": ["plan_edit"], + "steps": steps, + "trace": trace, + "results": results, + "agent_model": model_used, + }) + + @router.get("/api/editor/style-guide") + async def style_guide_route() -> JSONResponse: + """The shared intent -> ops guide (single source of truth). + + The browser fallback planner fetches this so the client and server + routing rules cannot drift. + """ + return JSONResponse(style_guide.as_json()) + + @router.get("/api/editor/image-models") + async def editor_image_models(request: Request) -> JSONResponse: + """List the image models available from configured, owner-visible providers. + + Each entry is ``{id, label, kind, provider}`` where ``id`` is the model id + to pass as ``model`` to ai_edit_image / generate_image, and ``kind`` is + ``edit`` (editing/inpainting models) or ``generate`` (text-to-image). + """ + owner = _user(request) + models: List[Dict[str, Any]] = [] + seen: set = set() + try: + from core.database import SessionLocal, ModelEndpoint + from src.auth_helpers import owner_filter + from src.endpoint_resolver import _endpoint_enabled_models + + db = SessionLocal() + try: + query = owner_filter( + db.query(ModelEndpoint).filter(ModelEndpoint.is_enabled == True), # noqa: E712 + ModelEndpoint, + owner, + ) + for ep in query.all(): + if getattr(ep, "model_type", None) != "image": + continue + provider = getattr(ep, "name", None) or getattr(ep, "base_url", "") or "image" + for name in _endpoint_enabled_models(ep): + key = (provider, name) + if key in seen: + continue + seen.add(key) + models.append({ + "id": name, + "label": name, + "kind": "edit" if _EDIT_MODEL_RE.search(name) else "generate", + "provider": provider, + "endpoint_id": getattr(ep, "id", None), + "base_url": getattr(ep, "base_url", "") or "", + }) + finally: + db.close() + except Exception: + logger.warning("editor image-models lookup failed", exc_info=True) + return JSONResponse({"models": models}) + + @router.get("/api/editor/agent-models") + async def editor_agent_models(request: Request) -> JSONResponse: + """List the models the editor agent/planner can run on. + + Each entry is ``{id, label, source, vision, base_url}`` where ``id`` is + passed as ``agent_model`` to /api/editor/plan or /api/editor/agent, and + ``source`` is ``local`` (the local Qwen3.5-9B vision server) or + ``hosted`` (the configured default/utility chat model that the planner + falls back to). + """ + owner = _user(request) + try: + models = await editor_agent_model.list_editor_agent_models(owner) + except Exception: + logger.warning("editor agent-models lookup failed", exc_info=True) + models = [] + return JSONResponse({ + "models": models, + "probe": editor_agent_model.probe_status(), + }) + + @router.post("/api/editor/self-check") + async def editor_self_check(request: Request) -> JSONResponse: + """Same-origin VL self-check: compare BEFORE/AFTER against the intent. + + The local VL server (seneca :8097) sends no CORS headers, so the browser + cannot call it directly and the self-check silently "passed". This route + proxies the two images + intent to the agent VLM and returns + ``{done, issues, retry_hint, text}`` (``done=True`` with + ``reason="unavailable"`` on any failure so the client never loops). + """ + owner = _user(request) + try: + body = await request.json() + except Exception: + body = {} + result = await editor_agent_model.self_check( + body.get("before"), + body.get("after"), + str(body.get("prompt") or body.get("intent") or ""), + model=str(body.get("model") or "").strip() or None, + owner=owner, + ) + return JSONResponse(result) + + return router diff --git a/routes/email/email_helpers.py b/routes/email/email_helpers.py index d697b2fe7..1808af13b 100644 --- a/routes/email/email_helpers.py +++ b/routes/email/email_helpers.py @@ -17,8 +17,6 @@ import base64 import time import imaplib import smtplib -import socket -import ssl import email as email_mod import email.header import email.utils @@ -38,7 +36,6 @@ from typing import Optional, List from src.auth_helpers import _auth_disabled, get_current_user from src.secret_storage import decrypt as _decrypt -from src.url_safety import OutboundAddressBlocked, connect_outbound_tcp logger = logging.getLogger(__name__) @@ -163,94 +160,6 @@ def _smtp_security_mode(cfg: dict) -> str: return "ssl" -# Raised when a mail host resolves into a denied range (see _mail_private_blocked). -MailServerAddressBlocked = OutboundAddressBlocked - - -def _mail_private_blocked(owner: str | None) -> bool: - """Whether private/loopback/shared mail destinations are denied for *owner*. - - Link-local (cloud metadata), multicast, reserved and unspecified addresses - are always denied (src/url_safety.py). Private ranges are where a mail - tester or saved account becomes an internal-network probe, so they are - only allowed for an admin or in single-user mode (AUTH_ENABLED=false), the - same principals trusted with host-level tools. Operators of multi-user - deployments with a LAN mail server opt in with EMAIL_ALLOW_PRIVATE_IPS=true; - EMAIL_BLOCK_PRIVATE_IPS=true denies private ranges for everyone. - """ - if os.getenv("EMAIL_BLOCK_PRIVATE_IPS", "").strip().lower() == "true": - return True - if os.getenv("EMAIL_ALLOW_PRIVATE_IPS", "").strip().lower() == "true": - return False - from src.tool_security import owner_is_admin_or_single_user - - return not owner_is_admin_or_single_user((owner or "").strip() or None) - - -def _mail_socket_timeout(timeout): - return socket._GLOBAL_DEFAULT_TIMEOUT if timeout is None else timeout - - -class _PolicyIMAP4(imaplib.IMAP4): - """IMAP4 whose socket comes from connect_outbound_tcp. - - The mail host is resolved once and connected to only at the addresses the - address policy approved, so DNS rebinding between check and connect is - impossible. STARTTLS still verifies against ``self.host``. - """ - - def __init__(self, host, port, *, block_private: bool, timeout=None): - self._block_private = block_private - super().__init__(host, port, timeout=timeout) - - def _create_socket(self, timeout): - if timeout is not None and not timeout: - raise ValueError("Non-blocking socket (timeout=0) is not supported") - return connect_outbound_tcp( - self.host, self.port, timeout=_mail_socket_timeout(timeout), block_private=self._block_private, - ) - - -class _PolicyIMAP4_SSL(imaplib.IMAP4_SSL): - """IMAP4_SSL over a policy-checked socket; TLS SNI/verification use the hostname.""" - - def __init__(self, host, port, *, block_private: bool, timeout=None, ssl_context=None): - self._block_private = block_private - super().__init__(host, port, ssl_context=ssl_context, timeout=timeout) - - def _create_socket(self, timeout): - sock = _PolicyIMAP4._create_socket(self, timeout) - return self.ssl_context.wrap_socket(sock, server_hostname=self.host) - - -class _PolicySMTP(smtplib.SMTP): - """SMTP whose socket comes from connect_outbound_tcp (see _PolicyIMAP4).""" - - def __init__(self, host="", port=0, *, block_private: bool, **kwargs): - self._block_private = block_private - super().__init__(host, port, **kwargs) - - def _get_socket(self, host, port, timeout): - if timeout is not None and not timeout: - raise ValueError("Non-blocking socket (timeout=0) is not supported") - return connect_outbound_tcp( - host, port, timeout=timeout, block_private=self._block_private, - source_address=self.source_address, - ) - - -class _PolicySMTP_SSL(smtplib.SMTP_SSL): - """SMTP_SSL over a policy-checked socket; TLS SNI/verification use the hostname.""" - - def __init__(self, host="", port=0, *, block_private: bool, **kwargs): - self._block_private = block_private - super().__init__(host, port, **kwargs) - - def _get_socket(self, host, port, timeout): - sock = _PolicySMTP._get_socket(self, host, port, timeout) - return self.context.wrap_socket(sock, server_hostname=self._host) - - def _send_smtp_message(cfg: dict, from_addr: str, recipients: list[str], message: str | bytes, timeout: int = 30) -> None: """Send through SMTP using the configured transport security mode.""" host = cfg["smtp_host"] @@ -269,15 +178,14 @@ def _send_smtp_message(cfg: dict, from_addr: str, recipients: list[str], message smtp.login(user, password) security = _smtp_security_mode(cfg) - block_private = _mail_private_blocked(cfg.get("owner")) if security == "ssl": - with _PolicySMTP_SSL(host, port, timeout=timeout, block_private=block_private) as smtp: + with smtplib.SMTP_SSL(host, port, timeout=timeout) as smtp: _auth_smtp(smtp) smtp.sendmail(from_addr, recipients, message) return - with _PolicySMTP(host, port, timeout=timeout, block_private=block_private) as smtp: + with smtplib.SMTP(host, port, timeout=timeout) as smtp: if security == "starttls": smtp.starttls() _auth_smtp(smtp) @@ -316,39 +224,6 @@ def _friendly_email_auth_error(protocol: str, host: str, error: object) -> str: return raw[:200] -# Bound at import: callers (and tests) may swap out imaplib.IMAP4 itself. -_IMAP_ABORT = imaplib.IMAP4.abort - - -def _mail_connection_test_error(protocol: str, host: str, error: BaseException) -> str: - """User-facing result for a failed IMAP/SMTP connection test. - - Transport failures are reported by category, never with the peer's own - bytes: a non-mail service answering on the chosen host/port would - otherwise have its banner echoed back (an internal-service fingerprinting - oracle). Errors from a server that does speak the protocol (login/auth - rejections) keep their text via _friendly_email_auth_error, since that is - what the user needs to fix their settings. - """ - if isinstance(error, MailServerAddressBlocked): - return f"{protocol} server address is not allowed by this server's network policy" - if isinstance(error, (_IMAP_ABORT, smtplib.SMTPConnectError, smtplib.SMTPServerDisconnected)): - return f"{protocol} server did not respond like an {protocol} server" - if isinstance(error, ssl.SSLCertVerificationError): - return f"{protocol} TLS certificate verification failed" - if isinstance(error, ssl.SSLError): - return f"{protocol} TLS handshake failed; check the port and security setting" - if isinstance(error, (socket.timeout, TimeoutError)): - return f"{protocol} connection timed out" - if isinstance(error, ConnectionRefusedError): - return f"{protocol} connection refused" - if isinstance(error, socket.gaierror): - return f"{protocol} server name could not be resolved" - if isinstance(error, OSError) and not isinstance(error, smtplib.SMTPException): - return f"{protocol} connection failed" - return _friendly_email_auth_error(protocol, host, error) - - def _strip_think(text: str) -> str: """Email-flavored think strip — thin wrapper over the central helper. @@ -810,21 +685,18 @@ def _ensure_sender_signatures_table(conn): _lg.getLogger(__name__).warning(f"sender_signatures owner-migration skipped: {_mig_e}") -def attachment_extract_dir(folder: str, uid: str, *, owner: str, account_id: str | None) -> Path: - """Containment-safe extraction directory for one message's attachments. +def attachment_extract_dir(folder: str, uid: str) -> Path: + """Containment-safe extraction directory for an attachment. - IMAP UIDs are small per-mailbox counters, so `folder_uid` alone put every - user's and every account's "INBOX 42" in one directory, where extractions - overwrote each other by filename (a path handed to one user's agent could - then hold another user's attachment). See src.mail_attachment_paths: the - directory is keyed by (owner, account, folder, uid) and contained in - ATTACHMENTS_DIR whatever `folder`/`uid` hold.""" - from src.mail_attachment_paths import attachment_scope_dir - - try: - return attachment_scope_dir(ATTACHMENTS_DIR, folder, uid, owner=owner, account_id=account_id) - except ValueError: + `folder` and `uid` are user-controlled (query/path params). Flatten them to + a single safe path segment so a value like folder='../../tmp' can't escape + ATTACHMENTS_DIR, then assert containment as belt-and-suspenders.""" + key = re.sub(r"[^A-Za-z0-9._-]", "_", f"{folder}_{uid}") or "_" + target = (ATTACHMENTS_DIR / key).resolve() + base = ATTACHMENTS_DIR.resolve() + if target != base and base not in target.parents: raise HTTPException(400, "Invalid attachment location") + return target def _init_scheduled_db(): @@ -1205,8 +1077,6 @@ def _get_email_config(account_id: str | None = None, owner: str = "") -> dict: cfg = { "account_id": row.id, "account_name": row.name, - # Principal for the outbound mail address policy. - "owner": row.owner or owner or "", "smtp_host": row.smtp_host or "", "smtp_port": int(row.smtp_port or 465), "smtp_security": _smtp_security_mode({"smtp_security": getattr(row, "smtp_security", ""), "smtp_port": row.smtp_port}), @@ -1240,7 +1110,6 @@ def _get_email_config(account_id: str | None = None, owner: str = "") -> dict: cfg = { "account_id": resolved_id, "account_name": "legacy", - "owner": owner or "", "smtp_host": settings.get("smtp_host", os.environ.get("SMTP_HOST", "")), "smtp_port": int(settings.get("smtp_port", os.environ.get("SMTP_PORT", "465")) or 465), "smtp_security": _smtp_security_mode({ @@ -1304,16 +1173,11 @@ def _open_imap_connection( starttls: bool, timeout: int = _IMAP_TIMEOUT_SECONDS, ssl_context=None, - owner: str | None = None, ): - """Open an IMAP connection using the configured security mode. - - `owner` is the principal the destination is judged for - (see _mail_private_blocked).""" + """Open an IMAP connection using the configured security mode.""" port = int(port or 993) - block_private = _mail_private_blocked(owner) if starttls: - conn = _PolicyIMAP4(host, port, timeout=timeout, block_private=block_private) + conn = imaplib.IMAP4(host, port, timeout=timeout) try: if ssl_context: conn.starttls(ssl_context=ssl_context) @@ -1329,9 +1193,9 @@ def _open_imap_connection( raise elif port == 993: kwargs = {"ssl_context": ssl_context} if ssl_context else {} - conn = _PolicyIMAP4_SSL(host, port, timeout=timeout, block_private=block_private, **kwargs) + conn = imaplib.IMAP4_SSL(host, port, timeout=timeout, **kwargs) else: - conn = _PolicyIMAP4(host, port, timeout=timeout, block_private=block_private) + conn = imaplib.IMAP4(host, port, timeout=timeout) try: conn.sock.settimeout(timeout) except Exception: @@ -1371,7 +1235,6 @@ def _imap_connect(account_id: str | None = None, owner: str = "", cfg["imap_port"], starttls=bool(cfg.get("imap_starttls")), timeout=timeout, - owner=cfg.get("owner"), ) try: if cfg.get("oauth_provider") == "google": diff --git a/routes/email/email_routes.py b/routes/email/email_routes.py index 172790a6c..12d2dd82a 100644 --- a/routes/email/email_routes.py +++ b/routes/email/email_routes.py @@ -36,10 +36,10 @@ from pathlib import Path from email.mime.text import MIMEText from email.mime.multipart import MIMEMultipart -from starlette.concurrency import run_in_threadpool from fastapi import APIRouter, Query, UploadFile, File, BackgroundTasks, HTTPException, Depends, Request from fastapi.responses import FileResponse, StreamingResponse from src.constants import DATA_DIR +from src import shutdown as _shutdown from src.path_confinement import confine from src.llm_core import llm_call_async @@ -61,7 +61,6 @@ from .email_helpers import ( _fetch_sender_thread_context, _pre_retrieve_context, _EMAIL_REPLY_SYS_PROMPT_BASE, _POOL_HOOKS, _friendly_email_auth_error, _email_summary_failure_log_detail, - _mail_connection_test_error, _mail_private_blocked, _PolicySMTP, _PolicySMTP_SSL, _generate_email_summary, EMAIL_SUMMARY_ERROR_CODE, EMAIL_SUMMARY_ERROR_MESSAGE, SendEmailRequest, ExtractStyleRequest, ATTACHMENTS_DIR, COMPOSE_UPLOADS_DIR, SCHEDULED_DB, @@ -1543,44 +1542,6 @@ def _envelope_recipients(*fields: str) -> list: return out -_MD_LINK_URL_RE = re.compile(r"\((https?://)[^)\s]*") - - -def _md_links_to_html(s: str) -> str: - """Linear ``re.sub(r"\\[([^\\]]+)\\]\\((https?://[^)\\s]+)\\)", '\\1', s)``. - - Every `[` before the next `]` shares that `]`, so only the first can match; - and when a URL runs into whitespace/end without `)`, every later `[` whose - `]` also lies before that stop fails the same way. Skipping those spans - keeps a `[[[` or `[a](http://x[a](http://x` flood O(n) instead of the - regex's O(n^2). - """ - out: list[str] = [] - emitted = scan = 0 - while True: - open_at = s.find("[", scan) - if open_at < 0: - break - close_at = s.find("]", open_at + 1) - if close_at < 0: - break - scan = close_at + 1 - if close_at == open_at + 1: - continue - url = _MD_LINK_URL_RE.match(s, close_at + 1) - if url is None: - continue - end = url.end() - if end < len(s) and s[end] == ")" and end > url.end(1): - out.append(s[emitted:open_at]) - out.append(f'{s[open_at + 1:close_at]}') - emitted = scan = end + 1 - elif end > url.end(1): - scan = max(scan, s.rfind("]", scan, end) + 1) - out.append(s[emitted:]) - return "".join(out) - - def _md_to_email_html(text: str) -> str: """Render the compose markdown body to a SAFE HTML fragment for the email's text/html part. Everything is HTML-escaped FIRST (so a pasted ', + '' + ].join('\n'); + } else if (uid === 'long_msg') { + body_html = `

${'A'.repeat(5000)}

`; + } else { + body_html = `

Standard message body for ${uid}

`; + } + + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ + uid: uid, + subject: `Subject ${uid}`, + from_name: 'Sender', + from_address: 'sender@example.com', + to: 'user@example.com', + date: '2026-10-20', + date_display: 'Oct 20, 2026', + body: `Plain text for ${uid}`, + body_html: body_html, + })); + return; + } + + let filePath = path.join(ROOT, url.pathname); + if (fs.existsSync(filePath) && fs.statSync(filePath).isFile()) { + const ext = path.extname(filePath); + const mime = ext === '.js' ? 'application/javascript' : ext === '.html' ? 'text/html' : ext === '.css' ? 'text/css' : 'text/plain'; + res.writeHead(200, { 'Content-Type': mime }); + res.end(fs.readFileSync(filePath)); + return; + } + res.writeHead(404); + res.end('Not found'); + }); + + await new Promise(r => server.listen(0, '127.0.0.1', r)); + const port = server.address().port; + const origin = `http://127.0.0.1:${port}`; + + const launchOpts = { headless: true }; + const chromePath = findChromiumPath(); + if (chromePath) launchOpts.executablePath = chromePath; + + const browser = await chromium.launch(launchOpts); + const context = await browser.newContext(); + const page = await context.newPage(); + + const results = {}; + + try { + await page.goto(`${origin}/static/index.html`); + await page.evaluate(() => { + if (!document.getElementById('email-lib-bulk-actions')) { + const d = document.createElement('div'); d.id = 'email-lib-bulk-actions'; document.body.appendChild(d); + } + if (!document.getElementById('email-lib-selected-count')) { + const d = document.createElement('div'); d.id = 'email-lib-selected-count'; document.body.appendChild(d); + } + }); + + // TEST 1: Full Print Image Sanitization and Replacement + const printImageResult = await page.evaluate(async (base) => { + window.API_BASE = base; + const mod = await import(`${base}/static/js/emailLibrary/index.js`); + const stateMod = await import(`${base}/static/js/emailLibrary/state.js`); + + stateMod.state._selectedUids = new Set(['img_test']); + stateMod.state._libEmails = [{ uid: 'img_test', subject: 'Image Test', folder: 'INBOX' }]; + + let iframePrintInvoked = false; + let printDocHtml = ''; + let afterPrintTriggered = false; + let hasPlaceholderElement = false; + + const observer = new MutationObserver(() => { + const frame = document.getElementById('email-browser-print-frame'); + if (frame && frame.contentWindow && !frame._patched) { + frame._patched = true; + frame.contentWindow.print = function() { + iframePrintInvoked = true; + printDocHtml = frame.contentDocument ? frame.contentDocument.documentElement.outerHTML : ''; + const body = frame.contentDocument ? frame.contentDocument.body : null; + hasPlaceholderElement = !!body?.querySelector('.email-inline-image-placeholder'); + frame.contentWindow.dispatchEvent(new Event('afterprint')); + afterPrintTriggered = true; + }; + } + }); + observer.observe(document.body, { childList: true }); + + await mod._exportSelectedEmailPdf(); + observer.disconnect(); + + const frameAfter = document.getElementById('email-browser-print-frame'); + + return { + iframePrintInvoked, + afterPrintTriggered, + frameRemovedAfterPrint: frameAfter === null, + hasLogoNotice: printDocHtml.includes('[Image: Company Brand Logo]'), + hasTrackerNotice: printDocHtml.includes('[Image: Tracking Pixel]'), + hasNoAltNotice: printDocHtml.includes('[Image omitted]'), + hasPlaceholderElement, + hasRemoteImageBlockedText: printDocHtml.includes('Remote image blocked'), + hasDownloadButton: printDocHtml.includes('email-inline-image-download-btn'), + hasInteractiveActionBtn: printDocHtml.includes('btn-action'), + hasXSS: printDocHtml.includes('