Skip to content

fix(app): resolve menu-bar avatar from workspace, not repoRoot - #1931

Merged
qingyun-wu merged 2 commits into
mainfrom
fix/app-avatar-workspace-path
Jul 6, 2026
Merged

fix(app): resolve menu-bar avatar from workspace, not repoRoot#1931
qingyun-wu merged 2 commits into
mainfrom
fix/app-avatar-workspace-path

Conversation

@sonichi

@sonichi sonichi commented Jul 6, 2026

Copy link
Copy Markdown
Owner

The menu-bar avatar is loaded from repoRoot + "/assets/stand-avatar.png", but the avatar asset lives under the workspace, not the code checkout. Align both load sites (setupMenuBar, avatarImage) to resolve from workspace, per the workspace contract.

Stand: Echo Act IV Pro

Avatar lives under the workspace assets/, not the code checkout; align both
load sites (setupMenuBar, avatarImage) with the workspace contract.

Stand: Echo Act IV Pro

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

LGTM — verified against the canonical resolver, not just the PR description.

  • src/util_paths.ts already special-cases stand-avatar.pngjoin(ws, 'assets', filename) (both the existence-probing and non-probing paths), so dashboard/web-client surfaces resolve the avatar under the workspace. These two Swift sites were the last stragglers on repoRoot.
  • assets/ doesn't exist in the repo at all (404 on the contents API) and the "refuse workspace/ contents in commits" CI guard means it never can — so the old repoRoot + "/assets/..." path was dead-by-default, and this change is strictly a fix, not a relocation risk.
  • Both load sites fail soft (if let/guard let) if a workspace lacks the asset, same as before.
  • CI green incl. CLA.

@john-the-dev

Copy link
Copy Markdown
Collaborator

Cold review at head (dd55163) — LGTM with one observation.

Verified on this host (mac-local checkout):

  • AppDelegate.workspace exists (main.swift:33, SutandoConfig.resolveWorkspace()), so the symbol resolves; direction matches the workspace contract (per-user asset → workspace, not code checkout).
  • Both load sites fail nil-safe: setupMenuBar falls back to the "S" title button, and avatarImage's only caller uses if let — so a missing asset degrades gracefully either way.
  • Observation: on this host stand-avatar.png exists in neither <repo>/assets/ nor <workspace>/assets/ (and assets/ isn't git-tracked), so the menu bar has always shown the "S" fallback here. The PR is a no-op on hosts without the asset and correct on hosts that keep it in the workspace — but nothing currently places it there. Worth a follow-up line in the install/setup docs (or startup.sh) so <workspace>/assets/stand-avatar.png is an advertised drop point.
  • Couldn't build-verify Swift on this host (known Swift 5.3 CLT limitation post-feat(workspace-revamp): staging→main rollup (M0+M1+M2) #1454); review is source-level.

Owner merges.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

@john-the-dev john-the-dev 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.

LGTM. Two-line fix: repoRoot is the checkout path, not the user-data path — the avatar is served state, so workspace is correct. Matters when the workspace has been migrated or when the app bundle lives at a different path than the checkout.

@qingyun-wu
qingyun-wu merged commit 0334325 into main Jul 6, 2026
6 checks passed
@qingyun-wu
qingyun-wu deleted the fix/app-avatar-workspace-path branch July 6, 2026 22:36
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