fix: green V1 baseline — 34 failing tests, 7 root causes - #722
Open
henrynguyeninfo1 wants to merge 4 commits into
Open
fix: green V1 baseline — 34 failing tests, 7 root causes#722henrynguyeninfo1 wants to merge 4 commits into
henrynguyeninfo1 wants to merge 4 commits into
Conversation
…ot causes) Reproduced from clean state: 12 failed files / 34 failed tests / 708 pass. Each cluster traced to implementation-vs-test intent via git history: 1. vitest collected e2e/*.spec.ts Playwright specs (@playwright/test not a dependency; specs need a live server). Added e2e/** to test.exclude in vite.config.ts — they run via the Playwright runner only. 2. Stores consolidated under the workspace state dir ($HERMES_HOME/workspace, outsourc-e#439/08920193) but mcp-presets-store, mcp-hub-sources-store, mcp-tools-cache and /api/mcp/presets tests (from outsourc-e#231) still wrote/asserted files at $HERMES_HOME root, so every validation/caching/seeding contract silently exercised the seed path (24 tests). Tests now resolve paths through the stores' own presetsFilePath()/hubSourcesFilePath()/state-dir contract; stale module doc in mcp-presets-store.ts corrected. Two tools-cache corrupt-file tests were passing for the wrong reason (file written where the store never reads) — also corrected. 3. /api/mcp/hub-search: outsourc-e#325 deliberately added offset pagination and raised the limit cap 100→500; tests predated it. Updated expectations, added offset parse/clamp coverage, fixed stale route doc comment. 4. i18n: outsourc-e#248 deliberately renamed zh label to 中文（简体） when adding zh-TW 繁體中文; test expected the old label. Updated + now covers zh-TW. 5. chat-composer workspace picker: release-bot commit 901ffcd ("fix(update): realign clean repos") silently deleted the 75-line workspace-context picker JSX while leaving its state, query and mutation dead in place. Restored the picker (button + dropdown + menu-exclusivity wiring) — implementation was defective, test right. 6. hermes-config route test: vi.doUnmock('gateway-capabilities') in the 503 test stripped the file-level mock for the later legacy-alias test, which then hit the real capability probe → providers:[] → TypeError. Replaced doMock/doUnmock with mutable hoisted capability state. The maskedKeys alias implementation was already correct. 7. chat-message-list: test contracted behavior whose implementation slice was never landed (wip 92c19c3 notes "needs reconciliation"). Implemented it: buildDisplayEntries no longer glues trailing persisted tool-only assistant messages onto the previous text reply, and new getTrailingToolOnlyTurnSummary() reports hidden trailing tool activity {count, toolNames, hasFinalAssistantText} for UI disclosure. Independent Codex review (REQUEST-CHANGES) drove three follow-up fixes: buildDisplayEntries now routes a toolResult to the pending tool-only turn before considering the previous entry (result no longer leaks onto the earlier text reply; +2 behavior tests); model-menu button restores setIsWorkspaceMenuOpen(false) exclusivity; the HERMES_HOME override test pins the literal workspace/mcp-presets.json path instead of asserting through presetsFilePath() tautologically. Verification: pnpm test → 115 files / 745 tests green; pnpm build → green. Zero-fork capability-gated design untouched; no assertions weakened — stale tests were realigned to deliberate landed features, defective implementations fixed. Not-tested: getTrailingToolOnlyTurnSummary is not yet wired into a UI surface (no design existed); no migration exists for pre-outsourc-e#439 user files left at $HERMES_HOME root (upstream gap, pre-existing); e2e specs have no in-repo Playwright runner config (pre-existing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7NmxpGp3YneJBaqir9F4T
henrynguyeninfo1
marked this pull request as ready for review
July 19, 2026 00:22
The Workspace picker only showed models advertised on the gateway's /v1/models (active model + hermes-agent alias), while the agent CLI and Hermes desktop dashboard offer the full provider catalog. The agent already persists that catalog at $HERMES_HOME/cache/model_catalog.json (providers.*.models, refreshed by `hermes model`). Add readAgentModelCatalog() as an additional merge source in /api/models: flattens providers.*.models into provider-tagged entries, tolerates string or object entries, and degrades to an empty list when the cache is missing or malformed (capability-gated by file presence — no new config, no fork of agent behavior). Verified live: picker grows from 2 to 41 models (source=models.json+hermes-agent+agent-catalog) with the full OpenRouter/Nous catalog. 3 new behavior-contract tests; suite 748 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7NmxpGp3YneJBaqir9F4T
henrynguyeninfo1
force-pushed
the
henrynguyeninfo1/fix-workspace-baseline
branch
from
July 19, 2026 00:43
789ef5e to
0fd4b64
Compare
The marketplace tab always fell back to bundled skills ("No hub skills
found") because the search pipeline was broken in three places:
1. scripts/skills-search.py hardcoded sys.path to ~/hermes-agent — the
agent's standard install lives at ~/.hermes/hermes-agent, so the
tools.skills_hub import always failed. Root resolution now tries
HERMES_AGENT_ROOT, $HERMES_HOME/hermes-agent, ~/.hermes/hermes-agent,
then the legacy ~/hermes-agent path.
2. The route invoked the system python3, which lacks the agent's deps
(httpx). It now prefers $HERMES_HOME/hermes-agent/venv/bin/python
when present, falling back to python3.
3. The Electron build never shipped scripts/skills-search.py, so the
packaged app could not hub-search at all. Added to electron-builder
files.
Verified live (dev + packaged app): /api/skills/hub-search?q=github now
returns source=skills-hub results from the skills.sh index instead of
bundled-skills-fallback. Suite 748 green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7NmxpGp3YneJBaqir9F4T
The packaged Electron app broke three ways for one underlying reason:
server code assumed a dev-checkout layout that does not exist inside
<App>.app/Contents/Resources/app.
1. MCP catalog dead in app: seedAssetPath() called
new URL('.', import.meta.url), which throws "Invalid URL" in the
esbuild CJS server bundle (import.meta.url is undefined), so
/api/mcp/presets returned source=invalid and the hub 'local' source
rejected. The import.meta walk is now try/caught with the cwd-based
assets/ candidate (shipped in the app) as fallback.
2. Terminal dead in app: PTY_HELPER resolved to <electron>/pty-helper.py
(bundle __dirname) while vite ships the helper at
dist/server/assets/pty-helper.py — every session died instantly with
exit code 2 in a reconnect loop. Helper path now probes the dev
(src/server), dist-assets, and bundle-relative candidates.
3. Mutable state written INSIDE the installed bundle: local session
history, tool artifacts, pending update notes, and swarm
mission/mode/runtime state all used <cwd>/.runtime, which in the app
is the read-only-by-convention Resources/app dir — wiped on every
update. New runtimeStateDir() keeps per-checkout <cwd>/.runtime for
dev but routes desktop runs (HERMES_WORKSPACE_DESKTOP=1) to
<workspace-state-dir>/runtime.
Verified in the packaged app: /api/mcp/presets ok=true (seeded 7),
hub-search local source clean, Terminal spawns a live shell, and no
.runtime is created inside /Applications/hermes-workspace.app.
Suite 748 green; web build green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7NmxpGp3YneJBaqir9F4T
codeCraft-Ritik
left a comment
codeCraft-Ritik
left a comment
There was a problem hiding this comment.
Impressive cleanup! I like that each failing test was classified before making changes, distinguishing stale tests, harness issues, and genuine product regressions. That approach makes the resulting fixes much easier to trust.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fresh checkout of v2.3.0 (
c1e6ed97) reproducibly failedpnpm test: 12 files / 34 tests. Every cluster was traced against git history to decide implementation-defect vs stale-test before touching anything. Result: 29 stale/misconfigured tests realigned, 2 product regressions fixed, 2 test-harness bugs fixed, 1 never-landed contract implemented.Root causes
e2e/**added to vitest exclude$HERMES_HOMEroot901ffcd5deleted 75 lines of picker JSX unrelated to its message, left dead state/mutationmaskedKeysTypeErrorvi.doUnmockstripped the file-level mock for later testsgetTrailingToolOnlyTurnSummarymissing92c19c30"needs reconciliation")Independent review
Cross-vendor (Codex) review drove 3 follow-ups now included:
buildDisplayEntriesroutes atoolResultto the pending tool-only turn before the previous entry (+2 behavior tests); model-menu button restores workspace-menu exclusivity; HERMES_HOME override test pins the literal path instead of asserting tautologically.Verification
pnpm test: 115 files / 745 tests green (multiple clean runs)pnpm build: green/api/workspaceentries, menu exclusivity holds, zero console errorsKnown follow-ups (out of scope here)
getTrailingToolOnlyTurnSummaryhas no UI consumer yet — needs a disclosure affordance design@playwright/testwas never a dependency)$HERMES_HOMEroot🤖 Generated with Claude Code
https://claude.ai/code/session_01X7NmxpGp3YneJBaqir9F4T