Skip to content

fix(ego-browser): probe the installed skill location for the agent workspace - #223

Open
Hotragn wants to merge 1 commit into
citrolabs:devfrom
Hotragn:fix/agent-workspace-installed-fallback
Open

fix(ego-browser): probe the installed skill location for the agent workspace#223
Hotragn wants to merge 1 commit into
citrolabs:devfrom
Hotragn:fix/agent-workspace-installed-fallback

Conversation

@Hotragn

@Hotragn Hotragn commented Aug 6, 2026

Copy link
Copy Markdown

Summary

In the app-bundled CLI the agent-workspace fallback resolves to a path that does not exist (from a root cwd, literally /skills/ego-browser), so the entire site-learnings subsystem is unreachable even though the learning packs ship with the app and are present on disk. This PR keeps the existing resolution precedence and adds one narrow step: when neither module-relative candidate exists on disk, probe the user-level location onboarding registers, accepting it only when it actually holds learnings/.

Related issue

Fixes the runtime-side root cause reported in #215. Note the reporter self-closed that issue (before this PR opened) after finding the in-process process.env.EGO_BROWSER_AGENT_WORKSPACE = ... workaround — the resolution defect itself is still present on dev, and this PR makes the workaround unnecessary for the CLI half. The app-side half (caller env not forwarded into the embedded runtime) remains out of this repo's reach.

Changes

  • src/env.tsagentWorkspace() keeps the current precedence (env override → skill bundled next to the build output → repo layout). New behavior only starts where the old chain returned a nonexistent path: when neither module-relative candidate exists, it probes ~/.local/share/ego/ego-skills (the location onboarding links, per the report in [Bug]: Site learnings unreachable — agent workspace falls back to /skills/ego-browser (filesystem root) in the bundled CLI #215) both as the skill directory itself and as a parent of ego-browser/. A candidate is accepted only when it holds learnings/, so a stale leftover directory cannot claim the workspace. When nothing exists anywhere, the returned value is byte-identical to today's (the repo-layout path).
  • The home directory is resolved from HOME / USERPROFILE directly rather than through resolvePath("~..."), deliberately: the ~-expansion defect in resolvePath is already being fixed by fix(ego-browser): expand ~ paths to the home directory instead of the filesystem root #146 and fix(ego-browser): make the repo build, test, and commit on Windows #148, and this PR must not collide with either.
  • agentWorkspace() gains an injectable { env, exists } seam (defaulting to process.env / existsSync) so the bundled-runtime layout is testable without touching the real filesystem — same injectable-seam pattern as update-notice.ts. All call sites are unchanged (agentWorkspace()).
  • src/env.test.mjs — 8 regression tests covering the full decision table: override precedence, bundled candidate, repo-layout candidate, installed-root fallback, ego-browser/ subdirectory priority, rejection of an installed directory without learnings/, USERPROFILE resolution, and the nothing-exists default.

Note: prettier normalized two pre-existing unformatted blocks in env.ts (resolvePath, loadEnvFile) because the changed-file style gate checks whole files. Same situation #148 documented.

Verification

Run from package/ego-browser (Windows 11, Node 24; suite is platform-neutral):

npm test                      -> 319 pass, 0 fail (311 existing + 8 new)
npm run typecheck             -> ok
npm run validate:site-skills  -> site skills ok
npm audit --audit-level=moderate -> 0 vulnerabilities
prettier --check src/env.ts src/env.test.mjs -> ok

The new tests fail without the fix: reverting the src/env.ts change makes "falls back to the installed skill registered by onboarding", "prefers an ego-browser subdirectory inside the install root", and "resolves the home directory from USERPROFILE" fail (the old chain returns the repo-layout path).

I don't have a macOS install to run the shipped app against; the repro and paths come from the detailed report in #215. Behavior for repo checkouts is covered by the tests and unchanged.

Impact

  • Public helper API or behavior
  • Agent skill or instructions
  • Site learning
  • Installation or update flow
  • Build, CI, or release process
  • Documentation only
  • No externally visible impact

Behavior change is strictly additive: the fallback only fires where the previous result was a nonexistent directory, so learnContext() / runSiteTool / runSiteBrowserTool start working in the app-bundled CLI without EGO_BROWSER_AGENT_WORKSPACE. Repo checkouts, the env override, and the SDK path resolve exactly as before. Composes cleanly with #146/#148 (different functions in the same file; no shared hunks beyond formatting).

Checklist

  • The PR targets the correct base branch (dev for normal changes; only dev may target main).
  • The change is focused and does not include unrelated cleanup.
  • Tests were added or updated for behavior changes, or the reason they are unnecessary is explained above.
  • Relevant tests and validation commands pass locally.
  • Public helper JSDoc and agent-facing documentation are updated when the helper surface changes.
  • No credentials, tokens, cookies, personal data, or other secrets are included.
  • A release-note label is selected (fix).

@Hotragn

Hotragn commented Aug 19, 2026

Copy link
Copy Markdown
Author

Freshness check, since this has been open a while and the tree has moved elsewhere: the defect is still present on the active line.

package/ego-browser/src/env.ts on sprint-1.3.0 (currently c1b1184) is byte-identical to dev in the part this PR changes — agentWorkspace() still returns resolve(REPO_ROOT, "..", "..", "skills", "ego-browser") unconditionally when the bundled-sibling probe misses, which is how the search root becomes /skills/ego-browser in the app-bundled runtime (the report in #215).

So unlike #224/#251/#252 — whose files the rearchitecture removes — this one applies unchanged to whichever branch you take it on. It is still green and conflict-free against dev, and I have a sprint-1.3.0-based rebase ready if you would rather have it there; one word and I will retarget instead of opening anything new.

(The ~-expansion defect in the same file is also still present on sprint-1.3.0; that half belongs to #146/#148 and this PR deliberately stays clear of it.)

@Hotragn

Hotragn commented Aug 23, 2026

Copy link
Copy Markdown
Author

Rebased onto the active line and re-verified. agentWorkspace() on 2.0.0-beta-dev (a25f488) still returns resolve(REPO_ROOT, .., .., skills, ego-browser) unconditionally when the bundled-sibling probe misses, so the defect this fixes is present there too — not just on dev.

A v2-ready branch is pushed: https://github.com/Hotragn/ego-lite/tree/beta/agent-workspace (branched from a25f488, 8/8 new tests pass). One adaptation was needed: env.ts no longer exports SRC_DIR/REPO_ROOT on that line, so the test derives those paths from the built module's own location rather than widening the module's public surface for a test.

I could not open a PR for it — 2.0.0-beta-dev refuses CreatePullRequest from forks, while the same account opens PRs against dev normally (details in #312). Happy to send it wherever you prefer, or you can pull the branch directly. This PR remains valid for the dev line as-is.

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.

1 participant