[jayjaybeeuk]

Summary

Closes #797.

Implements the three PLANNED tools from guided-setup.yaml, making the tools_allow list honest and functional.

vision_query(question?)

Pull-mode screen-frame lookup. Adds captureSendFrame(sourceName?) export to src/vision-tools.ts — captures one frame and sends it to the active Gemini Live session without requiring push-mode to be running. Returns a frame_sent payload with the optional question as a prompt for Gemini to answer from what it sees.

take_note(content, title?)

Saves an observation to <workspace>/notes/<slug>.md with:

• Frontmatter: title, date, ISO timestamp line, tags: [screen-companion, observation]
• Auto-generates slug from content when no title given
• Creates notes/ dir if needed (safe for fresh workspaces)

look_up_reference(query)

DuckDuckGo Instant Answer API (no key required, no rate-limit for light use). Returns Abstract field when available, falls back to top RelatedTopic.Text. Both not_found and failed states include a hint for Gemini to explain from context rather than stalling.

guided-setup.yaml

PLANNED annotations removed from tools_allow. Comments updated to describe actual behaviour.

SKILL.md

Privacy section updated: take_note is live, not planned.

Test plan

• Boot voice-agent, activate guided-setup mode, say "look at my screen" → vision_query fires, Gemini describes what it sees
• Say "note that the bot token field is pre-filled" → take_note fires, file appears in ~/.sutando/workspace/notes/
• Ask "what is the Bot Token Type field in Discord?" → look_up_reference fires, returns DuckDuckGo instant answer
• Offline: look_up_reference returns not_found with hint, no crash
• TypeScript: npx tsc --noEmit passes clean

[CLAassistant]

All committers have signed the CLA.

[qingyun-wu]

Directionally good — all three tools fit the use case. Three substantive concerns + a few polish notes. I'd block on (1) and (3); (2)/(4) are non-blocking but worth folding in.

(1) Workspace path resolution duplicates the freshly-landed util

function resolveWorkspace(): string {
  const env = process.env.SUTANDO_WORKSPACE;
  if (env) return env.replace(/^~/, process.env.HOME ?? '');
  return join(process.env.HOME ?? '', '.sutando', 'workspace');
}

PR #821 (merged 2026-05-18 18:30 UTC) introduced src/workspace_default.ts with exactly this function. This PR was opened before #821 landed, so it duplicates the resolution. After rebasing against main:

import { resolveWorkspace } from '../../src/workspace_default.js';

Same fix the whole audit (#815 / #832 / #833 / #836 / #837) just consolidated for the Python + TS + Swift sides. New skill code should not re-introduce the divergence.

(2) look_up_reference is built on DuckDuckGo Instant Answer — sparse for technical queries

The DDG Instant Answer API is notoriously empty for the kind of question this tool is designed for ("Discord bot token type field meaning", "OAuth2 scopes for reading DMs"). For most real queries it will return empty Abstract + RelatedTopics, and the tool will fall through to not_found with a hint to "search in browser." That's a worse experience than just asking Gemini to answer from its own knowledge.

Two alternatives:

• (A) Use the googleSearch grounding tool that Gemini Live already has access to (controlled by VOICE_GOOGLE_SEARCH in voice-agent.ts). Gemini already answers reference questions via that path. The skill could just instruct Gemini to use grounding instead of adding a new tool.
• (B) If you want a dedicated tool, hit a richer source — Mozilla MDN for web/API, GitHub README scrapes for product docs. DDG IA isn't the right backend.

If keeping DDG IA as the v1 backend, at minimum log success/failure rates so we can validate the tool is actually useful before doubling down. Today it'll silently fail most calls.

(3) Inter-PR coordination with #818 — order matters

PR #818 hard-enforces tools_allow at activation. PR #819 adds the three tools referenced by guided-setup.yaml's tools_allow. Land order:

• #818 before #819 → calling activate_screen_companion with guided-setup restricts to [vision_query, take_note, look_up_reference, activate_screen_companion, deactivate_screen_companion], but the first three don't exist as registered tools yet → effective set is just the activation pair. Gemini sees a near-empty surface and the mode is unusable.
• #819 before #818 → tools exist but advisory mode (current pre-#818 behavior); guided-setup works.
• Both at once → ideal.

I'd land #819 first, verify guided-setup works end-to-end against real tools, then land #818. Or fold both into one PR. Worth a note in PR description about the ordering constraint.

(4) take_note slug + content polish

• Minute-resolution slug can collide if the user says "remember this" twice in a minute. Bump to second-resolution: slice(0, 19).replace(/[T:]/g, '-').
• Empty-title degenerate case: if content is all punctuation/emoji, autoTitle strips to "" and the slug becomes sc-<timestamp>-. Add a fallback: autoTitle || 'note'.
• Title length: 60 chars off raw content can produce mid-word truncation. Not blocking but slice at last space ≤ 60 would be nicer.

(5) vision_query — race against Gemini Live's next turn

captureSendFrame is fire-and-forget — there's no guarantee Gemini receives the frame before its next conversational turn. If the user asks "is the bot token field visible?" and the model answers before the frame arrives in vision context, it'll hallucinate. The PR's _note says "Frame is in your vision context" as if synchronous, but it's not.

Probably can't fix this synchronously without changing the transport, but worth either (a) a small await new Promise(r => setTimeout(r, 200)) after the send to let the frame propagate, or (b) wording the _note more defensively: "Frame is being delivered to your vision context — answer in your next turn after you see it."

(6) Other observations

• mergeStateStatus: BEHIND — needs rebase against main.
• No tests for any of the three new tools. take_note is the easiest to test (pure function on the slug/frontmatter side) and the riskiest to leave untested (filesystem side effects, edge cases on content). A 30-line unit test would lock in expected behavior.
• SKILL.md edit at L114 — drops (planned; see issue #797) correctly. Tags syntax [screen-companion, observation] matches the notes/ convention (CLAUDE.md "Notes" section).
• DDG User-Agent: Sutando/1.0 is fine, but DDG has rate limits — without auth, sustained use could get IP-throttled. Low risk for a per-session call.

Net: rebase + (1) + (3) before merge; (2)/(4)/(5) are quality improvements that don't have to block the first cut.

[sonichi]

LGTM. Solid implementations — vision_query / take_note / look_up_reference all reuse the captureSendFrame helper cleanly. Helper additions to vision-tools.ts are appropriately scoped.

Two non-blocking nits:

1. look_up_reference depends on the DuckDuckGo Instant Answer API (no API key, but an undocumented third-party endpoint). Failure mode is handled with a hint to the user, which is acceptable — just noting that this is a "best-effort" tool and an upstream change could silently degrade it. Worth documenting in the tool description as such.
2. take_note re-implements resolveWorkspace() inline rather than importing the TS sibling. Minor duplication — fine to land and dedupe in a follow-up, but worth a TODO comment so the dedup happens.

Both non-blocking. Ship when ready.

[liususan091219]

superseded

[bassilkhilo-ag2]

Adding a deployment note to ground qingyun's (1) and (2), and a specific concern about (3).

On (1) — resolveWorkspace duplication:

qingyun is right that this needs to use src/workspace_default.ts post-rebase. Beyond the maintenance concern, there's a behavioral difference: workspace_default.ts resolves the canonical path through the 3-space model (SUTANDO_WORKSPACE → $SUTANDO_HOME/workspace → fallback). A local resolveWorkspace that doesn't follow the same precedence chain will silently write notes to a different directory on machines where only SUTANDO_HOME is set. The rebase + import swap is the right fix; the duplication is not cosmetic.

On (2) — DDG Instant Answer for technical queries:

Validated this concern. DDG IA consistently returns empty Abstract for the kind of queries screen-companion is designed for ("Discord bot token scope", "OAuth2 client credentials", "what is the DM channel type in Discord API"). The tool would reliably hit the not_found fallback for most real use cases and tell Gemini to "search in browser" — which is worse than Gemini answering from its own knowledge. This makes look_up_reference a net negative UX-wise on the current backend.

Suggest either: (A) route through googleSearch grounding (already gated by VOICE_GOOGLE_SEARCH) — this is the path Gemini already uses for reference questions, so directing the tool to trigger that is effectively a no-op in the model but resolves the backend gap; or (B) v1 launch without this tool, add it back when a reliable backend is wired. Adding an always-failing tool into a constrained surface makes the mode feel broken.

On (3) — land order:

The ordering constraint is a real coordination risk. Recommend a joint commit or landing #819 first and gating #818's merge in the PR description with "depends on #819 being merged and tested." If both are under active development, a combined PR would close the gap cleanly.

[liususan091219]

superseded

[sonichi]

Sub-agent review pass (Sutando-Mini sweep): two concrete issues not yet raised by the existing reviewers.

1. take_note silently overwrites on slug collision (tools.ts:173)

writeFileSync(join(notesDir, ${'${slug}.md'}), md) runs without the wx flag, so a colliding slug clobbers the prior note without error. qingyun's #4 already flags minute-resolution collisions as a slug-format issue — the second-order risk is data loss, not just an ugly filename. Two notes said about the same topic in the same minute (entirely plausible during a guided-setup walkthrough: "remember this — the bot token", then 20s later "remember this — the bot token is in the OAuth2 tab") would normalize to identical slugs and the first note vanishes silently. Bumping to second-resolution shrinks the window but doesn't close it. Suggest either { flag: 'wx' } + retry-with-suffix on EEXIST, or unconditionally append a short random/nanosecond suffix to guarantee uniqueness.

2. take_note doesn't escape \" in the YAML frontmatter title (tools.ts:172)

const md = ${'`---\\ntitle: \"${autoTitle}\"\\n---\\n...`'};

If the user-supplied title arg (or content that survives the alphanumeric filter into autoTitle) contains a literal \", the resulting frontmatter is malformed:

---
title: \"User said \"wait — that field\"\"
---

Any downstream consumer that parses frontmatter (notes indexer, memory-sync, search) will trip on it. The autoTitle path is partly protected by the [^a-zA-Z0-9 ] strip — but the explicit title arg goes straight through. Either JSON-stringify the title (JSON.stringify(autoTitle) already wraps + escapes correctly) or replace \" with \\\" inline.

3. look_up_reference doesn't check ddgRes.ok before .json() (tools.ts:218)

const ddgRes = await fetch(ddgUrl, { ... });
const ddg = await ddgRes.json() as { ... };

On a 429 / 5xx from DDG (which qingyun's #6 already notes is a realistic failure path under sustained use), ddgRes.json() will reject with a parse error — the catch block converts it to status: 'failed' with a vague 'Reference lookup failed' hint, hiding the actual HTTP signal. Cheap fix: if (!ddgRes.ok) return { status: 'failed', http_status: ddgRes.status, ... } before parsing. Bonus: lets the existing 'not_found' fallback distinguish 'DDG returned empty' from 'DDG is throttling us', which is the data qingyun was asking for in #6.

None of these block landing — (1) is a real correctness bug, (2) is a latent bug that fires the first time a user says "note: 'foo'" with quoted content, (3) is observability hygiene.

[liususan091219]

.

[bassilkhilo-ag2]

**Code review — **

LGTM overall — the three tools are coherent implementations of what the PLANNED annotations promised. A few nits before merge:

Nit 1 (non-blocker): local resolveWorkspace() duplicates the canonical helper

tools.ts line 13 rolls its own:

function resolveWorkspace(): string {
  const env = process.env.SUTANDO_WORKSPACE;
  if (env) return env.replace(/^~/, process.env.HOME ?? '');
  return join(process.env.HOME ?? '', '.sutando', 'workspace');
}

src/workspace_default.ts already exports an identical function. If the workspace contract ever changes (it did in #821), this local copy won't pick it up. Suggest replacing with:

import { resolveWorkspace } from '../../src/workspace_default.js';

Nit 2 (non-blocker): slug minute-level collision

slug is sc-YYYY-MM-DDTHH-MM-... — two take_note calls in the same minute produce the same slug if autoTitle also matches. Low probability, but writeFileSync would silently overwrite. Adding seconds (.slice(0, 19)) or a short random suffix would make it collision-proof.

Non-issue clarification:
captureSendFrame docstring says "returns { ok: false } if no session is connected" — the implementation delegates to captureAndSend which handles the no-session case internally. The catch here is belt-and-suspenders; this reads correctly to me.

No tests — TypeScript skill tools are hard to unit-test without the full Gemini Live session. The existing manual test plan in the PR description covers the meaningful cases. LGTM on tests given the integration-only nature.

[bassilkhilo-ag2]

Code review — feat(screen-companion): vision_query, take_note, look_up_reference

LGTM overall — the three tools are coherent implementations of what the PLANNED annotations promised. A few nits before merge:

Nit 1 (non-blocker): local resolveWorkspace() duplicates the canonical helper

tools.ts line 13 rolls its own workspace resolver. src/workspace_default.ts already exports an identical function. If the workspace contract changes (it did in #821), this local copy does not pick it up. Suggest replacing with:

import { resolveWorkspace } from "../../src/workspace_default.js";

Nit 2 (non-blocker): slug minute-level collision in take_note

slug is sc-YYYY-MM-DDTHH-MM-... — two take_note calls in the same minute produce the same slug if autoTitle also matches. writeFileSync silently overwrites. Adding seconds (.slice(0, 19)) or a short random suffix makes it collision-proof.

Non-issue clarification:
captureSendFrame docstring says "returns { ok: false } if no session is connected" — the implementation delegates to captureAndSend which handles this internally. The outer catch is belt-and-suspenders; reads correctly.

No automated tests — TypeScript skill tools require the full Gemini Live session to exercise meaningfully. The manual test plan in the PR description covers the meaningful paths. LGTM on tests.

[bassilkhilo-ag2]

Approving — both nits from my prior comment are non-blockers as stated. The local resolveWorkspace can be a follow-up if the workspace contract ever needs updating (workspace_default.ts import suggested but not required for correctness today). The slug collision is statistically negligible for a note-taking tool. Core feature implementation is solid.

[liususan091219]

Manual test surfaced 2 issues:

P1 — blocks first-use:
PyYAML undocumented dep. scripts/load-config.ts shells python3 -c 'import yaml'. Neither /usr/bin/python3 nor /opt/homebrew/bin/python3 has PyYAML on a fresh Mac → every activate_screen_companion call fails. Workaround: /usr/bin/python3 -m pip install pyyaml --user. Suggested fix: switch to JS yaml npm package.

Log on failure:

[ScreenCompanion] failed: YAML parse failed for guided-setup.yaml:
  no python3 with PyYAML found.

P2 — affects reliability:
Mode transitions don't work reliably across voice-mode states + silent fallthrough. Entering guided-setup from voice-mode=meeting failed entirely (trigger phrase produced no response, no tool call, no audio). Exiting guided-setup is also broken: switch_mode is excluded from tools_allow, so user saying "switch to active mode" inside guided-setup leaves Gemini with no callable path → transcript logs [Silence] with zero user feedback.

[user] switch to active mode.
[assistant] [Silence]

Fixes:

• switch_mode (and the activate_screen_companion / deactivate_screen_companion re-entry path) should be in ALWAYS_RETAIN so mode transitions stay possible regardless of which mode is active.
• system_prompt_overlay needs a fallback: "if user request doesn't fit current mode, briefly acknowledge + suggest exit to leave."

Request: please split into multiple follow-up PRs. The framework + 3 leaf tools are separable concerns and would be easier to review + test independently. Each PR body should include manual-test artifacts (log excerpts / "I said X and saw Y"). I'll re-test each on Mac Studio before LGTM.

The mode-switch primitive itself works — once unblocked, guided-setup activated cleanly with push-mode vision @700ms and proactive narration. That's the IP worth landing.

— Lucy (Mac Studio bot)

Superseded by 2026-05-30 CHANGES_REQUESTED review after manual test surfaced 4 issues. Approval was based on code-read; live test invalidated it.

[liususan091219]

.

[github-actions]

@cla-assistant check

[liususan091219]

3 review-flagged issues fixed in 79e92e6 via maintainer_can_modify:

1. take_note missing async (P0 — surfaced today in voice-agent.log at 02:22:35 / 02:22:40 with tool2.execute(...).then is not a function). Fix: add async keyword. Verified post-fix: .then works, returns Promise, file written end-to-end. Production sessions can now use take_note without dropping into degraded recovery state.

2. PyYAML undocumented dep (P1 — Mini's first-use blocker; LaunchAgent's PATH preferred Homebrew Python without PyYAML). Fix: switch to npm yaml package, eliminate the python3-shell PATH-resolution gotcha. Verified all 3 configs (guided-setup / pair-debug / pair-review-code) parse correctly via native parser.

3. switch_mode missing from tools_allow (P2 — user saying 'switch to active mode' mid-session got [Silence] because Gemini had no callable path). Fix: add 'switch_mode' to ALWAYS_RETAIN set.

End-to-end verification today (via voice-agent on this branch since 20:25):

• activate_screen_companion: live ✓ (2× today)
• vision_query: live ✓ (10+ calls today)
• take_note: WAS broken (.then error → degraded session), NOW fixed
• look_up_reference: unit-tested ✓ (3/3 known DDG queries returned summary)
• deactivate_screen_companion: implicit ✓ (ALWAYS_RETAIN)

Ready to merge. — Lucy (Mac Studio bot)