Skip to content

feat: add image inspector plugin - #2286

Open
henryZhouLikeStudy wants to merge 4 commits into
TencentCloud:developfrom
henryZhouLikeStudy:codex/image-inspector-plugin-pr
Open

henryZhouLikeStudy wants to merge 4 commits into
TencentCloud:developfrom
henryZhouLikeStudy:codex/image-inspector-plugin-pr

Conversation

@henryZhouLikeStudy

@henryZhouLikeStudy henryZhouLikeStudy commented Oct 21, 2026 •

Copy link
Copy Markdown

Summary

Adds an independent image-inspector tool plugin for visual review of images generated in an Octop workspace.

The tool is manually invoked with the localPath returned by generate_image. It validates that the image is inside an Octop workspace, reads a supported raster image, and makes one dedicated multimodal model request with a Base64 image_url block. The returned review covers clipping, overlap, alignment, spacing, contrast, hierarchy, and other visible defects.

Why the model request is inside the tool

Returning an image block from the tool result is insufficient in the current runtime: Octop serializes that media block to a text representation before the next model call. Browser screenshots are also represented as path-preview blocks in persisted history.

To ensure the image actually reaches the model, inspect_image invokes the Agent's resolved auto model directly with the image as a data URL. The outer Agent then receives a text-only visual assessment and can continue its normal workflow.

Parent-stream isolation

The nested visual-review request must not inherit the parent LangGraph callbacks. Without isolation, its generated tokens are projected as normal assistant output and can be sent to an IM channel or persisted as an extra assistant message.

The dedicated request now:

  • disables inherited run callbacks;
  • marks its metadata with lc_source=image_inspector;
  • returns the review only as the inspect_image tool result.

A regression test installs a parent chat-model callback before invoking the tool and verifies that the callback receives zero starts from the nested review request.

Workspace compatibility and safety

  • Discovers data roots from OCTOP_HOME, OCTOP_DATA_DIR, and the default ~/.octop.
  • Accepts only regular files under Octop agents/ or workspaces/.
  • Remaps agent-facing absolute paths and falls back to .harness-materialize/ for Docker sandbox files.
  • Resolves unique workspace-relative paths and rejects ambiguous matches.
  • Supports PNG, JPEG, and WebP, detected by Pillow, and rejects files larger than 20 MiB.
  • Does not fetch HTTP(S) URLs, log image bytes or credentials, or modify image files and Octop package files.

Validation

  • Rebased onto current develop; PR head: d9f671cc754bed81e33de55e6154ff9fe9ae8bb3.
  • uv run --frozen pytest tests/unit/test_image_inspector_plugin.py tests/unit/test_plugins.py tests/unit/test_bundled_plugins_layout.py -q
    • 32 passed
  • uv run --frozen ruff check src tests
    • passed
  • uv run --frozen ruff format --check src tests
    • 1294 files already formatted
  • uv run --frozen mypy src/octop
    • Success: no issues found in 627 source files
  • Full uv run --frozen pytest -n auto -m "not live" on Windows:
    • 5113 passed, 225 skipped; 34 environment-only failures remain in unchanged database-migration GBK and Bash/WSL-dependent Docker/vendor tests. None involve image-inspector or its changed files.
  • Live validation on a self-hosted Octop instance confirmed custom data-root resolution, Docker .harness-materialize fallback, and real multimodal inspection of a synthetic image with intentional clipping.

GitHub Actions requires maintainer approval for this fork PR; no CI jobs have run for the rebased head yet.

Checklist

  • Added focused unit tests
  • Verified real Octop behavior
  • Added regression coverage for parent-stream leakage and sandbox path resolution
  • No credentials, server addresses, user images, or user chat content included
  • No core Octop package changes required
  • Plugin can be installed independently from this directory or a package archive

Closes #2283

@henryZhouLikeStudy

henryZhouLikeStudy commented Oct 23, 2026 •

Copy link
Copy Markdown
Author

CI run https://github.com/TencentCloud/Octop/actions/runs/39939099843 is currently action_required because fork pull-request workflows are awaiting maintainer approval; no jobs have run yet. Local validation for this revision is complete: 28 plugin tests passed, full Ruff passed, and mypy passed across 609 source files.

@xlwreally

Copy link
Copy Markdown

Thanks for the image inspector tool — we integrated it into a self-hosted deployment and hit a path-resolution bug that is worth fixing upstream.

Symptom

inspect_image returns Error: image file not found: <absolute path> for an image that exists in the agent workspace and is readable through read_file.

Environment

Octop 1.0.2, self-hosted; agent backend is the docker sandbox; the data directory is a mounted volume instead of ~/.octop.

Root cause

  1. _workspace_root() hard-codes ~/.octop/workspaces. In any deployment where the data directory lives elsewhere (a mounted volume, a custom OCTOP_HOME) the containment check can never match, so every path is rejected.
  2. _load_image() only tries Path.cwd() / relative for relative inputs and uses absolute inputs verbatim. Files produced inside an agent sandbox are not directly visible to the server process — they only exist under <workspace>/.harness-materialize/ after being materialized — so <data>/agents/<agent-id>/analysis/page.png never resolves.

Suggested fix

  • derive candidate data roots from OCTOP_HOME, OCTOP_DATA_DIR and ~/.octop, and use those roots for the containment check;
  • resolve the input in order: absolute path → <root>/agents/<id>/... remap → <root>/agents/<id>/.harness-materialize/... → relative to cwd / data roots / each agent workspace;
  • when nothing resolves, return an actionable error that tells the model to materialize the file first (send_file_to_user) or to pass the .harness-materialize path.

Tests

Two unit tests added to tests/unit/test_image_inspector_plugin.py (sandbox absolute path, materialize fallback); both pass with the patch. Behavior for the existing OCTOP_HOME/workspaces layout is unchanged.

Happy to open a PR with the implementation if you would like to take it.

@henryZhouLikeStudy

Copy link
Copy Markdown
Author

@xlwreally Thanks for the detailed reproduction. The path-resolution issue is confirmed and fixed in 613a8b7a636cbb409d90fff51e4c7dcd7e6ec2d6.

The resolver now:

  • discovers data roots from OCTOP_HOME, OCTOP_DATA_DIR, and the default ~/.octop;
  • accepts files only under Octop agents/ or workspaces/;
  • remaps agent-facing absolute paths to host agents/<id>/ and workspaces/<id>/ layouts;
  • falls back to <workspace>/.harness-materialize/ for Docker sandbox files;
  • resolves unique workspace-relative paths and rejects ambiguous matches;
  • returns an actionable error when a sandbox file still needs to be materialized.

Added regression coverage for custom OCTOP_DATA_DIR, Docker materialize fallback, and unique relative agent-workspace paths.

Validation:

  • tests/unit/test_image_inspector_plugin.py tests/unit/test_plugins.py tests/unit/test_bundled_plugins_layout.py: 31 passed
  • Ruff and format checks: passed
  • mypy: passed

GitHub Actions is still action_required because the fork workflow awaits maintainer approval; no CI jobs have run yet.

@henryZhouLikeStudy

Copy link
Copy Markdown
Author

@xlwreally The reported path-resolution scenario has now been validated in a self-hosted production Octop environment.

Deployment:

  • Validated PR head: 613a8b7a636cbb409d90fff51e4c7dcd7e6ec2d6
  • Updated image-inspector in place with rollback backup retained
  • Octop restarted after confirming no active tool/cron/IM turn
  • Service recovered in 4 seconds, database healthy, all 5 Agents running
  • No warnings after restart

Online path test:

  • Created a synthetic, non-user image with an intentionally clipped right-side card and text
  • Stored it only at the host-side agents/<id>/.harness-materialize/analysis/... path
  • Passed a different virtual workspaces/<id>/analysis/... path to inspect_image
  • The updated resolver found the materialized file and invoked the real multimodal model
  • The model correctly reported that the right-side card, round corner, border, and text were clipped at the image boundary

Note: one ordinary CLI Agent round incorrectly selected read_file instead of the registered inspect_image tool, so that round was not counted as a pass. The final validation invoked the actually registered inspect_image tool directly with the live Agent model factory. This confirms the plugin path handling and multimodal request path; model tool-selection behavior remains a separate runtime concern.

This record contains no user image, chat content, account ID, server address, API key, or token. GitHub Actions remains action_required pending maintainer approval.

@henryZhouLikeStudy
henryZhouLikeStudy force-pushed the codex/image-inspector-plugin-pr branch 2 times, most recently from d9f671c to a5bbb3c Compare November 2, 2026 09:39
# Conflicts:
#	CHANGELOG.md

# Conflicts:
#	CHANGELOG.md
# Conflicts:
#	CHANGELOG.md
@henryZhouLikeStudy
henryZhouLikeStudy force-pushed the codex/image-inspector-plugin-pr branch from a5bbb3c to 00f1d5b Compare November 3, 2026 08:43
@henryZhouLikeStudy

Copy link
Copy Markdown
Author

Rebased onto current develop and resolved the changelog conflict while preserving both branches' entries. New head: 00f1d5b. Image-inspector/plugin/layout tests: 34 passed; Ruff and diff checks pass; CodeCC check passes.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants