Skip to content

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

Merged
qingyun-wu merged 3 commits into
sonichi:mainfrom
john-the-dev:fix/shell-test-ubuntu-portability
Jul 9, 2026
Merged

fix(tests): fix start-cli, startup-env-token-persist, sutando-migrate for Ubuntu CI#2029
qingyun-wu merged 3 commits into
sonichi:mainfrom
john-the-dev:fix/shell-test-ubuntu-portability

Conversation

@john-the-dev

@john-the-dev john-the-dev commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Closes

Removes 3 items from tests/shell-ci-known-failures.txt once #1948 lands.
Closes 3 of the failures from #1961.

Summary

Three shell test suites fail on ubuntu-latest (wired into CI by #1948) due to macOS-specific tooling. All pass on macOS today and will gate the CI matrix on Ubuntu once #1948 merges.

start-cli-claude-config-dir.test.sh (3 fixes in ba48f69):

  1. Ambient CLAUDE_CONFIG_DIR leak — test runner session had it set, leaked into subprocess, broke "not set" assertion. Fix: unset CLAUDE_CONFIG_DIR in setup_sandbox().
  2. /var/folders vs /private/var symlink mismatch — macOS mktemp returns /var/... but cd+pwd -P resolves to /private/var/.... Fix: resolve SANDBOX to its real path immediately after mktemp.
  3. Stale expected path — expected was keyed on $SUTANDO_WORKSPACE (no longer read since fix(workspace-v0.8): strip $SUTANDO_WORKSPACE from resolver + colored warnings [DRAFT] #1440) instead of the config-derived path. Fix: align expected to $REPO_FAKE/workspace.

startup-env-token-persist.test.sh (1 fix in ba48f69):

  • stat -f '%Lp' (BSD mode format) and stat -f '%m' (BSD mtime) are macOS-only. Fix: Python stat.S_IMODE + os.stat.st_mtime (portable, already used in sync-workspace.test.sh).

sutando-migrate.test.sh (3 fixes in 3eda785):

  1. Wrong DEST — used SUTANDO_WORKSPACE="$DEST" but the resolver ignores $SUTANDO_WORKSPACE since fix(workspace-v0.8): strip $SUTANDO_WORKSPACE from resolver + colored warnings [DRAFT] #1440. Backup landed in real workspace. Fix: use SUTANDO_MIGRATE_DEST (documented E2E test hook, symmetric with SUTANDO_MIGRATE_SRC_{A,B,C}).
  2. Early exit via set -e — grep "^sutando-migrate: backup" exits 1 when dest is empty. Fix: broaden pattern to migration-backup-.*\.tar\.gz + || true.
  3. BSD-only stat -f %m for mtime comparison. Fix: Python os.stat().st_mtime.

Checklist

  • Confirmed no other open PR closes the same issue (gh pr list --repo sonichi/sutando --search "closes #N")
  • Git author email matches CLA-signed email (52230987+john-the-dev@users.noreply.github.com)
  • Single concern per PR — shell test Ubuntu portability fixes only
  • Confirmed failures exist on upstream/main (macOS-only tooling, non-portable stat syntax)
  • Tests: all 3 suites pass locally after fixes (4/4, 7/7, 14/14 assertions)
  • Doesn't touch V1-workspace-hold areas ✓

Test plan

bash tests/start-cli-claude-config-dir.test.sh   # PASSED: 4
bash tests/startup-env-token-persist.test.sh      # Results: 7 passed, 0 failed
bash tests/sutando-migrate.test.sh                 # ALL ASSERTIONS PASSED (14)

All pass locally on macOS. Portability fixes are stat→Python and env-var unset, which work on both platforms.

john-the-dev and others added 2 commits July 8, 2026 08:21
@john-the-dev @claude
test(shell-ci): fix two ubuntu-portability failures from shell-ci-kno…
ba48f69 
…wn-failures

Two tests fail on ubuntu-latest CI (wired in by sonichi#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 sonichi#1948). Closes two items from sonichi#1961.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EL2XBcV3RsWhevzTPtW4jw
@john-the-dev @claude
test(shell-ci): fix sutando-migrate.test.sh — wrong DEST + BSD stat +…
3eda785 
… 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 sonichi#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
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

@john-the-dev
john-the-dev marked this pull request as ready for review July 8, 2026 17:43
qingyun-wu
qingyun-wu reviewed Jul 9, 2026
View reviewed changes

@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.

Reviewed — LGTM. All three fixes are correct test hygiene: pwd -P for macOS's /var→/private/var symlink, unsetting ambient CLAUDE_CONFIG_DIR so the runner's own session can't poison the "not set" assertion, python-based mode/mtime checks for BSD/GNU stat portability, and the expectation moving off the deprecated $SUTANDO_WORKSPACE onto the config-resolved path (matches the post-#1440 resolver). CI green.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

@qingyun-wu
qingyun-wu merged commit e450815 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
@john-the-dev @claude @qingyun-wu
fix(tests): fix start-cli, startup-env-token-persist, sutando-migrate…
a602a1b 
… for Ubuntu CI (sonichi#2029)

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

Two tests fail on ubuntu-latest CI (wired in by sonichi#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 sonichi#1948). Closes two items from sonichi#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 sonichi#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>
@qingyun-wu qingyun-wu mentioned this pull request Jul 10, 2026
Merged
2 tasks done
liususan091219 added a commit that referenced this pull request Jul 13, 2026
@john-the-dev @claude
@qingyun-wu @liususan091219
fix(friction-detector): replace deprecated datetime.utcnow() with tim…
a8aae51 
…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

Reviewers

@qingyun-wu qingyun-wu qingyun-wu left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@john-the-dev @qingyun-wu