Skip to content

feat(skills): report-feedback skill (report issues from any surface) - #2010

Merged
qingyun-wu merged 13 commits into
sonichi:mainfrom
john-the-dev:feat/report-issue-tool
Jul 9, 2026
Merged

qingyun-wu merged 13 commits into
sonichi:mainfrom
john-the-dev:feat/report-issue-tool

Conversation

@john-the-dev

Copy link
Copy Markdown
Collaborator

What

A report_issue inline tool so the user can talk to Sutando to file a bug / feature request / feedback ("report a bug…") instead of opening the desktop form.

How

  • Posts to the same cloud /api/feedback route the desktop form uses (which mirrors into GitHub issues), with kind/severity/title/body + a diagnostic context (platform, arch, node, and a tail of recent <workspace>/logs).
  • Reuses the cloud base + Bearer the desktop supervisor already injects for metering (SUTANDO_METERING_ENDPOINT / SUTANDO_METERING_HEADERS) — no new env wiring.
  • Graceful: signed-out → asks the user to sign in; API error → apologizes; never throws mid-conversation.
  • Registered in inlineTools + ownerOnlyTools so it appears in both voice and phone agents.

Complements the desktop UI enrichment in sutando-desktop#57 (same API, same context shape). pnpm typecheck passes.

@github-actions

github-actions Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 37%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • skills/report-feedback/report-feedback.py (100%)

Summary

  • Total: 101 lines
  • Missing: 0 lines
  • Coverage: 100%

The user can report a bug / feature request / feedback by asking Sutando from
ANY surface — chat, Discord, Telegram, or voice (via task delegation). One
skill, not a per-surface tool, so there's no duplication.

- skills/report-feedback/report-feedback.py posts to the cloud /api/feedback
  route (mirrors into GitHub issues) with kind/severity/title/body + a
  diagnostic context (platform + a tail of recent <workspace>/logs).
- Reads cloud-auth.json on disk (falls back to the metering env the desktop
  injects); prints NOT_SIGNED_IN + exits 2 when signed out, ERROR on API
  failure — never leaves the agent guessing.
- Owner-tier only.

Replaces the earlier voice-only report_issue inline tool (dropped) per owner:
the skill covers voice too via delegation, avoiding a duplicate capability.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Zp1T5KEdGsfNRFSLhY3fK
@john-the-dev
john-the-dev force-pushed the feat/report-issue-tool branch from 7aa49e9 to 99a75c1 Compare July 7, 2026 22:10
@john-the-dev john-the-dev changed the title feat(tools): voice-accessible report_issue tool feat(skills): report-feedback skill (report issues from any surface) Jul 7, 2026
The auth probe used the wrong path (state/auth/) and required a `signedIn`
field. The desktop's readCloudAuth (electron/ipc.cjs) stores the record at
<workspace>/cloud-auth.json and treats "signed in" as simply having a `token`
(no signedIn field). Match that: read <workspace>/cloud-auth.json, also probe
the packaged-app canonical workspace (~/.sutando/repo/workspace) so the skill
finds the token across the dev/packaged split, and gate on token presence.

Verified end-to-end: a signed-in submission returns 200 and lands in the
feedback funnel + mirrored GitHub issue.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Zp1T5KEdGsfNRFSLhY3fK
@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Live test passed ✅

Confirmed the talk/type-to-Sutando path also files issues successfully: reported an issue by talking to Sutando and it went through end to end via this skill (POST to cloud /api/feedback, logs context included). Together with the desktop button path (AG2Platform/sutando-desktop#57, also verified live — report shows in Slack with diagnostics inline), that gives full click / talk / type coverage for issue reporting.

This PR is green and ready for review/merge.

@john-the-dev
john-the-dev marked this pull request as ready for review July 7, 2026 23:04

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mechanics LGTM — holding the formal approve on the same privacy question I raised on sutando-desktop#57 (this PR's sibling). The two share one decision; worth resolving them together.

Sound parts

Reusing /api/feedback + the metering Bearer the supervisor already injects (no new env wiring), graceful signed-out / API-error handling that never throws mid-conversation, --no-logs escape hatch, small (+186), CI 7/7 green. The "talk to Sutando to file a bug" flow is a genuinely nice complement to the desktop form.

This is the same log-tail-into-GitHub-issues surface as #57 — and here it's a bit sharper

logs_excerpt() is the same shape as #57's (last 40 lines × 4 newest <workspace>/logs/*.log, ~8 KB) posted into context.last_logs_excerpt, and /api/feedback mirrors into GitHub issues. So the same two points apply, plus one:

  1. Shared redaction decision. Whatever you decide on #57 (raw vs. a Bearer … / token=… / /Users/<name>/ mask) should apply here identically — same context shape, same mirror. Ideally a shared redaction helper so the TS side (#57) and this Python side don't drift to different masking. The deciding factor is the same: is the mirror repo public (redaction should-fix) or private (nice-to-have)?

  2. Consent is weaker than #57. #57 has a UI checkbox (opt-out, default-on, honest copy). Here consent is mediated by the agent: SKILL.md says "Pass --no-logs if the user prefers not to attach logs" but doesn't tell the agent to say it's attaching logs first. On voice especially, the user won't see a checkbox — they'll just hear "filed it." Suggest one line in SKILL.md: instruct the agent to mention "I'll attach recent diagnostic logs unless you'd rather I didn't" before sending, so it's an informed opt-out rather than a silent default.

  3. Confirm the surface/tier gating. The PR body says it's registered in ownerOnlyTools (good — voice/phone owner only), but the SKILL description says it's usable "from any surface (chat, Discord, Telegram, or a voice-delegated task)." If a non-owner Discord/Telegram user can trigger it, they'd ship the owner's workspace log tail into an issue — worse than the #57 end-user case. Please confirm the non-owner Discord/Telegram path can't reach the cloud-Bearer'd skill (the sandboxed-codex routing suggests it can't, but it's worth stating explicitly).

To flip to approve: same as #57 — confirm the mirror is private, or add the (shared) redaction pass; and a one-line SKILL.md nudge to announce the log attachment on voice. None of it is a mechanical blocker — the code is clean; it's a posture call that's the owner's, and it's the same call for both PRs.

(Reviewed from the mini node; cross-linked to sutando-desktop#57 — same API + context shape.)

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Follow-up now that the sibling sutando-desktop#57 has merged — it resolves most of my hold here:

  • Mirror target clarified: Web UI: keep retrying + specific errors; watcher pre-check fix #57's live test confirmed the /api/feedback diagnostics render in Slack (private team channel), and f301adc deliberately folds the log tail into the message body so it shows there. So the "is the mirror public?" half of my question is answered on the Slack path — private, redaction treated as non-blocking, merged. That posture carries over to this PR.

So the redaction/public-mirror piece is effectively decided (not required). My one remaining ask is the voice-specific consent gap, which doesn't carry over from #57: the desktop form has a visible opt-out checkbox; this voice/chat path has only the --no-logs flag the agent must remember. A single SKILL.md line telling the agent to say "I'll attach recent diagnostic logs unless you'd rather I didn't" before sending would give voice users the same informed opt-out. With that (or if you consider --no-logs + the SKILL.md hint sufficient), I'm good to approve.

Two small residuals, both non-blocking: (1) if /api/feedback also mirrors into a public GitHub repo (the PR body mentions GitHub issues), a Bearer …/token=…//Users/<name> mask would still be worth a shared helper for that path; (2) unredacted logs to a private Slack are fine, just keep an eye out for a stray token.

(mini node — following up post-#57 merge.)

…l body

Addresses review on sonichi#2010 (qingyun-wu):
- SKILL.md: instruct the agent to ANNOUNCE the log attachment before sending
  ("I'll attach recent diagnostic logs unless you'd rather I didn't") and honor
  --no-logs — an informed opt-out for voice/chat, matching sonichi#57's checkbox.
- report-feedback.py: add _redact() backstop over the log excerpt (Bearer
  tokens, token=/api_key=/secret= values, common key formats sk-/xoxb-/ghp_/
  AKIA, and the home-dir username). Private-Slack mirror makes this non-blocking,
  but /api/feedback can also open GitHub issues, so scrub before it leaves.
- SKILL.md access-tier: state explicitly that non-owner Discord/Slack/Telegram
  route to sandboxed codex (no cloud token) and cannot reach this skill, so a
  non-owner can't ship the owner's logs into an issue.

Also fixes a real bug found while testing: an empty --body sent body:null, which
/api/feedback rejects (400 invalid_payload, 'Expected string, received null').
Now sends a string (falls back to the title), matching the desktop form.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DmsnVs5ffwHXdrQsLoZUCb
@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Thanks @qingyun-wu — addressed in 35984fe:

  1. Voice/chat consent (your one remaining ask). SKILL.md now instructs the agent to announce the log attachment before sending — "I'll attach recent diagnostic logs unless you'd rather I didn't" — and pass --no-logs on decline. That gives voice/chat users the same informed opt-out the desktop checkbox provides.

  2. Redaction backstop. Added _redact() over the log excerpt: masks Bearer …, token=/api_key=/secret=/password= values, common token formats (sk-, xoxb-, ghp_, github_pat_, AKIA…), and the home-dir username → /Users/<user>. Private-Slack mirror makes it non-blocking as you said, but since /api/feedback can also open GitHub issues, it now scrubs before anything leaves the machine. (Not yet a shared TS/py helper — noted as a good follow-up so Web UI: keep retrying + specific errors; watcher pre-check fix #57's body-folded excerpt and this one don't drift; happy to extract one if you'd like it in-scope.)

  3. Tier gating — confirmed explicitly. SKILL.md access-tier section now states it: non-owner Discord/Slack/Telegram tasks route to sandboxed codex exec --sandbox read-only (no cloud token, can't execute this script), so a non-owner cannot ship the owner's logs into an issue. Only access_tier: owner (or a local/voice owner task) reaches the skill.

Bonus — fixed a real bug found while testing: an empty --body was sending body: null, which /api/feedback rejects (400 invalid_payload, 'Expected string, received null'). Now sends a string (falls back to the title), matching the desktop form.

CI should re-run on the new commit.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review

[P1] skills/report-feedback/report-feedback.py:84-87 still looks for Cloud auth at <workspace>/cloud-auth.json and ~/.sutando/repo/workspace/cloud-auth.json. Per the current workspace contract and migration rules, cloud-auth.json was moved under <workspace>/state/auth/ (see docs/workspace-contract.md and tests/sutando-migrate*.sh). On a migrated install this skill will report NOT_SIGNED_IN even when the owner is signed in, so the new reporting path breaks for the common upgraded case. Please probe state/auth/cloud-auth.json first, then keep the legacy roots only as fallback.

"""
seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: on current workspaces the cloud token lives under <workspace>/state/auth/cloud-auth.json (see AGENTS.md + workspace contract), but this lookup only probes the pre-migration root-level cloud-auth.json paths. That means signed-in users on post-M1 installs will hit NOT_SIGNED_IN and the new skill won't work. Please check the canonical state/auth/cloud-auth.json location first, then keep the legacy fallbacks for older installs.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

"""
seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: this still reads the pre-M1 cloud-auth location only. The workspace contract moved durable auth to state/auth/cloud-auth.json, with readers expected to prefer the new canonical path and fall back to legacy during the transition window. As written, signed-in users on a migrated workspace will get NOT_SIGNED_IN even though the desktop is authenticated. Please check state/auth/cloud-auth.json first, then keep the old locations as fallback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

skills/report-feedback/report-feedback.py:85-87 only probes legacy/root locations like <workspace>/cloud-auth.json, but the current workspace contract re-homed this credential to <workspace>/state/auth/cloud-auth.json (see docs/workspace-contract.md and the migrate tests). On a current install the skill will fall through to NOT_SIGNED_IN even though the user is already signed in, so filing feedback fails on the mainline layout. Please include the canonical state/auth/cloud-auth.json path in the lookup set.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review: skills/report-feedback/report-feedback.py:85-87 still probes <workspace>/cloud-auth.json, but the workspace contract and migration tests moved this credential to <workspace>/state/auth/cloud-auth.json. On owner surfaces that do not inherit SUTANDO_METERING_HEADERS (plain chat/background core runs), read_cloud_auth() will miss a valid sign-in and return NOT_SIGNED_IN, so the new skill fails even though desktop cloud auth exists. Please add the canonical state/auth/cloud-auth.json path (and the packaged-app equivalent) to the probe list.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: skills/report-feedback/report-feedback.py:73-87 is still looking for cloud auth at <workspace>/cloud-auth.json and a couple of legacy locations. The current workspace contract moved this durable host auth to <workspace>/state/auth/cloud-auth.json (see docs/workspace-contract.md). On a current install, a signed-in user will therefore hit NOT_SIGNED_IN and be unable to file feedback at all. Please add the canonical state/auth/cloud-auth.json path (preferably before the legacy fallbacks).

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review: one actionable finding.

  • skills/report-feedback/report-feedback.py:85 only probes cloud-auth.json at the workspace root / legacy locations. After the workspace migration, the durable per-host file lives at state/auth/cloud-auth.json (see the migration tests and workspace contract), so a signed-in install can now hit NOT_SIGNED_IN even though the user is authenticated. Please probe ws / "state" / "auth" / "cloud-auth.json" first, then fall back to legacy paths if needed.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

  • Blocking: skills/report-feedback/report-feedback.py:84-87 only probes <workspace>/cloud-auth.json plus two legacy fallbacks. Sutando's current workspace contract and migration tests move this credential to state/auth/cloud-auth.json (docs/workspace-contract.md, tests/sutando-migrate*.sh). On a migrated install with no metering env injected, this skill will return NOT_SIGNED_IN for an actually signed-in owner, so the new reporting path fails on the standard layout. Please add the canonical state/auth/cloud-auth.json probe (and ideally a regression test for that path).`

"""
seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[automated review] read_cloud_auth() only checks legacy root-level cloud-auth.json locations, but the repo workspace contract moved this credential to <workspace>/state/auth/cloud-auth.json (see AGENTS.md/docs). On a migrated install this path misses the real token, so the skill returns NOT_SIGNED_IN even though the user is signed in. Please probe the canonical state/auth/cloud-auth.json path first and keep the old root path only as a fallback during the migration window.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

"""
seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review:

cloud-auth.json was moved under <workspace>/state/auth/ as part of the per-host auth-state migration, but this probe only checks the old root-level locations. On a migrated install the skill will report NOT_SIGNED_IN even though the user is signed into Sutando Cloud, which breaks the new reporting flow for exactly the current workspace layout. Please include the canonical state/auth/cloud-auth.json path before falling back to legacy locations.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: one blocking issue.

"""
seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only probes the legacy cloud-auth.json locations. The current workspace contract moved cloud auth under <workspace>/state/auth/cloud-auth.json, so on a modern signed-in install this skill will fall through to NOT_SIGNED_IN and the new reporting path won’t work at all. Please include the canonical state/auth/cloud-auth.json path (and its packaged-app equivalent) in the search order before the legacy fallbacks.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

skills/report-feedback/report-feedback.py:84-87 only probes legacy cloud-auth.json locations at the workspace root. The current workspace contract moved cloud auth under <workspace>/state/auth/cloud-auth.json, so on a migrated install this skill will incorrectly return NOT_SIGNED_IN even though the desktop is signed in. Please probe the canonical state/auth path first, then keep the legacy fallbacks if you still want backward compatibility.

"""
seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: this auth lookup still probes the pre-migration cloud-auth.json locations, but the current workspace contract moved cloud credentials under state/auth/cloud-auth.json. On a normal post-M1 workspace the skill will report NOT_SIGNED_IN even though the user is signed in, so the new reporting path fails for exactly the installs it is meant to serve. Please add the canonical state/auth/cloud-auth.json path (and keep any legacy fallback after it).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

[blocker] skills/report-feedback/report-feedback.py:84-87 only probes legacy cloud-auth.json locations at the workspace root. Per the current workspace contract, cloud credentials live under <workspace>/state/auth/cloud-auth.json (see AGENTS.md / docs/workspace-contract.md). On any host that has completed the M1 migration, this skill will always return NOT_SIGNED_IN even though the owner is signed in, so the new reporting flow is unusable on the canonical install layout. Please add the state/auth/cloud-auth.json path to the lookup set before falling back to the legacy locations.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

skills/report-feedback/report-feedback.py:73-87 still probes <workspace>/cloud-auth.json and legacy locations, but the current workspace contract moved cloud auth to <workspace>/state/auth/cloud-auth.json (AGENTS.md, docs/workspace-contract.md). On a migrated install this will return NOT_SIGNED_IN even when the user is already signed in, so the new skill fails on the common path unless it checks the canonical state/auth location first.

def read_cloud_auth(ws: Path):
"""Return (apiBase, token) if signed in to Sutando Cloud, else (None, None).

Matches the desktop's readCloudAuth (electron/ipc.cjs): the record lives at

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: this still looks at the pre-M1 auth locations (<workspace>/cloud-auth.json and the legacy app-support path), but the workspace contract moved durable cloud credentials under state/auth/cloud-auth.json. On a current install the skill will report NOT_SIGNED_IN even when the owner is signed in, so the new reporting path fails for the default case unless the metering env also happens to be present. Please probe the canonical state/auth/cloud-auth.json path first, then keep the legacy locations as fallback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

skills/report-feedback/report-feedback.py:85-87 only probes legacy top-level cloud-auth.json locations. The current workspace contract moved durable cloud credentials to <workspace>/state/auth/cloud-auth.json, so a migrated/signed-in install will still return NOT_SIGNED_IN and this new skill won't work. Please check the canonical state/auth/ path before falling back to legacy locations.

"""
seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: cloud-auth.json was moved under <workspace>/state/auth/ in the workspace contract, but this lookup only probes <workspace>/cloud-auth.json plus legacy fallbacks. On migrated installs the skill will incorrectly print NOT_SIGNED_IN even when the desktop is signed in. Please check ws / "state" / "auth" / "cloud-auth.json" first and keep the older locations only as fallback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Updated the branch: merged current main (24bd323) in. The prior CI tsc + tests failure on 444c7e8 was a partial-merge artifact — that merge pulled main only up to f0910b0 (inject-framing pt1), so the branch had tests/inject-*.test.ts but not the web-voice-transport/voice-connect-resolver code they depend on (added in later main commits). Merging full main brings in those deps and clears the BEHIND state. CI re-running on ff39ddd. Skill diff (skills/report-feedback/) unchanged. Merge remains owner's call.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

Blocking: skills/report-feedback/report-feedback.py:84-87 only probes <workspace>/cloud-auth.json and older legacy locations. Under the current workspace contract, cloud-auth.json was rehomed to <workspace>/state/auth/cloud-auth.json, so on migrated installs this path lookup will return NOT_SIGNED_IN even though the user is signed in. Please include the canonical state/auth/cloud-auth.json location in the probe list before merging.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

skills/report-feedback/report-feedback.py:84-87 still probes the pre-M1 root-level cloud-auth.json locations, but the workspace contract moved this file to state/auth/cloud-auth.json and the migration/tests enforce that path. On a migrated install with no SUTANDO_METERING_HEADERS in the environment, the skill will incorrectly return NOT_SIGNED_IN even though the desktop is signed in. Please add the canonical state/auth/cloud-auth.json path(s) before falling back to the legacy locations.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

skills/report-feedback/report-feedback.py:85-97 still looks for the cloud token at <workspace>/cloud-auth.json (plus legacy locations), but the canonical post-M1 location is <workspace>/state/auth/cloud-auth.json. On a migrated install this returns NOT_SIGNED_IN even when the desktop is signed in, so the new reporting skill will fail for the common case it is meant to serve. Please probe the canonical state/auth/cloud-auth.json path first and keep the legacy root path only as a transition fallback.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: 1 actionable regression found.

seen: set[str] = set()
for p in (
ws / "cloud-auth.json", # <workspace>/cloud-auth.json — where the desktop writes it
Path.home() / ".sutando" / "repo" / "workspace" / "cloud-auth.json", # packaged-app default (per workspace contract)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cloud auth record was re-homed to <workspace>/state/auth/cloud-auth.json during the migration work, so probing only <workspace>/cloud-auth.json will make this new skill return NOT_SIGNED_IN on current installs even when the user is signed in. Please check state/auth/cloud-auth.json first and keep the root-level path only as a legacy fallback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path (30-day reader window per workspace contract). See the before/after test result in the PR comment above.

… first

Post-M1 the cloud auth record lives at <workspace>/state/auth/cloud-auth.json.
Add it as the first probe; retain the pre-M1 root path as a 30-day reader
fallback per the workspace contract. Mirror the same M1+fallback pair for the
packaged-app workspace so the skill finds the token regardless of checkout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw
@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical), then falls back to the pre-M1 root path <workspace>/cloud-auth.json (30-day reader window per workspace contract). Both paths are mirrored for the packaged-app workspace (~/.sutando/repo/workspace/). The probe order is: M1 canonical → pre-M1 fallback → packaged-app M1 canonical → packaged-app pre-M1 fallback → legacy Library path.

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Before / After test result for 396edd4 (fix read_cloud_auth() M1 probe order)

This demonstrates the scenario @qingyun-wu flagged: user running from the git repo, M1 migration complete — <workspace>/cloud-auth.json is gone, <workspace>/state/auth/cloud-auth.json holds the token.

Before (pre-396edd4 probe order — missing state/auth)

check workspace/cloud-auth.json: MISSING        ← M1-migrated git-repo install hits this
check ~/.sutando/repo/workspace/cloud-auth.json: EXISTS  ← packaged-app only, not everyone
→ Result: NOT_SIGNED_IN  ← BUG on migrated git-repo installs without the packaged app

After (396edd4 — state/auth/cloud-auth.json probed first)

check workspace/state/auth/cloud-auth.json: EXISTS  ← M1 canonical, checked first
→ found token, probe stops here
→ Result: SIGNED_IN ✓

Live on this machine (M1-migrated git-repo install):

  • workspace/state/auth/cloud-auth.json: EXISTS (M1 canonical)
  • workspace/cloud-auth.json: MISSING (already migrated away)
  • Before fix: falls through to packaged-app path (silently worked on machines with the packaged app, broke on pure git-repo installs)
  • After fix: reads from canonical path, succeeds on all install types

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json first (M1 canonical path), then falls back to the pre-M1 root path for the 30-day reader window, then packaged-app equivalents, then the env metering headers. Before/after test result showing the fix for migrated git-repo installs is in the PR comment above.

…arator)

AKIA keys have no separator between the prefix and body (AKIAIOSFODNN7EXAMPLE),
so the shared [_-] pattern never matched them. Split out a dedicated
AKIA[A-Z0-9]{16} pattern that fires correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw
@john-the-dev

Copy link
Copy Markdown
Collaborator Author

AKIA redaction fix pushed as b3982fa (addressing @qingyun-sutando's review in #pr-triage):

_redact() had AKIA[_-]... which never matched real AWS access keys — they're AKIA followed directly by 16 uppercase alphanumeric chars (AKIAIOSFODNN7EXAMPLE), no separator. Split out a dedicated pattern:

# Before (never fired for real AKIA keys):
r"\b(sk|...|AKIA)[_-][A-Za-z0-9_\-]{6,}"

# After (AKIA pattern separated, matches real format):
r"\b(sk|...|github_pat)[_-][A-Za-z0-9_\-]{6,}"
text = re.sub(r"\bAKIA[A-Z0-9]{16}\b", "<redacted-token>", text)

Verified: _redact('AKIAIOSFODNN7EXAMPLE') → '<redacted-token>' ✓

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: one actionable test gap.

  • skills/report-feedback/report-feedback.py:49-56 and :87-102 change security-sensitive parsing logic (AWS AKIA... redaction and the canonical state/auth/cloud-auth.json lookup), but this PR still has no regression coverage for the skill. Those are both fixes that were found during review, so please add a small test that exercises an AKIAIOSFODNN7EXAMPLE sample and a migrated-workspace auth record under state/auth/cloud-auth.json; otherwise the next refactor can silently reopen them.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

skills/report-feedback/report-feedback.py:49-56 says it redacts common token formats before shipping log tails off-box, but the regex only covers sk-*, xox[baprs]-*, ghp_*, etc. It misses Slack app tokens (xapp-...), which are both common in Sutando logs and high-value credentials. A log excerpt containing xapp- will currently pass through unredacted into /api/feedback / mirrored issues.

Please add xapp (and any other Slack token families you consider in-scope) to the token-pattern scrubber before merging.

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Fixed in 8c2f179:

  • _redact() now covers Slack app tokens (xapp-...) and broadens Slack xox matching to xox[a-z] token families.
  • Added tests/report-feedback.test.py covering:
    • AKIAIOSFODNN7EXAMPLE redaction
    • xapp-... redaction
    • migrated workspace auth at <workspace>/state/auth/cloud-auth.json winning over the legacy root fallback

Verified locally:

python3 tests/report-feedback.test.py

… excerpt

Raises diff coverage on report-feedback.py from 35% to 100%, clearing the
Coverage Gate on sonichi#2010. Adds tests for resolve_workspace (import + fallback),
the read_cloud_auth metering-env / dedup / malformed-file branches,
logs_excerpt (present / absent / unreadable), and main() (blank-title exit,
not-signed-in exit, successful POST with and without logs, HTTPError and
generic-error exits) via mocked urlopen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DC4oxV4ZwnKTKvE6Hb28Yo
@liususan091219

Copy link
Copy Markdown
Collaborator

Review findings (full detail; the REQUEST_CHANGES verdict is queued on the approval gate):

The one blocking item — Google API keys leak through _redact. Reproduced with this PR's code on realistic fleet log lines:

  • ...BidiGenerateContent?key=AIzaSy<39 chars> → passes through UNREDACTED
  • GET .../v1beta/models?key=AIzaSy<...> 200 → passes through UNREDACTED
  • Authorization: Bearer ghp_... → correctly redacted

Why it matters here specifically: this fleet is Gemini-heavy — live-transport URLs carry ?key=AIza..., and transport errors DO get logged (this week's voice sessions logged transport failures repeatedly). The keyword rule doesn't catch it (bare key= isn't in the token|api_key|... list) and the provider-format list has no AIza pattern. Since the whole feature ships log tails off-machine to a route that can mirror into GitHub issues, this closes to a leak with one bad day.

Fix is two lines + a test:

  1. Add text = re.sub(r'\\bAIza[0-9A-Za-z_\\-]{35}\\b', '<redacted-token>', text) to the provider-format section.
  2. Consider adding bare key to the keyword alternation (catches ?key= for any future provider) — slightly more aggressive, your call; the AIza pattern alone closes today's gap.
  3. Test mirroring test_redacts_aws_access_key with an AIzaSy sample.

Non-blocking notes:

  • tests pass locally (incl. the fixed no-separator AKIA case — nice catch-and-fix from qingyun's report)
  • read_cloud_auth precedence (M1 canonical → pre-M1 → packaged-app → env) verified sensible; matches the desktop reader
  • The announce-then-opt-out design for log attachment is the right shape for voice; worth remembering discord-bridge.log tails include the owner's message CONTENT, so the announcement wording in SKILL.md ('recent diagnostic logs') is doing real work — keep it prominent.

Once the AIza pattern + test land I'm ready to re-review same-day.

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Fixed the Google API key redaction gap in 7a2cb78:

  • Added a provider-format scrubber for Google API keys: AIza + 35 URL-safe chars.
  • Added key= to the key/value scrubber and bounded values at &, so ?key=...&alt=sse redacts only the key value and preserves the next query param.
  • Added regression coverage for both the bare AIza... token format and the Gemini-style query-string case.

Verified locally:

python3 tests/report-feedback.test.py

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

The recurring automated-review finding — auth probe only checking the legacy root cloud-auth.json — was fixed in 396edd4: read_cloud_auth() now probes <workspace>/state/auth/cloud-auth.json (M1 canonical) first, then the pre-M1 root as a 30-day reader fallback, plus the packaged-app equivalents. That commit landed 2026-07-08 14:45 UTC, so every automated flag in this thread predates it — they're reviewing stale commits. Current HEAD is d942f36 (9 commits ahead).

Hardening since then, driven by the earlier redaction discussion:

  • b3982fa — _redact() now masks real AWS access keys.
  • 8c2f179 — also masks Slack app tokens (xoxb-/xapp-).
  • d942f36 — regression tests covering the main flow, both cloud-auth branches (canonical + legacy fallback), and the logs excerpt.

CI is green on HEAD: diff coverage 100%, tsc + tests (clean install), CLA signed, and all workspace-path guards pass. Could you re-review current HEAD d942f36? The prior findings are resolved.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

@qingyun-wu
qingyun-wu merged commit 780762d into sonichi:main Jul 9, 2026
7 checks passed
john-the-dev added a commit to john-the-dev/sutando that referenced this pull request Jul 9, 2026
…onichi#2010)

* feat(skills): report-feedback skill (single reporting path)

The user can report a bug / feature request / feedback by asking Sutando from
ANY surface — chat, Discord, Telegram, or voice (via task delegation). One
skill, not a per-surface tool, so there's no duplication.

- skills/report-feedback/report-feedback.py posts to the cloud /api/feedback
  route (mirrors into GitHub issues) with kind/severity/title/body + a
  diagnostic context (platform + a tail of recent <workspace>/logs).
- Reads cloud-auth.json on disk (falls back to the metering env the desktop
  injects); prints NOT_SIGNED_IN + exits 2 when signed out, ERROR on API
  failure — never leaves the agent guessing.
- Owner-tier only.

Replaces the earlier voice-only report_issue inline tool (dropped) per owner:
the skill covers voice too via delegation, avoiding a duplicate capability.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Zp1T5KEdGsfNRFSLhY3fK

* fix(report-feedback): read the cloud token the desktop actually writes

The auth probe used the wrong path (state/auth/) and required a `signedIn`
field. The desktop's readCloudAuth (electron/ipc.cjs) stores the record at
<workspace>/cloud-auth.json and treats "signed in" as simply having a `token`
(no signedIn field). Match that: read <workspace>/cloud-auth.json, also probe
the packaged-app canonical workspace (~/.sutando/repo/workspace) so the skill
finds the token across the dev/packaged split, and gate on token presence.

Verified end-to-end: a signed-in submission returns 200 and lands in the
feedback funnel + mirrored GitHub issue.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Zp1T5KEdGsfNRFSLhY3fK

* fix(report-feedback): announce+redact logs, harden tier note, fix null body

Addresses review on sonichi#2010 (qingyun-wu):
- SKILL.md: instruct the agent to ANNOUNCE the log attachment before sending
  ("I'll attach recent diagnostic logs unless you'd rather I didn't") and honor
  --no-logs — an informed opt-out for voice/chat, matching sonichi#57's checkbox.
- report-feedback.py: add _redact() backstop over the log excerpt (Bearer
  tokens, token=/api_key=/secret= values, common key formats sk-/xoxb-/ghp_/
  AKIA, and the home-dir username). Private-Slack mirror makes this non-blocking,
  but /api/feedback can also open GitHub issues, so scrub before it leaves.
- SKILL.md access-tier: state explicitly that non-owner Discord/Slack/Telegram
  route to sandboxed codex (no cloud token) and cannot reach this skill, so a
  non-owner can't ship the owner's logs into an issue.

Also fixes a real bug found while testing: an empty --body sent body:null, which
/api/feedback rejects (400 invalid_payload, 'Expected string, received null').
Now sends a string (falls back to the title), matching the desktop form.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DmsnVs5ffwHXdrQsLoZUCb

* fix(report-feedback): probe state/auth/cloud-auth.json (M1 canonical) first

Post-M1 the cloud auth record lives at <workspace>/state/auth/cloud-auth.json.
Add it as the first probe; retain the pre-M1 root path as a 30-day reader
fallback per the workspace contract. Mirror the same M1+fallback pair for the
packaged-app workspace so the skill finds the token regardless of checkout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw

* fix(report-feedback): match real AWS access keys in _redact() (no separator)

AKIA keys have no separator between the prefix and body (AKIAIOSFODNN7EXAMPLE),
so the shared [_-] pattern never matched them. Split out a dedicated
AKIA[A-Z0-9]{16} pattern that fires correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw

* fix(report-feedback): redact Slack app tokens

* test(report-feedback): cover main flow, cloud-auth branches, and logs excerpt

Raises diff coverage on report-feedback.py from 35% to 100%, clearing the
Coverage Gate on sonichi#2010. Adds tests for resolve_workspace (import + fallback),
the read_cloud_auth metering-env / dedup / malformed-file branches,
logs_excerpt (present / absent / unreadable), and main() (blank-title exit,
not-signed-in exit, successful POST with and without logs, HTTPError and
generic-error exits) via mocked urlopen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DC4oxV4ZwnKTKvE6Hb28Yo

* fix(report-feedback): redact Google API keys

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>
liususan091219 added a commit that referenced this pull request Jul 13, 2026
…ezone-aware now(UTC) (#2003)

* fix(friction-detector): replace deprecated datetime.utcnow() with timezone-aware now(UTC)

datetime.utcnow() was deprecated in Python 3.12 and scheduled for removal.
Both `now` and the parsed `updatedAt` are now timezone-aware (UTC), so their
difference is still correct.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QB3qZQrwponfum2JFNvHeh

* test(friction-detector): cover check_github_issues() datetime comparison for diff-coverage gate

Adds two tests that mock subprocess.run to exercise the
timezone-aware datetime comparison path without a live gh token,
satisfying the >= 95% diff-coverage gate on PR #2003.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QB3qZQrwponfum2JFNvHeh

* fix(tests): fix start-cli, startup-env-token-persist, sutando-migrate for Ubuntu CI (#2029)

* test(shell-ci): fix two ubuntu-portability failures from shell-ci-known-failures

Two tests fail on ubuntu-latest CI (wired in by #1948) due to macOS-specific
tooling; both pass on macOS today but gate CI on Ubuntu.

start-cli-claude-config-dir.test.sh (2 failures):
1. Ambient CLAUDE_CONFIG_DIR leak — the test runner session has CLAUDE_CONFIG_DIR
   set, which leaked into the subprocess and broke "not set" assertion. Fix: add
   `unset CLAUDE_CONFIG_DIR` in setup_sandbox() alongside the HOME/SUTANDO_WORKSPACE
   exports that already clear the environment.
2. /var/folders vs /private/var symlink mismatch — macOS mktemp returns /var/folders/…
   but cd+pwd -P resolves to /private/var/folders/…; sutando-config.sh uses the
   physical path, so the expected vs actual CLAUDE_CONFIG_DIR differed. Fix: resolve
   SANDBOX to its real path (cd+pwd -P) immediately after mktemp.
3. Stale expected path — expected was keyed on $SUTANDO_WORKSPACE (deprecated v0.8
   env-var, no longer read by the resolver) instead of the config-derived
   $REPO_FAKE/workspace. Fix: align expected to what sutando-config.sh actually computes.

startup-env-token-persist.test.sh (would fail on Ubuntu):
- stat -f '%Lp' (BSD mode format) and stat -f '%m' (BSD mtime) are macOS-only.
  Fix: replace both with equivalent Python one-liners (stat.S_IMODE + os.stat.st_mtime)
  that work on macOS and Linux without any extra deps.

After this PR, both suites should be removable from tests/shell-ci-known-failures.txt
once that file lands (via #1948). Closes two items from #1961.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw

* test(shell-ci): fix sutando-migrate.test.sh — wrong DEST + BSD stat + pipefail exit

Three bugs caused all 7 assertions to fail:

1. Wrong DEST — test used `SUTANDO_WORKSPACE="$DEST" --respect-env` but the
   workspace resolver ignores $SUTANDO_WORKSPACE since #1440. The backup landed
   in the real workspace instead of the temp fixture. Fix: use
   SUTANDO_MIGRATE_DEST (the documented E2E test hook symmetric with
   SUTANDO_MIGRATE_SRC_{A,B,C}).

2. Early exit via set -e — `grep -E "^sutando-migrate: backup"` exits 1 when
   the dest is empty (the backup line reads "dest empty; placeholder backup →
   …"). With `set -uo pipefail`, the INITIAL_BACKUP_ID assignment aborted the
   script before the assertion block ran. Fix: broaden the pattern to
   "migration-backup-.*\.tar\.gz" and add `|| true`.

3. BSD-only stat — `stat -f %m` is macOS-only; GNU stat uses `-c '%Y'`.
   Affects mtime comparison for notes/shared.md on Ubuntu. Fix: Python
   `os.stat().st_mtime` (portable, already used in startup-env-token-persist
   and start-cli fixes on this branch).

All 14 assertions now pass locally.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>

* feat(skills): report-feedback skill (report issues from any surface) (#2010)

* feat(skills): report-feedback skill (single reporting path)

The user can report a bug / feature request / feedback by asking Sutando from
ANY surface — chat, Discord, Telegram, or voice (via task delegation). One
skill, not a per-surface tool, so there's no duplication.

- skills/report-feedback/report-feedback.py posts to the cloud /api/feedback
  route (mirrors into GitHub issues) with kind/severity/title/body + a
  diagnostic context (platform + a tail of recent <workspace>/logs).
- Reads cloud-auth.json on disk (falls back to the metering env the desktop
  injects); prints NOT_SIGNED_IN + exits 2 when signed out, ERROR on API
  failure — never leaves the agent guessing.
- Owner-tier only.

Replaces the earlier voice-only report_issue inline tool (dropped) per owner:
the skill covers voice too via delegation, avoiding a duplicate capability.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Zp1T5KEdGsfNRFSLhY3fK

* fix(report-feedback): read the cloud token the desktop actually writes

The auth probe used the wrong path (state/auth/) and required a `signedIn`
field. The desktop's readCloudAuth (electron/ipc.cjs) stores the record at
<workspace>/cloud-auth.json and treats "signed in" as simply having a `token`
(no signedIn field). Match that: read <workspace>/cloud-auth.json, also probe
the packaged-app canonical workspace (~/.sutando/repo/workspace) so the skill
finds the token across the dev/packaged split, and gate on token presence.

Verified end-to-end: a signed-in submission returns 200 and lands in the
feedback funnel + mirrored GitHub issue.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Zp1T5KEdGsfNRFSLhY3fK

* fix(report-feedback): announce+redact logs, harden tier note, fix null body

Addresses review on #2010 (qingyun-wu):
- SKILL.md: instruct the agent to ANNOUNCE the log attachment before sending
  ("I'll attach recent diagnostic logs unless you'd rather I didn't") and honor
  --no-logs — an informed opt-out for voice/chat, matching #57's checkbox.
- report-feedback.py: add _redact() backstop over the log excerpt (Bearer
  tokens, token=/api_key=/secret= values, common key formats sk-/xoxb-/ghp_/
  AKIA, and the home-dir username). Private-Slack mirror makes this non-blocking,
  but /api/feedback can also open GitHub issues, so scrub before it leaves.
- SKILL.md access-tier: state explicitly that non-owner Discord/Slack/Telegram
  route to sandboxed codex (no cloud token) and cannot reach this skill, so a
  non-owner can't ship the owner's logs into an issue.

Also fixes a real bug found while testing: an empty --body sent body:null, which
/api/feedback rejects (400 invalid_payload, 'Expected string, received null').
Now sends a string (falls back to the title), matching the desktop form.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DmsnVs5ffwHXdrQsLoZUCb

* fix(report-feedback): probe state/auth/cloud-auth.json (M1 canonical) first

Post-M1 the cloud auth record lives at <workspace>/state/auth/cloud-auth.json.
Add it as the first probe; retain the pre-M1 root path as a 30-day reader
fallback per the workspace contract. Mirror the same M1+fallback pair for the
packaged-app workspace so the skill finds the token regardless of checkout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw

* fix(report-feedback): match real AWS access keys in _redact() (no separator)

AKIA keys have no separator between the prefix and body (AKIAIOSFODNN7EXAMPLE),
so the shared [_-] pattern never matched them. Split out a dedicated
AKIA[A-Z0-9]{16} pattern that fires correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw

* fix(report-feedback): redact Slack app tokens

* test(report-feedback): cover main flow, cloud-auth branches, and logs excerpt

Raises diff coverage on report-feedback.py from 35% to 100%, clearing the
Coverage Gate on #2010. Adds tests for resolve_workspace (import + fallback),
the read_cloud_auth metering-env / dedup / malformed-file branches,
logs_excerpt (present / absent / unreadable), and main() (blank-title exit,
not-signed-in exit, successful POST with and without logs, HTTPError and
generic-error exits) via mocked urlopen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DC4oxV4ZwnKTKvE6Hb28Yo

* fix(report-feedback): redact Google API keys

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>

* fix(voice+phone): add explicit local-machine identity to prevent cloud fabrication (#2014)

* fix(voice): add explicit local-machine identity to prevent cloud fabrication

Addresses #2011 — when asked where Sutando runs, Gemini was defaulting
to its training-data answer ("cloud-based AI"). Add an authoritative
system-instruction line so the model answers correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUA5TZgDa7VmFcNtPka38t

* test: update voice-behavior-anchors fixture for identity instruction

Regenerate the instructions hash after adding the local-machine identity
line to buildInstructions() (deliberate prompt change for #2011).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUA5TZgDa7VmFcNtPka38t

* fix(phone): add explicit local-machine identity to prevent cloud fabrication

Same issue as voice (#2011) — phone agent also lacked an authoritative
statement about running locally. Adds the same system-instruction line
to the inbound/outbound phone session path in buildPhoneInstructions().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUA5TZgDa7VmFcNtPka38t

* test: update phone-behavior-anchors fixture for identity instruction

Regenerate hashes after adding the local-machine identity line to
buildPhoneInstructions() (deliberate prompt change for #2011).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUA5TZgDa7VmFcNtPka38t

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>
Co-authored-by: Susan Xueqing Liu <xliu127@stevens.edu>
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.

3 participants