Skip to content

Linux Inspector: window-id capture and frame-geometry alignment#700

Open
mahbd wants to merge 2 commits into
devfrom
linux-inspector-installer
Open

Linux Inspector: window-id capture and frame-geometry alignment#700
mahbd wants to merge 2 commits into
devfrom
linux-inspector-installer

Conversation

@mahbd

@mahbd mahbd commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add optional window_id parameter to capture_screenshot and get_ui_tree, allowing a specific window to be targeted directly instead of relying on app-name lookup.
  • Add _get_frame_geometry_for_window to find the AT-SPI frame matching a window and crop composite screenshot captures to its bounds, fixing a ~40px misalignment between bounding boxes and screenshot content (window decoration/title bar offset).
  • Existing callers (e.g. AI Recorder) that don't pass window_id are unaffected — they fall back to the previous app-name-based behavior.

Test plan

  • Verified composite capture for HampsonRussell window crops to 1896x1043, matching the AT-SPI frame geometry.
  • Verified geoview window capture produces correct 700x997 image.
  • Run through Linux Inspector end-to-end in browser against the corresponding server PR.

@riz-hossain

Copy link
Copy Markdown
Contributor

🔎 ZeuZ PR Review

Open the full report in ZeuZ: Review findings and apply suggestions

Overview Value
Agents ✅ 4 completed
Suggestions 💡 4

Agent breakdown

→ General Review

Status: ✅ Completed
Suggestions: 1 suggestion

The PR is directionally solid, but there is a notable correctness gap around window_id handling that can make the DOM and screenshot refer to different windows.

→ Security Review

Status: ✅ Completed
Suggestions: 0 suggestions

No high-signal security issues found in the PR diff. The changes add Linux window enumeration and screenshot alignment logic, plus pinned dependencies, without introducing a clear injection, authz, or secret-handling flaw.

→ Performance Review

Status: ✅ Completed
Suggestions: 1 suggestion

I found one performance risk: the new Linux apps/window discovery path now shells out to xdotool repeatedly per PID/window, which can get expensive as the number of visible windows grows.

→ Testing Review

Status: ✅ Completed
Suggestions: 2 suggestions

The Linux inspector change adds new window-aware behavior and a new /linux/apps payload shape, but I found no corresponding tests covering those code paths or the regression scenarios they’re meant to fix.

Open ZeuZ to inspect full findings, continue an agent conversation, or apply safe patch suggestions.

@riz-hossain

Copy link
Copy Markdown
Contributor

🔎 ZeuZ PR Review

Open the full report in ZeuZ: Review findings and apply suggestions

Overview Value
Agents ✅ 4 completed
Suggestions 💡 4

Agent breakdown

→ General Review

Status: ✅ Completed
Suggestions: 2 suggestions

I found one correctness issue in the new window-selection fallback, plus one maintainability issue in the API model definition.

→ Security Review

Status: ✅ Completed
Suggestions: 0 suggestions

No high-signal security issues found in this PR diff. The changes add window selection and geometry alignment without introducing obvious injection, authz, or secret-handling regressions.

→ Performance Review

Status: ✅ Completed
Suggestions: 1 suggestion

The PR adds useful multi-window support, but it also introduces several extra xdotool round-trips on the hot paths for app lookup and /apps, which can get expensive as the number of windows grows.

→ Testing Review

Status: ✅ Completed
Suggestions: 1 suggestion

The PR adds important Linux window-selection and geometry-alignment logic, but there are no tests covering the new code paths in server/linux.py or Framework/Built_In_Automation/Desktop/Linux/BuiltInFunctions.py yet.

Open ZeuZ to inspect full findings, continue an agent conversation, or apply safe patch suggestions.

mahbd added 2 commits June 13, 2026 15:35
…geometry alignment

Crops composite captures to the AT-SPI frame bounds so screenshots align
with accessibility-tree coordinates, and allows callers to target a
specific window by id instead of just app name.
@mahbd mahbd force-pushed the linux-inspector-installer branch from 1d36d8e to a3c40a7 Compare June 13, 2026 09:36
@riz-hossain

Copy link
Copy Markdown
Contributor

🔎 ZeuZ PR Review

Open the full report in ZeuZ: Review findings and apply suggestions

Overview Value
Agents ✅ 4 completed
Suggestions 💡 3

Agent breakdown

→ General Review

Status: ✅ Completed
Suggestions: 1 suggestion

I found one high-impact issue in the new screenshot alignment logic: the crop rectangle is computed from AT-SPI desktop coordinates but compared against the captured window's local geometry, so the alignment path will often never apply. I also noted a smaller maintainability issue around a mutable model default.

→ Security Review

Status: ✅ Completed
Suggestions: 0 suggestions

No security-relevant issues found in the PR diff; the changes are focused on window selection and screenshot alignment without introducing obvious injection, auth, or data-exposure risks.

→ Performance Review

Status: ✅ Completed
Suggestions: 1 suggestion

One performance concern: /linux/apps now does a per-process window enumeration that can balloon into many xdotool subprocesses, and the screenshot lookup path also repeats window queries per candidate. No correctness issues stood out beyond that.

→ Testing Review

Status: ✅ Completed
Suggestions: 1 suggestion

The PR adds new window-selection and frame-alignment behavior, but I don’t see any tests covering the new Linux inspector paths or the regression scenario they fix.

Open ZeuZ to inspect full findings, continue an agent conversation, or apply safe patch suggestions.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants