mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
Merge origin/main into lane/c3-memory-panel
This commit is contained in:
teknium1
commit
54f5696bbb
23 changed files with 1252 additions and 89 deletions
|
|
@ -6253,6 +6253,13 @@ def _resolve_task_provider_model(
|
|||
cfg_model = str(task_config.get("model", "")).strip() or None
|
||||
cfg_base_url = str(task_config.get("base_url", "")).strip() or None
|
||||
cfg_api_key = str(task_config.get("api_key", "")).strip() or None
|
||||
# Resolve key_env → env var when api_key is not set directly
|
||||
if not cfg_api_key:
|
||||
cfg_key_env = str(
|
||||
task_config.get("key_env") or task_config.get("api_key_env") or ""
|
||||
).strip()
|
||||
if cfg_key_env:
|
||||
cfg_api_key = os.getenv(cfg_key_env, "").strip() or None
|
||||
cfg_api_mode = str(task_config.get("api_mode", "")).strip() or None
|
||||
|
||||
# 'auto' is a sentinel meaning "inherit from main runtime / auto-detect", not
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ from agent.prompt_builder import (
|
|||
drain_truncation_warnings,
|
||||
)
|
||||
from agent.runtime_cwd import resolve_context_cwd
|
||||
from hermes_constants import get_hermes_home
|
||||
from utils import is_truthy_value
|
||||
|
||||
|
||||
|
|
@ -395,7 +396,7 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None)
|
|||
if active_profile == "default":
|
||||
stable_parts.append(
|
||||
"Active Hermes profile: default. Other profiles (if any) live "
|
||||
"under ~/.hermes/profiles/<name>/. Each profile has its own "
|
||||
"under " + str(get_hermes_home()) + "/profiles/<name>/. Each profile has its own "
|
||||
"skills/, plugins/, cron/, and memories/ that affect a different "
|
||||
"session than this one. Do not modify another profile's "
|
||||
"skills/plugins/cron/memories unless the user explicitly directs "
|
||||
|
|
@ -404,9 +405,9 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None)
|
|||
else:
|
||||
stable_parts.append(
|
||||
f"Active Hermes profile: {active_profile}. This session reads "
|
||||
f"and writes ~/.hermes/profiles/{active_profile}/. The default "
|
||||
f"profile's data lives at ~/.hermes/skills/, ~/.hermes/plugins/, "
|
||||
f"~/.hermes/cron/, ~/.hermes/memories/ — those belong to a "
|
||||
f"and writes {get_hermes_home()}/profiles/{active_profile}/. The default "
|
||||
f"profile's data lives at {get_hermes_home()}/skills/, {get_hermes_home()}/plugins/, "
|
||||
f"{get_hermes_home()}/cron/, {get_hermes_home()}/memories/ — those belong to a "
|
||||
f"different session run from a different shell. Do NOT modify "
|
||||
f"another profile's skills/plugins/cron/memories unless the user "
|
||||
f"explicitly directs you to. The cross-profile write guard will "
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
import { TextMessagePartProvider, useMessagePartText } from '@assistant-ui/react'
|
||||
import {
|
||||
parseMarkdownIntoBlocks,
|
||||
type StreamdownTextComponents,
|
||||
StreamdownTextPrimitive,
|
||||
type SyntaxHighlighterProps,
|
||||
|
|
@ -17,6 +16,7 @@ import { chunkByLines, SyntaxHighlighter } from '@/components/chat/shiki-highlig
|
|||
import { ZoomableImage } from '@/components/chat/zoomable-image'
|
||||
import { normalizeExternalUrl, openExternalLink, PrettyLink } from '@/lib/external-link'
|
||||
import { createMemoizedMathPlugin } from '@/lib/katex-memo'
|
||||
import { parseMarkdownIntoBlocksCached } from '@/lib/markdown-blocks'
|
||||
import { preprocessMarkdown } from '@/lib/markdown-preprocess'
|
||||
import {
|
||||
downloadGatewayMediaFile,
|
||||
|
|
@ -59,43 +59,6 @@ function preprocessWithTailRepair(text: string): string {
|
|||
}
|
||||
}
|
||||
|
||||
// Memoized block splitter. Streamdown calls `parseMarkdownIntoBlocks` (a full
|
||||
// `marked` lex of the entire message, ~1.6ms per 28KB) inside a useMemo keyed
|
||||
// on the text — but the same text is re-lexed every time a message REMOUNTS
|
||||
// (virtualizer scroll, session switch). A small module-level
|
||||
// LRU keyed by the exact source string removes all of those repeat parses
|
||||
// with zero correctness risk (same input → same output). Streaming tail
|
||||
// growth misses the cache by design (every flush is a new string) — that
|
||||
// single lex is the irreducible cost.
|
||||
const BLOCK_CACHE_MAX = 64
|
||||
const BLOCK_CACHE_MIN_LENGTH = 1024
|
||||
const blockCache = new Map<string, string[]>()
|
||||
|
||||
function parseMarkdownIntoBlocksCached(markdown: string): string[] {
|
||||
if (markdown.length < BLOCK_CACHE_MIN_LENGTH) {
|
||||
return parseMarkdownIntoBlocks(markdown)
|
||||
}
|
||||
|
||||
const hit = blockCache.get(markdown)
|
||||
|
||||
if (hit) {
|
||||
// Refresh recency (Map iteration order is insertion order).
|
||||
blockCache.delete(markdown)
|
||||
blockCache.set(markdown, hit)
|
||||
|
||||
return hit
|
||||
}
|
||||
|
||||
const blocks = parseMarkdownIntoBlocks(markdown)
|
||||
blockCache.set(markdown, blocks)
|
||||
|
||||
if (blockCache.size > BLOCK_CACHE_MAX) {
|
||||
blockCache.delete(blockCache.keys().next().value as string)
|
||||
}
|
||||
|
||||
return blocks
|
||||
}
|
||||
|
||||
async function mediaSrc(path: string): Promise<string> {
|
||||
if (/^(?:https?|data):/i.test(path)) {
|
||||
return path
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ import { useEnterAnimation } from '@/lib/use-enter-animation'
|
|||
import { cn } from '@/lib/utils'
|
||||
import { recordPreviewArtifact } from '@/store/preview-status'
|
||||
import { $activeSessionId, $currentCwd } from '@/store/session'
|
||||
import { $toolInlineDiffs } from '@/store/tool-diffs'
|
||||
import { $toolInlineDiff } from '@/store/tool-diffs'
|
||||
import { $toolRowDismissed, dismissToolRow } from '@/store/tool-dismiss'
|
||||
import { $toolDisclosureOpen, $toolViewMode, setToolDisclosureOpen } from '@/store/tool-view'
|
||||
|
||||
|
|
@ -283,8 +283,9 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
const disclosureId = `tool-entry:${messageId}:${toolPartDisclosureId(stablePart)}`
|
||||
const dismissed = useStore($toolRowDismissed(disclosureId))
|
||||
const isPending = messageRunning && result === undefined
|
||||
const liveDiffs = useStore($toolInlineDiffs)
|
||||
const sideDiff = toolCallId ? liveDiffs[toolCallId] || '' : ''
|
||||
// Subscribe to this tool's diff only, so a live patch for one tool doesn't
|
||||
// re-render every mounted tool row (the factory caches a per-id atom).
|
||||
const sideDiff = useStore($toolInlineDiff(toolCallId ?? ''))
|
||||
const inlineDiff = stripInlineDiffChrome(sideDiff) || inlineDiffFromResult(result)
|
||||
const isFileEdit = isFileEditTool(toolName)
|
||||
const defaultOpen = Boolean(inlineDiff)
|
||||
|
|
|
|||
164
apps/desktop/src/lib/markdown-blocks.test.ts
Normal file
164
apps/desktop/src/lib/markdown-blocks.test.ts
Normal file
|
|
@ -0,0 +1,164 @@
|
|||
import { parseMarkdownIntoBlocks } from '@assistant-ui/react-streamdown'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { parseMarkdownIntoBlocksCached } from './markdown-blocks'
|
||||
|
||||
// The contract: streaming through the cached splitter (one call per growing
|
||||
// prefix, exactly how Streamdown calls it per flush) must produce, at every
|
||||
// step, the same blocks as a fresh full lex of that prefix. Byte equality —
|
||||
// a divergence would change what the memoized block renderer paints.
|
||||
|
||||
const CORPUS = `# Heading
|
||||
|
||||
Intro paragraph with **bold**, [a link](https://example.com), \`inline\` and $x^2$ math.
|
||||
|
||||
- list item one
|
||||
- list item two
|
||||
- nested item
|
||||
|
||||
1. ordered a
|
||||
|
||||
2. loose ordered b
|
||||
|
||||
\`\`\`python
|
||||
def f(x):
|
||||
return x * 2 # comment with \`\`\` inside string? no — fence chars below
|
||||
\`\`\`
|
||||
|
||||
A paragraph that will be followed by a setext underline
|
||||
===
|
||||
|
||||
| col a | col b |
|
||||
|---|---|
|
||||
| 1 | 2 |
|
||||
| 3 | 4 |
|
||||
|
||||
> blockquote line one
|
||||
> blockquote line two
|
||||
with a lazy continuation line
|
||||
|
||||
<div class="raw">
|
||||
html block content
|
||||
</div>
|
||||
|
||||
$$
|
||||
\\int_0^1 x\\,dx = \\tfrac12
|
||||
$$
|
||||
|
||||
Final paragraph after everything, long enough to stream in pieces so the tail
|
||||
block keeps getting reinterpreted while earlier blocks stay settled.
|
||||
`
|
||||
|
||||
// Deterministic PRNG so failures reproduce.
|
||||
function mulberry32(seed: number) {
|
||||
let a = seed
|
||||
|
||||
return () => {
|
||||
a |= 0
|
||||
a = (a + 0x6d2b79f5) | 0
|
||||
let t = Math.imul(a ^ (a >>> 15), 1 | a)
|
||||
t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t
|
||||
|
||||
return ((t ^ (t >>> 14)) >>> 0) / 4294967296
|
||||
}
|
||||
}
|
||||
|
||||
// Push the text past the cache MIN_LENGTH thresholds so the incremental
|
||||
// path actually engages.
|
||||
const LONG_CORPUS = Array.from({ length: 6 }, () => CORPUS).join('\n')
|
||||
|
||||
describe('parseMarkdownIntoBlocksCached', () => {
|
||||
it('matches a full lex at every random streaming cut (property)', () => {
|
||||
for (let seed = 1; seed <= 5; seed++) {
|
||||
const rand = mulberry32(seed)
|
||||
let cursor = 0
|
||||
|
||||
while (cursor < LONG_CORPUS.length) {
|
||||
cursor = Math.min(LONG_CORPUS.length, cursor + 1 + Math.floor(rand() * 120))
|
||||
const prefix = LONG_CORPUS.slice(0, cursor)
|
||||
|
||||
expect(parseMarkdownIntoBlocksCached(prefix)).toEqual(parseMarkdownIntoBlocks(prefix))
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('matches a full lex when streaming token-by-token through a fence boundary', () => {
|
||||
const base = `${'settled paragraph one.\n\n'.repeat(100)}opening a fence now:\n`
|
||||
const tail = '```js\nconst a = 1\nconst b = 2\n```\n\nafter the fence\n'
|
||||
|
||||
for (let i = 1; i <= tail.length; i++) {
|
||||
const text = base + tail.slice(0, i)
|
||||
|
||||
expect(parseMarkdownIntoBlocksCached(text)).toEqual(parseMarkdownIntoBlocks(text))
|
||||
}
|
||||
})
|
||||
|
||||
it('reconstructs the input exactly (join property the offsets rely on)', () => {
|
||||
const blocks = parseMarkdownIntoBlocksCached(LONG_CORPUS)
|
||||
|
||||
expect(blocks.join('')).toBe(LONG_CORPUS)
|
||||
})
|
||||
|
||||
it('falls back to a full lex for non-append rewrites (edit / branch swap)', () => {
|
||||
const grown = `${LONG_CORPUS}\n\nappended tail paragraph`
|
||||
parseMarkdownIntoBlocksCached(grown)
|
||||
|
||||
// A REWRITE that shares no prefix lineage must still be correct.
|
||||
const rewritten = `completely different start\n\n${LONG_CORPUS.slice(500)}`
|
||||
|
||||
expect(parseMarkdownIntoBlocksCached(rewritten)).toEqual(parseMarkdownIntoBlocks(rewritten))
|
||||
})
|
||||
|
||||
it('matches a full lex when a trailing setext underline merges the previous block (regression)', () => {
|
||||
// A trailing `-`/`=` line is a setext underline of the block ABOVE it, so
|
||||
// appending to it can retroactively merge the previous parse's LAST TWO
|
||||
// blocks into one. Cached `"…#e\n5\n-"` lexes to [ …, "#e\n", "5\n-" ], but
|
||||
// grown to `"…#e\n5\n-p2=kj:c"` collapses `#e`/`5\n-` into one block. The
|
||||
// old boundary dropped only the single last content block, so it reused a
|
||||
// `"#e\n"` block that no longer exists. `blocks.join('') === text` still
|
||||
// holds for the wrong split, so the reconstruction guard cannot catch it.
|
||||
// The settled prefix pushes the text past the append-cache threshold so
|
||||
// the incremental path actually engages.
|
||||
const settled = 'settled line paragraph text.\n\n'.repeat(80)
|
||||
const prev = `${settled}#e\n5\n-`
|
||||
const grown = `${prev}p2=kj:c`
|
||||
|
||||
// Seed the append cache with `prev`, then grow it — the exact two-call
|
||||
// sequence a streaming flush produces.
|
||||
parseMarkdownIntoBlocksCached(prev)
|
||||
|
||||
expect(parseMarkdownIntoBlocksCached(grown)).toEqual(parseMarkdownIntoBlocks(grown))
|
||||
})
|
||||
|
||||
// 12 seeds × 500 growing prefixes is ~6000 full+cached lexes; it first trips
|
||||
// the pre-fix boundary at seed 11 / step 257, so the workload can't shrink
|
||||
// without gutting the guard. The work is bounded but exceeds one test's 5s
|
||||
// default budget, so raise the timeout rather than weaken the coverage.
|
||||
it('matches a full lex at every char-level streaming cut over noisy markdown (property fuzz)', () => {
|
||||
// Character-level append fuzz over the markdown control alphabet — the
|
||||
// harness that surfaced the setext-underline merge above. Growing a single
|
||||
// lineage one small chunk at a time keeps `startsWith` lineage intact so
|
||||
// the incremental path runs on nearly every step; each prefix must
|
||||
// deep-equal a fresh full lex.
|
||||
const alphabet = '\n `#*-_>[]()|~:=abcdefghijklmnopqrstuvwxyz0123456789'
|
||||
|
||||
for (let seed = 1; seed <= 12; seed++) {
|
||||
const rand = mulberry32(seed)
|
||||
// Seed past the append-cache threshold so the incremental path engages.
|
||||
let text = `seed ${seed}\n\n`.repeat(180)
|
||||
|
||||
for (let step = 0; step < 500; step++) {
|
||||
const n = 1 + Math.floor(rand() * 24)
|
||||
let chunk = ''
|
||||
|
||||
for (let j = 0; j < n; j++) {
|
||||
chunk += alphabet[Math.floor(rand() * alphabet.length)]
|
||||
}
|
||||
|
||||
text += chunk
|
||||
|
||||
expect(parseMarkdownIntoBlocksCached(text)).toEqual(parseMarkdownIntoBlocks(text))
|
||||
}
|
||||
}
|
||||
}, 30_000)
|
||||
})
|
||||
136
apps/desktop/src/lib/markdown-blocks.ts
Normal file
136
apps/desktop/src/lib/markdown-blocks.ts
Normal file
|
|
@ -0,0 +1,136 @@
|
|||
import { parseMarkdownIntoBlocks } from '@assistant-ui/react-streamdown'
|
||||
|
||||
/**
|
||||
* Block splitting for the streaming markdown pipeline, without re-lexing the
|
||||
* whole message on every token flush.
|
||||
*
|
||||
* `parseMarkdownIntoBlocks` is a full `marked` lex of the entire text —
|
||||
* measured 3.4–9.6ms per call at 64–192KB. During streaming every flush is a
|
||||
* new string, so the stock splitter pays that O(full-text) cost ~30×/s on
|
||||
* long replies. Two caches remove it:
|
||||
*
|
||||
* 1. Exact-string LRU — a message that REMOUNTS with unchanged text
|
||||
* (virtualizer scroll, session switch) reuses its parse outright.
|
||||
* 2. Streaming-append cache — when the new text starts with a recently parsed
|
||||
* text (the token-append case), the previous parse's blocks are reused up
|
||||
* to a settled boundary and only the suffix is lexed. The boundary drops
|
||||
* the previous parse's trailing whitespace-only blocks AND its last content
|
||||
* block, because appended text can retroactively change how that last
|
||||
* block parses (open fence, list/table continuation, setext underline, a
|
||||
* lazy blockquote line). Blocks before it are separated by settled blank
|
||||
* lines and cannot be affected. Cross-block reference links can't regress:
|
||||
* Streamdown renders each block as an independent markdown document
|
||||
* already. Verified property: `blocks.join('') === text`, and incremental
|
||||
* output is asserted byte-identical to a full lex in tests across fences,
|
||||
* lists, tables, setext headings, blockquotes, and HTML blocks.
|
||||
*
|
||||
* Any doubt — no prefix match, reconstruction mismatch — falls back to the
|
||||
* full lex, i.e. exactly the previous behavior.
|
||||
*/
|
||||
|
||||
const EXACT_CACHE_MAX = 64
|
||||
const EXACT_CACHE_MIN_LENGTH = 1024
|
||||
const exactCache = new Map<string, string[]>()
|
||||
|
||||
// Streaming messages grow monotonically, and only a handful stream at once
|
||||
// (main reply + reasoning part, maybe a tile). A tiny ring is enough; each
|
||||
// entry holds the last parse for one growing text lineage.
|
||||
const APPEND_CACHE_MAX = 4
|
||||
const APPEND_CACHE_MIN_LENGTH = 2048
|
||||
const appendCache: { blocks: string[]; text: string }[] = []
|
||||
|
||||
function rememberAppend(text: string, blocks: string[]): void {
|
||||
if (text.length < APPEND_CACHE_MIN_LENGTH) {
|
||||
return
|
||||
}
|
||||
|
||||
// Replace the lineage this text grew from (its cached prefix), else push.
|
||||
const index = appendCache.findIndex(entry => text.startsWith(entry.text))
|
||||
|
||||
if (index !== -1) {
|
||||
appendCache.splice(index, 1)
|
||||
}
|
||||
|
||||
appendCache.push({ blocks, text })
|
||||
|
||||
if (appendCache.length > APPEND_CACHE_MAX) {
|
||||
appendCache.shift()
|
||||
}
|
||||
}
|
||||
|
||||
function lexIncrementally(text: string): null | string[] {
|
||||
const entry = appendCache.find(cached => text.length > cached.text.length && text.startsWith(cached.text))
|
||||
|
||||
if (!entry) {
|
||||
return null
|
||||
}
|
||||
|
||||
// Settled boundary: drop the last TWO content blocks (skipping any
|
||||
// whitespace-only blocks around them). Dropping only the single last content
|
||||
// block is unsound: appended text can retroactively merge the previous
|
||||
// parse's last two blocks into one. The trigger is a trailing Setext
|
||||
// underline — `marked` only treats `-`/`=` as an underline for the paragraph
|
||||
// ABOVE it, so a settled `"#e\n5\n-"` lexes as ["#e\n", "5\n-"], but growing
|
||||
// the tail to `"#e\n5\n-p2=kj:c"` collapses both into one paragraph. The
|
||||
// block before the last is the deepest an append can reach (the underline
|
||||
// consumes exactly one preceding block), so re-lexing the last two is safe;
|
||||
// earlier blocks are fenced off by settled blank lines. join('') === text
|
||||
// still holds either way, so the reconstruction check below can't catch this.
|
||||
let keep = entry.blocks.length
|
||||
|
||||
for (let dropped = 0; dropped < 2 && keep > 0; dropped += 1) {
|
||||
while (keep > 0 && !entry.blocks[keep - 1].trim()) {
|
||||
keep -= 1
|
||||
}
|
||||
|
||||
if (keep > 0) {
|
||||
keep -= 1
|
||||
}
|
||||
}
|
||||
|
||||
if (keep === 0) {
|
||||
return null
|
||||
}
|
||||
|
||||
const settled = entry.blocks.slice(0, keep)
|
||||
let settledLength = 0
|
||||
|
||||
for (const block of settled) {
|
||||
settledLength += block.length
|
||||
}
|
||||
|
||||
// Defensive reconstruction check — the splitter's join(blocks) === text
|
||||
// property is what makes offsets exact. If it ever doesn't hold, full lex.
|
||||
if (settledLength > entry.text.length || !text.startsWith(entry.text.slice(0, settledLength), 0)) {
|
||||
return null
|
||||
}
|
||||
|
||||
return [...settled, ...parseMarkdownIntoBlocks(text.slice(settledLength))]
|
||||
}
|
||||
|
||||
export function parseMarkdownIntoBlocksCached(markdown: string): string[] {
|
||||
if (markdown.length < EXACT_CACHE_MIN_LENGTH) {
|
||||
return parseMarkdownIntoBlocks(markdown)
|
||||
}
|
||||
|
||||
const hit = exactCache.get(markdown)
|
||||
|
||||
if (hit) {
|
||||
// Refresh recency (Map iteration order is insertion order).
|
||||
exactCache.delete(markdown)
|
||||
exactCache.set(markdown, hit)
|
||||
|
||||
return hit
|
||||
}
|
||||
|
||||
const blocks = lexIncrementally(markdown) ?? parseMarkdownIntoBlocks(markdown)
|
||||
|
||||
rememberAppend(markdown, blocks)
|
||||
exactCache.set(markdown, blocks)
|
||||
|
||||
if (exactCache.size > EXACT_CACHE_MAX) {
|
||||
exactCache.delete(exactCache.keys().next().value as string)
|
||||
}
|
||||
|
||||
return blocks
|
||||
}
|
||||
58
apps/desktop/src/lib/query-client.test.ts
Normal file
58
apps/desktop/src/lib/query-client.test.ts
Normal file
|
|
@ -0,0 +1,58 @@
|
|||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
import { invalidateProfileScopedQueries, queryClient } from './query-client'
|
||||
|
||||
function invalidated(key: unknown[]): boolean {
|
||||
return queryClient.getQueryState(key)?.isInvalidated ?? false
|
||||
}
|
||||
|
||||
describe('invalidateProfileScopedQueries', () => {
|
||||
beforeEach(() => {
|
||||
queryClient.clear()
|
||||
})
|
||||
|
||||
it('invalidates profile-scoped caches and leaves account/global caches intact', () => {
|
||||
const profileScoped = [
|
||||
['hermes-config-record'],
|
||||
['hermes-config-schema'],
|
||||
['skills-list'],
|
||||
['toolsets-list'],
|
||||
['model-options', 'global'],
|
||||
['command-palette', 'sessions'],
|
||||
['session-picker', 'sessions']
|
||||
]
|
||||
|
||||
const global = [
|
||||
['billing', 'state'],
|
||||
['billing', 'subscription'],
|
||||
['marketplace-themes', 'all'],
|
||||
['marketplace-themes-settings', 'x'],
|
||||
['onboarding-model-options', 'y'],
|
||||
['contrib-logs-tail']
|
||||
]
|
||||
|
||||
for (const key of [...profileScoped, ...global]) {
|
||||
queryClient.setQueryData(key, { seeded: true })
|
||||
}
|
||||
|
||||
invalidateProfileScopedQueries()
|
||||
|
||||
for (const key of profileScoped) {
|
||||
expect(invalidated(key), `${JSON.stringify(key)} should be invalidated`).toBe(true)
|
||||
}
|
||||
|
||||
for (const key of global) {
|
||||
expect(invalidated(key), `${JSON.stringify(key)} should be left intact`).toBe(false)
|
||||
}
|
||||
})
|
||||
|
||||
it('invalidates unknown/non-string-rooted keys by default (correctness-safe)', () => {
|
||||
queryClient.setQueryData(['some-future-profile-query'], 1)
|
||||
queryClient.setQueryData([{ scope: 'weird' }], 1)
|
||||
|
||||
invalidateProfileScopedQueries()
|
||||
|
||||
expect(invalidated(['some-future-profile-query'])).toBe(true)
|
||||
expect(invalidated([{ scope: 'weird' }])).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
@ -18,3 +18,31 @@ export const writeCache =
|
|||
<T>(key: QueryKey) =>
|
||||
(next: T | undefined | ((prev: T | undefined) => T | undefined)): void =>
|
||||
void queryClient.setQueryData<T>(key, next)
|
||||
|
||||
// Query-key roots that are NOT profile-scoped: account/billing, the theme
|
||||
// marketplace, onboarding, and contrib log tails all read global or
|
||||
// account-level state, so a profile/gateway swap must not refetch them. Any
|
||||
// other key is treated as profile-scoped and invalidated -- a denylist is
|
||||
// correctness-safe here: a root we forget to list just gets refetched (a small
|
||||
// cost), whereas an allowlist that misses a profile-scoped key would paint the
|
||||
// previous profile's data (a bug).
|
||||
const PROFILE_INDEPENDENT_QUERY_ROOTS = new Set<string>([
|
||||
'billing',
|
||||
'marketplace-themes',
|
||||
'marketplace-themes-settings',
|
||||
'onboarding-model-options',
|
||||
'contrib-logs-tail'
|
||||
])
|
||||
|
||||
// Invalidate profile-scoped query caches on a profile / gateway switch, leaving
|
||||
// account/global caches intact. Replaces a keyless invalidateQueries() that
|
||||
// refetched everything (billing, marketplace, onboarding) on every switch.
|
||||
export function invalidateProfileScopedQueries(): void {
|
||||
void queryClient.invalidateQueries({
|
||||
predicate: query => {
|
||||
const root = query.queryKey[0]
|
||||
|
||||
return typeof root !== 'string' || !PROFILE_INDEPENDENT_QUERY_ROOTS.has(root)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ import {
|
|||
import { $gatewaySwitching, wipeSessionListsForGatewaySwitch } from './gateway-switch'
|
||||
|
||||
vi.mock('@/lib/query-client', () => ({
|
||||
queryClient: { invalidateQueries: vi.fn() }
|
||||
invalidateProfileScopedQueries: vi.fn()
|
||||
}))
|
||||
|
||||
describe('wipeSessionListsForGatewaySwitch', () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { atom } from 'nanostores'
|
||||
|
||||
import { queryClient } from '@/lib/query-client'
|
||||
import { invalidateProfileScopedQueries } from '@/lib/query-client'
|
||||
import { resetSessionsLimit } from '@/store/layout'
|
||||
import {
|
||||
$unreadFinishedSessionIds,
|
||||
|
|
@ -57,5 +57,7 @@ export function wipeSessionListsForGatewaySwitch(): void {
|
|||
setMessages([])
|
||||
setFreshDraftReady(true)
|
||||
|
||||
void queryClient.invalidateQueries()
|
||||
// Narrowed: account/marketplace/onboarding caches are global, not gateway-
|
||||
// scoped, so a mode swap must not refetch them.
|
||||
invalidateProfileScopedQueries()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,14 +16,14 @@ vi.mock('@/hermes', () => ({
|
|||
getProfiles: vi.fn(async () => ({ profiles: [] })),
|
||||
setApiRequestProfile: vi.fn()
|
||||
}))
|
||||
vi.mock('@/lib/query-client', () => ({ queryClient: { invalidateQueries: vi.fn() } }))
|
||||
vi.mock('@/lib/query-client', () => ({ invalidateProfileScopedQueries: vi.fn() }))
|
||||
vi.mock('@/store/starmap', () => ({ resetStarmapGraph }))
|
||||
|
||||
const { $activeGatewayProfile, $profiles, ensureGatewayProfile, prewarmProfileBackend, refreshProfiles } =
|
||||
await import('./profile')
|
||||
|
||||
const { $connection } = await import('./session')
|
||||
const { queryClient } = await import('@/lib/query-client')
|
||||
const { invalidateProfileScopedQueries } = await import('@/lib/query-client')
|
||||
const { getProfiles } = await import('@/hermes')
|
||||
|
||||
const profile = (name: string, isDefault = false): ProfileInfo => ({
|
||||
|
|
@ -53,7 +53,7 @@ beforeEach(() => {
|
|||
$connection.set(localConn())
|
||||
$profiles.set([])
|
||||
vi.stubGlobal('window', { hermesDesktop: { getConnection } })
|
||||
vi.mocked(queryClient.invalidateQueries).mockClear()
|
||||
vi.mocked(invalidateProfileScopedQueries).mockClear()
|
||||
resetStarmapGraph.mockClear()
|
||||
})
|
||||
|
||||
|
|
@ -114,7 +114,7 @@ describe('profile-scoped cache invalidation', () => {
|
|||
it('drops the memory graph cache when the active gateway profile changes', () => {
|
||||
$activeGatewayProfile.set('coder')
|
||||
|
||||
expect(queryClient.invalidateQueries).toHaveBeenCalled()
|
||||
expect(invalidateProfileScopedQueries).toHaveBeenCalled()
|
||||
expect(resetStarmapGraph).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { atom, computed } from 'nanostores'
|
||||
|
||||
import { getProfiles, setApiRequestProfile, STARTUP_REQUEST_TIMEOUT_MS } from '@/hermes'
|
||||
import { queryClient } from '@/lib/query-client'
|
||||
import { invalidateProfileScopedQueries } from '@/lib/query-client'
|
||||
import {
|
||||
arraysEqual,
|
||||
persistBoolean,
|
||||
|
|
@ -177,7 +177,9 @@ $activeGatewayProfile.subscribe(value => {
|
|||
|
||||
if (_lastRoutedProfile !== null && _lastRoutedProfile !== key) {
|
||||
// Profile-scoped settings + the unified session list are now stale.
|
||||
void queryClient.invalidateQueries()
|
||||
// Narrowed so account/marketplace/onboarding caches don't refetch on
|
||||
// every profile switch.
|
||||
invalidateProfileScopedQueries()
|
||||
resetStarmapGraph()
|
||||
}
|
||||
|
||||
|
|
|
|||
47
apps/desktop/src/store/tool-diffs.test.ts
Normal file
47
apps/desktop/src/store/tool-diffs.test.ts
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { $toolInlineDiff, getToolDiff, recordToolDiff } from './tool-diffs'
|
||||
|
||||
describe('tool-diffs per-tool subscriptions', () => {
|
||||
it('returns a stable cached atom per toolCallId', () => {
|
||||
expect($toolInlineDiff('a')).toBe($toolInlineDiff('a'))
|
||||
expect($toolInlineDiff('a')).not.toBe($toolInlineDiff('b'))
|
||||
})
|
||||
|
||||
it('notifies only the tool whose diff changed', () => {
|
||||
const aCalls: string[] = []
|
||||
const bCalls: string[] = []
|
||||
const unsubA = $toolInlineDiff('notify-a').listen(v => aCalls.push(v))
|
||||
const unsubB = $toolInlineDiff('notify-b').listen(v => bCalls.push(v))
|
||||
|
||||
recordToolDiff('notify-a', 'diffA')
|
||||
expect(aCalls).toEqual(['diffA'])
|
||||
expect(bCalls).toEqual([]) // the unrelated tool row is never notified
|
||||
|
||||
recordToolDiff('notify-b', 'diffB')
|
||||
expect(aCalls).toEqual(['diffA']) // still not re-notified
|
||||
expect(bCalls).toEqual(['diffB'])
|
||||
|
||||
unsubA()
|
||||
unsubB()
|
||||
})
|
||||
|
||||
it('does not re-notify when the same diff is recorded again', () => {
|
||||
const calls: string[] = []
|
||||
const unsub = $toolInlineDiff('same').listen(v => calls.push(v))
|
||||
|
||||
recordToolDiff('same', 'x')
|
||||
recordToolDiff('same', 'x')
|
||||
|
||||
expect(calls).toEqual(['x'])
|
||||
unsub()
|
||||
})
|
||||
|
||||
it('reads the current diff for a tool and empty for unknown/blank ids', () => {
|
||||
recordToolDiff('read-me', 'value')
|
||||
expect(getToolDiff('read-me')).toBe('value')
|
||||
expect(getToolDiff('missing')).toBe('')
|
||||
expect(getToolDiff('')).toBe('')
|
||||
expect($toolInlineDiff('').get()).toBe('')
|
||||
})
|
||||
})
|
||||
|
|
@ -1,7 +1,13 @@
|
|||
import { atom } from 'nanostores'
|
||||
import { atom, computed, type ReadableAtom } from 'nanostores'
|
||||
|
||||
const $toolDiffs = atom<Record<string, string>>({})
|
||||
|
||||
// Per-tool derived atoms, cached by toolCallId. A `ToolEntry` subscribes only
|
||||
// to its own id's diff, so recording a diff for one tool re-renders that one
|
||||
// row -- not every mounted tool row. computed() only notifies when the derived
|
||||
// string actually changes, so unrelated writes to the map are inert here.
|
||||
const inlineDiffCache = new Map<string, ReadableAtom<string>>()
|
||||
|
||||
export function recordToolDiff(toolCallId: string, diff: string) {
|
||||
if (!toolCallId || !diff) {
|
||||
return
|
||||
|
|
@ -20,4 +26,13 @@ export function getToolDiff(toolCallId: string): string {
|
|||
return toolCallId ? $toolDiffs.get()[toolCallId] || '' : ''
|
||||
}
|
||||
|
||||
export const $toolInlineDiffs = $toolDiffs
|
||||
export function $toolInlineDiff(toolCallId: string): ReadableAtom<string> {
|
||||
let cached = inlineDiffCache.get(toolCallId)
|
||||
|
||||
if (!cached) {
|
||||
cached = computed($toolDiffs, diffs => (toolCallId ? diffs[toolCallId] || '' : ''))
|
||||
inlineDiffCache.set(toolCallId, cached)
|
||||
}
|
||||
|
||||
return cached
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7829,11 +7829,16 @@ def _quote_env_value(value: str) -> str:
|
|||
"""Quote .env values containing characters with special dotenv meaning."""
|
||||
if value == "":
|
||||
return value
|
||||
# Internal whitespace (space/tab/etc.) must be quoted so shell `set -a; . file`
|
||||
# word-splits don't break paths like macOS "Application Support". Leading/
|
||||
# trailing whitespace is already covered by the strip check; any() covers
|
||||
# internal runs that strip() would leave alone.
|
||||
needs_quoting = (
|
||||
"#" in value
|
||||
or '"' in value
|
||||
or "'" in value
|
||||
or value != value.strip()
|
||||
or any(c.isspace() for c in value)
|
||||
)
|
||||
if not needs_quoting:
|
||||
return value
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import codecs
|
||||
import io
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
|
@ -21,6 +23,12 @@ _CREDENTIAL_SUFFIXES = ("_API_KEY", "_TOKEN", "_SECRET", "_KEY")
|
|||
# tests) don't spam the same warning multiple times.
|
||||
_WARNED_KEYS: set[str] = set()
|
||||
|
||||
# Paths we've already emitted a UTF-32 refuse-to-mangle warning for.
|
||||
# load_hermes_dotenv can call _sanitize_env_file_if_needed multiple times
|
||||
# for the same file (user env + project env + hot-reload); once per path
|
||||
# is enough.
|
||||
_WARNED_UTF32_PATHS: set[str] = set()
|
||||
|
||||
# Map of env-var name → source label ("bitwarden", etc.) for credentials
|
||||
# that were injected by an external secret source during load_hermes_dotenv().
|
||||
# Used by setup / `hermes model` flows to label detected credentials so
|
||||
|
|
@ -176,6 +184,12 @@ def _sanitize_env_file_if_needed(path: Path) -> None:
|
|||
with ``ValueError: embedded null byte`` — typically introduced by
|
||||
copy-pasting API keys from terminals or rich-text editors.
|
||||
|
||||
Encoding: sniffs a leading BOM *before* any text decode. UTF-16
|
||||
(Notepad "Unicode") is decoded correctly and rewritten as clean
|
||||
UTF-8. UTF-32 is refused (left untouched) so we never fall through
|
||||
to the errors=replace corruption path. Order of BOM checks matters:
|
||||
UTF-32-LE's BOM starts with UTF-16-LE's FF FE.
|
||||
|
||||
We delegate to ``hermes_cli.config._sanitize_env_lines`` which
|
||||
already knows all valid Hermes env-var names and can split
|
||||
concatenated lines correctly.
|
||||
|
|
@ -187,16 +201,70 @@ def _sanitize_env_file_if_needed(path: Path) -> None:
|
|||
except ImportError:
|
||||
return # early bootstrap — config module not available yet
|
||||
|
||||
read_kw = {"encoding": "utf-8-sig", "errors": "replace"}
|
||||
try:
|
||||
with open(path, **read_kw) as f:
|
||||
original = f.readlines()
|
||||
raw = path.read_bytes()
|
||||
except Exception:
|
||||
return
|
||||
|
||||
# Sniff leading BOM bytes BEFORE decoding. ORDER MATTERS:
|
||||
# codecs.BOM_UTF32_LE is FF FE 00 00, which startswith
|
||||
# codecs.BOM_UTF16_LE (FF FE). Checking UTF-16 first would
|
||||
# misdetect UTF-32-LE as UTF-16-LE and mangle the file.
|
||||
force_utf8_rewrite = False
|
||||
if raw.startswith(codecs.BOM_UTF32_LE) or raw.startswith(codecs.BOM_UTF32_BE):
|
||||
# Lazy import keeps the module import block identical to #65124's
|
||||
# codecs/io additions so the two PRs auto-merge either order.
|
||||
path_key = str(path.resolve())
|
||||
if path_key not in _WARNED_UTF32_PATHS:
|
||||
_WARNED_UTF32_PATHS.add(path_key)
|
||||
import logging
|
||||
|
||||
logging.getLogger(__name__).warning(
|
||||
"Skipping .env sanitize for %s: UTF-32 BOM detected; "
|
||||
"leaving file untouched to avoid corruption",
|
||||
path,
|
||||
)
|
||||
return
|
||||
if raw.startswith(codecs.BOM_UTF16_LE) or raw.startswith(codecs.BOM_UTF16_BE):
|
||||
# "utf-16" uses the BOM to select endianness and strips it.
|
||||
# TextIOWrapper + newline=None matches open()'s universal-newlines
|
||||
# line splitting (\\n/\\r\\n/\\r only — not splitlines()'s extra
|
||||
# Unicode boundaries like U+2028), so sanitize sees the same lines
|
||||
# as the UTF-8 path.
|
||||
try:
|
||||
with io.TextIOWrapper(
|
||||
io.BytesIO(raw), encoding="utf-16", newline=None
|
||||
) as f:
|
||||
original = f.readlines()
|
||||
except UnicodeDecodeError:
|
||||
return
|
||||
# Source is UTF-16 on disk; always rewrite as clean UTF-8 so
|
||||
# the subsequent utf-8 dotenv load sees a canonical file.
|
||||
force_utf8_rewrite = True
|
||||
else:
|
||||
# Default path: utf-8-sig (strips UTF-8 BOM if present) with
|
||||
# errors=replace so embedded NULs can be stripped below.
|
||||
try:
|
||||
with open(path, encoding="utf-8-sig", errors="replace") as f:
|
||||
original = f.readlines()
|
||||
except Exception:
|
||||
return
|
||||
# Defense-in-depth: errors=replace turns undecodable leading
|
||||
# bytes into U+FFFD. Persisting that glues replacement chars
|
||||
# onto the first key name and rewrites the file permanently
|
||||
# (the UTF-16-with-BOM corruption path before BOM sniffing).
|
||||
# Leave the file untouched rather than write the mangling.
|
||||
if original and original[0].startswith("\ufffd"):
|
||||
return
|
||||
|
||||
try:
|
||||
# Strip null bytes before _sanitize_env_lines so they never
|
||||
# reach python-dotenv (which passes them to os.environ and
|
||||
# crashes with ValueError).
|
||||
# crashes with ValueError). Also intentionally repairs
|
||||
# BOM-less UTF-16 (NUL-padded ASCII) into clean UTF-8.
|
||||
stripped = [line.replace("\x00", "") for line in original]
|
||||
sanitized = _sanitize_env_lines(stripped)
|
||||
if sanitized != original:
|
||||
if sanitized != original or force_utf8_rewrite:
|
||||
import tempfile
|
||||
fd, tmp = tempfile.mkstemp(
|
||||
dir=str(path.parent), suffix=".tmp", prefix=".env_"
|
||||
|
|
|
|||
|
|
@ -443,7 +443,7 @@ function Get-PowerShellHostExe {
|
|||
}
|
||||
|
||||
function Install-Uv {
|
||||
# Hermes owns its own uv at $HermesHome\bin\uv.exe. Always install there —
|
||||
# Hermes owns its own uv at $HermesHome\bin\uv.exe. Always install there --
|
||||
# no PATH probing, no conda guards, no multi-location resolution chains.
|
||||
# The runtime update path (hermes_cli/managed_uv.py) looks in the same
|
||||
# place, so install.ps1 and `hermes update` stay in sync.
|
||||
|
|
@ -528,7 +528,7 @@ function Ensure-NodeExeOnPath {
|
|||
# prior process is not visible here. Later stages (Test-Python,
|
||||
# Install-Venv, Install-Dependencies, Install-PlatformSdks) call this
|
||||
# at the top to populate $script:UvCmd from the managed location.
|
||||
# Throws if uv is not findable — the caller's stage then surfaces a
|
||||
# Throws if uv is not findable -- the caller's stage then surfaces a
|
||||
# clean error via the stage-driver's try/catch.
|
||||
function Resolve-UvCmd {
|
||||
# Already resolved (default invocation path: Install-Uv ran earlier
|
||||
|
|
@ -544,7 +544,7 @@ function Resolve-UvCmd {
|
|||
# Stale; fall through to re-discover.
|
||||
}
|
||||
|
||||
# Check the managed location first — this is where Install-Uv puts it.
|
||||
# Check the managed location first -- this is where Install-Uv puts it.
|
||||
$managedUv = Join-Path $HermesHome "bin\uv.exe"
|
||||
if (Test-Path $managedUv) {
|
||||
$script:UvCmd = $managedUv
|
||||
|
|
@ -1516,7 +1516,7 @@ function Install-Repository {
|
|||
if ($LASTEXITCODE -ne 0) { throw "git checkout $Branch failed (exit $LASTEXITCODE)" }
|
||||
# Managed installs should follow origin/$Branch exactly. If
|
||||
# the checkout has diverged (or has local-only commits),
|
||||
# ff-only pull cannot succeed — mirror ``hermes update`` and
|
||||
# ff-only pull cannot succeed -- mirror ``hermes update`` and
|
||||
# reset to the fetched remote so bootstrap/install can recover.
|
||||
git -c windows.appendAtomically=false pull --ff-only origin $Branch
|
||||
if ($LASTEXITCODE -ne 0) {
|
||||
|
|
@ -1573,11 +1573,11 @@ function Install-Repository {
|
|||
Write-Host ""
|
||||
Write-Host "Conflicted files:"
|
||||
foreach ($file in $conflictedFiles) {
|
||||
Write-Host " • $file"
|
||||
Write-Host " - $file"
|
||||
}
|
||||
}
|
||||
Write-Host ""
|
||||
Write-Info "Your stashed changes are preserved — nothing is lost."
|
||||
Write-Info "Your stashed changes are preserved -- nothing is lost."
|
||||
Write-Info " Stash ref: $autostashRef"
|
||||
git -c windows.appendAtomically=false reset --hard HEAD 2>$null | Out-Null
|
||||
Write-Info "Working tree reset to clean state."
|
||||
|
|
@ -1798,8 +1798,8 @@ function Install-Venv {
|
|||
# /End stops a running task instance; /Change /DISABLE stops it
|
||||
# from re-firing mid-install. (The Startup-folder .vbs fallback is
|
||||
# NOT touched: it only fires at logon, so it cannot respawn a
|
||||
# gateway mid-install.) Re-enabled in the finally below — including
|
||||
# on failure — but only for tasks that were enabled to begin with.
|
||||
# gateway mid-install.) Re-enabled in the finally below -- including
|
||||
# on failure -- but only for tasks that were enabled to begin with.
|
||||
# Best-effort: a missing task just errors quietly.
|
||||
try {
|
||||
schtasks /Query /FO CSV 2>$null | ConvertFrom-Csv | Where-Object { $_.TaskName -like '*Hermes_Gateway*' } | ForEach-Object {
|
||||
|
|
@ -1893,7 +1893,7 @@ function Install-Venv {
|
|||
}
|
||||
|
||||
# Clean up parked venvs from previous installs whose handles have since
|
||||
# been released. Best-effort — a still-held tree just stays for next time.
|
||||
# been released. Best-effort -- a still-held tree just stays for next time.
|
||||
Get-ChildItem -Directory -Filter "venv.stale.*" -ErrorAction SilentlyContinue | ForEach-Object {
|
||||
Remove-Item -Recurse -Force $_.FullName -ErrorAction SilentlyContinue
|
||||
}
|
||||
|
|
@ -1926,10 +1926,10 @@ function Install-Venv {
|
|||
} finally {
|
||||
Pop-Location
|
||||
# Re-arm the gateway autostart tasks disabled during the venv teardown
|
||||
# — in a finally so a failed teardown/creation can never strand the
|
||||
# -- in a finally so a failed teardown/creation can never strand the
|
||||
# user's gateway autostart in the disabled state. Same function scope,
|
||||
# so the list survives even under the stage-per-process bootstrap.
|
||||
# Deliberately NOT started here — dependencies aren't installed yet;
|
||||
# Deliberately NOT started here -- dependencies aren't installed yet;
|
||||
# the task fires normally on next logon and `hermes update` / the
|
||||
# gateway resume path handles the immediate restart.
|
||||
if ($gatewayTasksDisabled -and $gatewayTasksDisabled.Count -gt 0) {
|
||||
|
|
@ -2247,7 +2247,7 @@ function Set-PathVariable {
|
|||
function Write-BootstrapMarker {
|
||||
# Writes $InstallDir\.hermes-bootstrap-complete which tells the Hermes
|
||||
# desktop app (apps/desktop/electron/main.ts) "install.ps1 ran
|
||||
# successfully — DON'T trigger the legacy first-launch bootstrap
|
||||
# successfully -- DON'T trigger the legacy first-launch bootstrap
|
||||
# runner."
|
||||
#
|
||||
# Schema mirrors what main.ts's writeBootstrapMarker() / isBootstrap
|
||||
|
|
@ -2268,7 +2268,7 @@ function Write-BootstrapMarker {
|
|||
# Resolve the pinned commit: explicit -Commit wins, otherwise read
|
||||
# the checkout's HEAD via git. If git can't run, leave commit empty
|
||||
# and the marker will fail desktop validation (pinnedCommit.length
|
||||
# >= 7) — better to be invalid than wrong.
|
||||
# >= 7) -- better to be invalid than wrong.
|
||||
$pinnedCommit = $Commit
|
||||
if (-not $pinnedCommit) {
|
||||
# PS 5.1 doesn't support the ?. null-conditional operator, so
|
||||
|
|
@ -2283,7 +2283,7 @@ function Write-BootstrapMarker {
|
|||
$pinnedCommit = $resolved.Trim()
|
||||
}
|
||||
} catch {
|
||||
# Ignore — pinnedCommit stays empty, marker stays invalid,
|
||||
# Ignore -- pinnedCommit stays empty, marker stays invalid,
|
||||
# desktop falls through to its legacy bootstrap path.
|
||||
} finally {
|
||||
Pop-Location
|
||||
|
|
@ -2302,7 +2302,7 @@ function Write-BootstrapMarker {
|
|||
pinnedCommit = $pinnedCommit
|
||||
pinnedBranch = $pinnedBranch
|
||||
completedAt = (Get-Date).ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ss.fffZ")
|
||||
# desktopVersion field intentionally omitted — only the desktop
|
||||
# desktopVersion field intentionally omitted -- only the desktop
|
||||
# app knows its own version, and the marker validator doesn't
|
||||
# require it. The desktop fills it in if/when it writes its
|
||||
# own marker (e.g. after a future in-app upgrade).
|
||||
|
|
@ -2311,7 +2311,7 @@ function Write-BootstrapMarker {
|
|||
|
||||
# Write WITHOUT a UTF-8 BOM. PowerShell 5.1's `Set-Content -Encoding UTF8`
|
||||
# always emits a BOM, and Node's plain JSON.parse rejects the BOM as an
|
||||
# unexpected character — so a BOM'd marker would silently fail the
|
||||
# unexpected character -- so a BOM'd marker would silently fail the
|
||||
# desktop's readJson(), make isBootstrapComplete() return null, and the
|
||||
# desktop would re-run the legacy bootstrap runner anyway. Defeats the
|
||||
# whole point. Use the .NET API directly for BOM-less UTF-8.
|
||||
|
|
@ -2411,7 +2411,7 @@ function Install-NodeDeps {
|
|||
# Cross-process driver mode (Hermes-Setup.exe runs each -Stage NAME
|
||||
# in a fresh powershell.exe) means $script:HasNode set by Stage-Node
|
||||
# in the previous process isn't visible here. Re-probe rather than
|
||||
# trust the stale global — Stage-Node already ran successfully or
|
||||
# trust the stale global -- Stage-Node already ran successfully or
|
||||
# the bootstrap would've aborted, so npm is reachable.
|
||||
if (-not (Get-Command npm -ErrorAction SilentlyContinue)) {
|
||||
Write-Info "Skipping Node.js dependencies (Node not installed)"
|
||||
|
|
@ -2688,7 +2688,7 @@ function Clear-ElectronBuildCache {
|
|||
# Last-resort Electron mirror after GitHub download fails (#47266).
|
||||
$script:DesktopElectronFallbackMirror = "https://npmmirror.com/mirrors/electron/"
|
||||
|
||||
# Electron package dir — workspace-local nest first, then root hoist.
|
||||
# Electron package dir -- workspace-local nest first, then root hoist.
|
||||
function Get-ElectronDir {
|
||||
param([string]$InstallDir)
|
||||
$desktopLocal = Join-Path $InstallDir 'apps\desktop\node_modules\electron'
|
||||
|
|
@ -2769,7 +2769,7 @@ function Install-Desktop {
|
|||
#
|
||||
# The Tauri bootstrap installer's launch_hermes_desktop command
|
||||
# resolves apps/desktop/release/win-unpacked/Hermes.exe directly,
|
||||
# so an "unpacked" build (electron-builder --dir) is enough — we
|
||||
# so an "unpacked" build (electron-builder --dir) is enough -- we
|
||||
# don't need to produce an NSIS/MSI artifact here.
|
||||
|
||||
# Always re-resolve Node here. Stages run in separate PowerShell processes,
|
||||
|
|
@ -2823,7 +2823,7 @@ function Install-Desktop {
|
|||
#
|
||||
# The streaming sink in bootstrap.rs's run_install_script
|
||||
# captures every stdout/stderr line as it's emitted, so we don't
|
||||
# need a side TEMP log file — the installer's bootstrap log
|
||||
# need a side TEMP log file -- the installer's bootstrap log
|
||||
# IS the artifact a support engineer reads.
|
||||
#
|
||||
# Prefer `npm ci`: it wipes node_modules and reinstalls from the
|
||||
|
|
@ -2878,7 +2878,7 @@ function Install-Desktop {
|
|||
# NOT signing the output. Combined with signAndEditExecutable=false in
|
||||
# apps/desktop/package.json's build.win block, electron-builder never
|
||||
# invokes signtool and therefore never fetches/extracts winCodeSign
|
||||
# (whose macOS symlinks crash 7-Zip on non-admin Windows — a dead end we
|
||||
# (whose macOS symlinks crash 7-Zip on non-admin Windows -- a dead end we
|
||||
# are NOT trying to work around). The Hermes icon + product name are
|
||||
# stamped onto Hermes.exe by our own rcedit step (Set-DesktopExeIdentity)
|
||||
# AFTER this build, completely decoupled from electron-builder signing.
|
||||
|
|
@ -2949,7 +2949,7 @@ function Install-Desktop {
|
|||
Pop-Location
|
||||
throw
|
||||
} finally {
|
||||
# Restore env to whatever the caller had — don't leak our
|
||||
# Restore env to whatever the caller had -- don't leak our
|
||||
# signing-off override into anything install.ps1 invokes later
|
||||
# (Stage-PlatformSdks, etc.).
|
||||
$env:CSC_IDENTITY_AUTO_DISCOVERY = $prevCSCAuto
|
||||
|
|
@ -2980,7 +2980,7 @@ function Install-Desktop {
|
|||
|
||||
# 3b. The Hermes icon + identity are stamped onto Hermes.exe by the
|
||||
# electron-builder `afterPack` hook (apps/desktop/scripts/after-pack.mjs)
|
||||
# during `npm run pack` above — for every build, so the installer's
|
||||
# during `npm run pack` above -- for every build, so the installer's
|
||||
# --update rebuild stays branded too. No separate stamp step needed here.
|
||||
# electron-builder's own rcedit step stays disabled (signAndEditExecutable
|
||||
# =false) because enabling it drags in signtool -> winCodeSign -> the
|
||||
|
|
@ -2990,7 +2990,7 @@ function Install-Desktop {
|
|||
# directory. Chromium's GPU/renderer sandboxes CHECK-fail with
|
||||
# 0x80000003 when this ACE is missing alongside orphan AppContainer
|
||||
# SIDs under %LOCALAPPDATA% (electron/electron#51761, hermes-agent#38216).
|
||||
# Best-effort — never fail an otherwise-good install over ACL repair.
|
||||
# Best-effort -- never fail an otherwise-good install over ACL repair.
|
||||
try {
|
||||
$appDir = Split-Path -Parent $desktopExe
|
||||
& icacls $appDir /grant "*S-1-15-2-2:(OI)(CI)(RX)" /T /C /Q | Out-Null
|
||||
|
|
@ -3006,7 +3006,7 @@ function Install-Desktop {
|
|||
# 4. Create Start Menu + Desktop shortcuts pointing DIRECTLY at the packed
|
||||
# Hermes.exe. We deliberately do NOT point them at `hermes desktop`: that
|
||||
# command rebuilds (npm install + electron-builder) on every launch,
|
||||
# which would cost minutes each time. The packed exe is the consumer —
|
||||
# which would cost minutes each time. The packed exe is the consumer --
|
||||
# launching it directly is instant, and updates flow through the
|
||||
# installer's --update path (which rebuilds once, then relaunches).
|
||||
New-DesktopShortcuts -TargetExe $desktopExe
|
||||
|
|
@ -3062,11 +3062,11 @@ function New-DesktopShortcuts {
|
|||
# cached bitmap. Critical on the --update path: the exe was re-stamped
|
||||
# with the Hermes icon, but without this the shortcut can keep drawing
|
||||
# the old Electron icon until the user manually refreshes / reboots.
|
||||
# Best-effort and silent — never fail the install over a cosmetic cache.
|
||||
# Best-effort and silent -- never fail the install over a cosmetic cache.
|
||||
try {
|
||||
& ie4uinit.exe -show 2>$null
|
||||
} catch {
|
||||
# ie4uinit may be absent/renamed on some SKUs — ignore.
|
||||
# ie4uinit may be absent/renamed on some SKUs -- ignore.
|
||||
}
|
||||
} catch {
|
||||
Write-Warn "Skipping shortcut creation: $($_.Exception.Message)"
|
||||
|
|
|
|||
|
|
@ -607,6 +607,210 @@ class TestSaveEnvValueSecure:
|
|||
assert parsed["ANTHROPIC_TOKEN"] == token
|
||||
assert load_env()["ANTHROPIC_TOKEN"] == token
|
||||
|
||||
def test_save_env_value_quotes_values_with_internal_spaces(self, tmp_path):
|
||||
"""Internal spaces must be quoted so shell-sourcing does not word-split.
|
||||
|
||||
Sibling of installer #57247: core writer left
|
||||
TERMINAL_SSH_KEY=/Users/.../Application Support/... unquoted.
|
||||
python-dotenv still parsed it; ``set -a; . file`` failed.
|
||||
"""
|
||||
import subprocess
|
||||
from dotenv import dotenv_values
|
||||
|
||||
path = "/Users/paulo/Library/Application Support/hermes/keys/id_ed25519"
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("TERMINAL_SSH_KEY", None)
|
||||
save_env_value("TERMINAL_SSH_KEY", path)
|
||||
|
||||
env_path = tmp_path / ".env"
|
||||
content = env_path.read_text(encoding="utf-8")
|
||||
assert f'TERMINAL_SSH_KEY="{path}"' in content
|
||||
|
||||
parsed = dotenv_values(str(env_path))
|
||||
assert parsed["TERMINAL_SSH_KEY"] == path
|
||||
assert load_env()["TERMINAL_SSH_KEY"] == path
|
||||
|
||||
# Shell source must round-trip (this is what the bug broke).
|
||||
r = subprocess.run(
|
||||
[
|
||||
"env",
|
||||
"-i",
|
||||
"sh",
|
||||
"-c",
|
||||
f"set -a; . '{env_path}'; set +a; "
|
||||
f'printf "%s" "$TERMINAL_SSH_KEY"',
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert r.stderr == ""
|
||||
assert r.stdout == path
|
||||
|
||||
def test_save_env_value_quotes_values_with_tabs(self, tmp_path):
|
||||
"""Tabs trigger quoting; round-trip via dotenv and shell source."""
|
||||
import subprocess
|
||||
from dotenv import dotenv_values
|
||||
|
||||
value = "left\tright"
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("TABBY_KEY", None)
|
||||
save_env_value("TABBY_KEY", value)
|
||||
|
||||
env_path = tmp_path / ".env"
|
||||
content = env_path.read_text(encoding="utf-8")
|
||||
assert f'TABBY_KEY="{value}"' in content
|
||||
|
||||
parsed = dotenv_values(str(env_path))
|
||||
assert parsed["TABBY_KEY"] == value
|
||||
assert load_env()["TABBY_KEY"] == value
|
||||
|
||||
r = subprocess.run(
|
||||
[
|
||||
"env",
|
||||
"-i",
|
||||
"sh",
|
||||
"-c",
|
||||
f"set -a; . '{env_path}'; set +a; "
|
||||
f'printf "%s" "$TABBY_KEY"',
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert r.stderr == ""
|
||||
assert r.stdout == value
|
||||
|
||||
def test_save_env_value_spaced_path_is_idempotent(self, tmp_path):
|
||||
"""Saving the same spaced value twice must not grow quotes."""
|
||||
path = "/Users/me/Application Support/key"
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("TERMINAL_SSH_KEY", None)
|
||||
save_env_value("TERMINAL_SSH_KEY", path)
|
||||
first = (tmp_path / ".env").read_text(encoding="utf-8")
|
||||
save_env_value("TERMINAL_SSH_KEY", path)
|
||||
second = (tmp_path / ".env").read_text(encoding="utf-8")
|
||||
|
||||
assert first == second
|
||||
assert first.count('TERMINAL_SSH_KEY="') == 1
|
||||
assert '""' not in first
|
||||
assert f'TERMINAL_SSH_KEY="{path}"' in first
|
||||
|
||||
def test_save_env_value_readback_resave_is_idempotent(self, tmp_path):
|
||||
"""hermes setup path: dotenv unquotes, then re-save must not grow quotes."""
|
||||
from dotenv import dotenv_values
|
||||
|
||||
path = "/Users/me/Application Support/key"
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("TERMINAL_SSH_KEY", None)
|
||||
save_env_value("TERMINAL_SSH_KEY", path)
|
||||
first = (tmp_path / ".env").read_text(encoding="utf-8")
|
||||
|
||||
# Real read-back boundary (what setup uses via get_env_value/dotenv).
|
||||
read_back = dotenv_values(str(tmp_path / ".env"))["TERMINAL_SSH_KEY"]
|
||||
assert read_back == path
|
||||
save_env_value("TERMINAL_SSH_KEY", read_back)
|
||||
second = (tmp_path / ".env").read_text(encoding="utf-8")
|
||||
|
||||
assert first == second
|
||||
assert f'TERMINAL_SSH_KEY="{path}"' in second
|
||||
|
||||
def test_save_env_value_strips_newlines_before_quoting(self, tmp_path):
|
||||
"""save_env_value strips \\n/\\r before _quote_env_value; result is one line.
|
||||
|
||||
Pins the boundary so any(c.isspace()) never quotes multi-line dotenv
|
||||
values through this writer (newlines never reach the quoter).
|
||||
"""
|
||||
from dotenv import dotenv_values
|
||||
|
||||
raw = "line1\nline2\rline3"
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("MULTI_KEY", None)
|
||||
save_env_value("MULTI_KEY", raw)
|
||||
|
||||
content = (tmp_path / ".env").read_text(encoding="utf-8")
|
||||
# Single KEY= line, no embedded raw newlines in the value payload.
|
||||
lines = [ln for ln in content.splitlines() if ln.startswith("MULTI_KEY=")]
|
||||
assert len(lines) == 1
|
||||
assert "\n" not in lines[0]
|
||||
assert "\r" not in lines[0]
|
||||
# Newlines stripped -> "line1line2line3" has no whitespace -> unquoted.
|
||||
assert lines[0] == "MULTI_KEY=line1line2line3"
|
||||
parsed = dotenv_values(str(tmp_path / ".env"))
|
||||
assert parsed["MULTI_KEY"] == "line1line2line3"
|
||||
|
||||
def test_save_env_value_simple_values_stay_unquoted(self, tmp_path):
|
||||
"""No quoting churn: plain values remain bare; untouched lines unchanged."""
|
||||
env_path = tmp_path / ".env"
|
||||
# Pre-existing lines: one simple, one already correctly bare.
|
||||
env_path.write_text(
|
||||
"KEEP_SIMPLE=plainvalue\n"
|
||||
"OTHER_KEY=foo123\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("NEW_KEY", None)
|
||||
os.environ.pop("KEEP_SIMPLE", None)
|
||||
save_env_value("NEW_KEY", "bar-simple")
|
||||
|
||||
content = env_path.read_text(encoding="utf-8")
|
||||
# Newly written simple value is unquoted.
|
||||
assert "NEW_KEY=bar-simple\n" in content
|
||||
assert 'NEW_KEY="' not in content
|
||||
# Untouched pre-existing simple lines are not re-quoted.
|
||||
assert "KEEP_SIMPLE=plainvalue\n" in content
|
||||
assert "OTHER_KEY=foo123\n" in content
|
||||
assert 'KEEP_SIMPLE="' not in content
|
||||
assert 'OTHER_KEY="' not in content
|
||||
|
||||
def test_save_env_value_does_not_requote_untouched_spaced_lines(self, tmp_path):
|
||||
"""Mass-requote guard: rewriting another key leaves legacy spaced
|
||||
lines as-is (fix only applies when that key is saved again).
|
||||
"""
|
||||
env_path = tmp_path / ".env"
|
||||
legacy = (
|
||||
"TERMINAL_SSH_KEY=/Users/me/Application Support/key\n"
|
||||
"PLAIN=ok\n"
|
||||
)
|
||||
env_path.write_text(legacy, encoding="utf-8")
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("PLAIN", None)
|
||||
save_env_value("PLAIN", "ok2")
|
||||
|
||||
content = env_path.read_text(encoding="utf-8")
|
||||
# Legacy spaced line not re-serialized by this write.
|
||||
assert (
|
||||
"TERMINAL_SSH_KEY=/Users/me/Application Support/key\n" in content
|
||||
)
|
||||
assert 'TERMINAL_SSH_KEY="' not in content
|
||||
assert "PLAIN=ok2\n" in content
|
||||
|
||||
def test_save_env_value_already_quoted_input_is_not_double_wrapped_idempotently(
|
||||
self, tmp_path
|
||||
):
|
||||
"""Callers pass raw values; if a value literally contains quote
|
||||
characters, escaping+wrap is the dialect (#57249). Re-saving the
|
||||
same raw value is stable (no quote growth). load_env round-trips.
|
||||
"""
|
||||
# User-typed value that already includes surrounding quotes as data.
|
||||
raw = '"/Users/me/Application Support/key"'
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}, clear=False):
|
||||
os.environ.pop("TERMINAL_SSH_KEY", None)
|
||||
save_env_value("TERMINAL_SSH_KEY", raw)
|
||||
first = (tmp_path / ".env").read_text(encoding="utf-8")
|
||||
save_env_value("TERMINAL_SSH_KEY", raw)
|
||||
second = (tmp_path / ".env").read_text(encoding="utf-8")
|
||||
assert first == second
|
||||
# One outer wrap layer only (escaped inner quotes, not nested wraps).
|
||||
line = [
|
||||
ln for ln in first.splitlines() if ln.startswith("TERMINAL_SSH_KEY=")
|
||||
][0]
|
||||
assert line.startswith('TERMINAL_SSH_KEY="')
|
||||
assert line.endswith('"')
|
||||
assert line.count('TERMINAL_SSH_KEY="') == 1
|
||||
# Escaping dialect end-to-end: load sees the raw input, not stripped quotes.
|
||||
assert load_env()["TERMINAL_SSH_KEY"] == raw
|
||||
|
||||
|
||||
class TestRemoveEnvValue:
|
||||
def test_removes_key_from_env_file(self, tmp_path):
|
||||
|
|
@ -726,7 +930,7 @@ class TestSaveConfigAtomicity:
|
|||
|
||||
# Read raw YAML to verify it's valid and correct
|
||||
config_path = tmp_path / "config.yaml"
|
||||
with open(config_path) as f:
|
||||
with open(config_path, encoding="utf-8") as f:
|
||||
raw = yaml.safe_load(f)
|
||||
assert raw["model"] == "test/atomic-model"
|
||||
assert raw["agent"]["max_turns"] == 77
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import codecs
|
||||
import importlib
|
||||
import os
|
||||
import sys
|
||||
|
|
@ -103,3 +104,268 @@ def test_main_import_applies_user_env_over_shell_values(tmp_path, monkeypatch):
|
|||
|
||||
assert os.getenv("OPENAI_BASE_URL") == "https://new.example/v1"
|
||||
assert os.getenv("HERMES_INFERENCE_PROVIDER") == "custom"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# UTF-16 / UTF-32 .env sanitizer coverage
|
||||
#
|
||||
# Scope note: intentionally NO UTF-8-BOM assertions here. UTF-8 BOM handling
|
||||
# for _load_dotenv_with_fallback is #65124's un-merged fix; a test here would
|
||||
# couple the PRs. This suite covers only the sanitizer rewrite path for
|
||||
# UTF-16/32 (and UTF-8 / cp1252 regression guards for that path).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _assert_clean_utf8_env_on_disk(env_file, *, first_key: str) -> None:
|
||||
"""On-disk file must be clean UTF-8: no BOM, no U+FFFD, canonical key."""
|
||||
after = env_file.read_bytes()
|
||||
assert not after.startswith(codecs.BOM_UTF8)
|
||||
assert not after.startswith(codecs.BOM_UTF16_LE)
|
||||
assert not after.startswith(codecs.BOM_UTF16_BE)
|
||||
text = after.decode("utf-8") # strict — raises if not clean UTF-8
|
||||
assert "\ufffd" not in text
|
||||
assert text.startswith(f"{first_key}=") or f"\n{first_key}=" in text
|
||||
assert first_key.encode("ascii") in after
|
||||
|
||||
|
||||
def test_utf16_le_bom_env_loads_and_rewrites_clean_utf8(tmp_path, monkeypatch):
|
||||
"""Notepad 'Unicode' (UTF-16-LE + BOM): first key loads; file rewritten UTF-8."""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
content = "HERMES_TEST_KEY=hello_utf16\nSECOND_KEY=world\n"
|
||||
env_file.write_bytes(codecs.BOM_UTF16_LE + content.encode("utf-16-le"))
|
||||
|
||||
monkeypatch.delenv("HERMES_TEST_KEY", raising=False)
|
||||
monkeypatch.delenv("SECOND_KEY", raising=False)
|
||||
monkeypatch.delenv("\ufffd\ufffdHERMES_TEST_KEY", raising=False)
|
||||
|
||||
loaded = load_hermes_dotenv(hermes_home=home)
|
||||
|
||||
assert loaded == [env_file]
|
||||
assert os.getenv("HERMES_TEST_KEY") == "hello_utf16"
|
||||
assert os.getenv("SECOND_KEY") == "world"
|
||||
assert os.environ.get("\ufffd\ufffdHERMES_TEST_KEY") is None
|
||||
_assert_clean_utf8_env_on_disk(env_file, first_key="HERMES_TEST_KEY")
|
||||
|
||||
|
||||
def test_utf16_be_bom_env_loads_and_rewrites_clean_utf8(tmp_path, monkeypatch):
|
||||
"""UTF-16-BE + BOM: first key loads; file rewritten as clean UTF-8."""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
content = "HERMES_TEST_KEY=hello_utf16\nSECOND_KEY=world\n"
|
||||
env_file.write_bytes(codecs.BOM_UTF16_BE + content.encode("utf-16-be"))
|
||||
|
||||
monkeypatch.delenv("HERMES_TEST_KEY", raising=False)
|
||||
monkeypatch.delenv("SECOND_KEY", raising=False)
|
||||
|
||||
loaded = load_hermes_dotenv(hermes_home=home)
|
||||
|
||||
assert loaded == [env_file]
|
||||
assert os.getenv("HERMES_TEST_KEY") == "hello_utf16"
|
||||
assert os.getenv("SECOND_KEY") == "world"
|
||||
_assert_clean_utf8_env_on_disk(env_file, first_key="HERMES_TEST_KEY")
|
||||
|
||||
|
||||
def test_utf16_le_no_bom_still_repairs_to_utf8(tmp_path, monkeypatch):
|
||||
"""BOM-less UTF-16-LE: NUL-strip repair is now intentional; rewrites UTF-8."""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
content = "HERMES_TEST_KEY=hello_utf16\nSECOND_KEY=world\n"
|
||||
env_file.write_bytes(content.encode("utf-16-le")) # no BOM
|
||||
|
||||
monkeypatch.delenv("HERMES_TEST_KEY", raising=False)
|
||||
monkeypatch.delenv("SECOND_KEY", raising=False)
|
||||
|
||||
loaded = load_hermes_dotenv(hermes_home=home)
|
||||
|
||||
assert loaded == [env_file]
|
||||
assert os.getenv("HERMES_TEST_KEY") == "hello_utf16"
|
||||
assert os.getenv("SECOND_KEY") == "world"
|
||||
_assert_clean_utf8_env_on_disk(env_file, first_key="HERMES_TEST_KEY")
|
||||
|
||||
|
||||
def test_utf16_be_no_bom_still_repairs_to_utf8(tmp_path, monkeypatch):
|
||||
"""BOM-less UTF-16-BE: NULs are on the opposite side; still repairs."""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
content = "HERMES_TEST_KEY=hello_utf16\nSECOND_KEY=world\n"
|
||||
env_file.write_bytes(content.encode("utf-16-be")) # no BOM
|
||||
|
||||
monkeypatch.delenv("HERMES_TEST_KEY", raising=False)
|
||||
monkeypatch.delenv("SECOND_KEY", raising=False)
|
||||
|
||||
loaded = load_hermes_dotenv(hermes_home=home)
|
||||
|
||||
assert loaded == [env_file]
|
||||
assert os.getenv("HERMES_TEST_KEY") == "hello_utf16"
|
||||
assert os.getenv("SECOND_KEY") == "world"
|
||||
_assert_clean_utf8_env_on_disk(env_file, first_key="HERMES_TEST_KEY")
|
||||
|
||||
|
||||
def test_utf16_le_bom_preserves_non_ascii_values(tmp_path, monkeypatch):
|
||||
"""UTF-16-LE+BOM rewrite must preserve non-ASCII values (not just ASCII keys).
|
||||
|
||||
Uses non-credential var names so _sanitize_loaded_credentials does not
|
||||
strip non-ASCII from values (that path only targets *_KEY/*_TOKEN/etc.).
|
||||
"""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
content = "GREETING=café\nCJK_LABEL=日本語\n"
|
||||
env_file.write_bytes(codecs.BOM_UTF16_LE + content.encode("utf-16-le"))
|
||||
|
||||
monkeypatch.delenv("GREETING", raising=False)
|
||||
monkeypatch.delenv("CJK_LABEL", raising=False)
|
||||
|
||||
loaded = load_hermes_dotenv(hermes_home=home)
|
||||
|
||||
assert loaded == [env_file]
|
||||
assert os.getenv("GREETING") == "café"
|
||||
assert os.getenv("CJK_LABEL") == "日本語"
|
||||
after = env_file.read_bytes()
|
||||
assert after.decode("utf-8") # strict
|
||||
assert "café".encode("utf-8") in after
|
||||
assert "日本語".encode("utf-8") in after
|
||||
assert b"\xef\xbf\xbd" not in after
|
||||
|
||||
|
||||
def test_utf32_le_bom_leaves_file_untouched(tmp_path, caplog):
|
||||
"""UTF-32-LE BOM: refuse-to-mangle (leave bytes untouched + warning).
|
||||
|
||||
UTF-32-LE's BOM starts with UTF-16-LE's FF FE; sniff order must check
|
||||
UTF-32 first so we never misdetect and corrupt.
|
||||
|
||||
Exercises ``_sanitize_env_file_if_needed`` only: the dotenv load path
|
||||
is out of scope here (#65124's surface) and still cannot ingest UTF-32.
|
||||
"""
|
||||
import logging
|
||||
|
||||
from hermes_cli.env_loader import _sanitize_env_file_if_needed
|
||||
|
||||
env_file = tmp_path / ".env"
|
||||
content = "HERMES_TEST_KEY=hello_utf32\nSECOND_KEY=world\n"
|
||||
raw = codecs.BOM_UTF32_LE + content.encode("utf-32-le")
|
||||
env_file.write_bytes(raw)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="hermes_cli.env_loader"):
|
||||
_sanitize_env_file_if_needed(env_file)
|
||||
|
||||
assert env_file.read_bytes() == raw # untouched
|
||||
assert any("UTF-32" in r.message for r in caplog.records)
|
||||
|
||||
|
||||
def test_utf32_be_bom_leaves_file_untouched(tmp_path, caplog):
|
||||
"""UTF-32-BE BOM: same refuse-to-mangle path as LE (ordering independence)."""
|
||||
import logging
|
||||
|
||||
from hermes_cli.env_loader import _sanitize_env_file_if_needed
|
||||
|
||||
env_file = tmp_path / ".env"
|
||||
content = "HERMES_TEST_KEY=hello_utf32\nSECOND_KEY=world\n"
|
||||
raw = codecs.BOM_UTF32_BE + content.encode("utf-32-be")
|
||||
env_file.write_bytes(raw)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="hermes_cli.env_loader"):
|
||||
_sanitize_env_file_if_needed(env_file)
|
||||
|
||||
assert env_file.read_bytes() == raw
|
||||
assert any("UTF-32" in r.message for r in caplog.records)
|
||||
|
||||
|
||||
def test_utf32_warning_fires_once_per_path(tmp_path, caplog, monkeypatch):
|
||||
"""Three sanitize calls on the same UTF-32 file → exactly one warning.
|
||||
|
||||
Matches house style for warn-once (module-level seen-set, same class as
|
||||
``_WARNED_KEYS``): hot-reload / multi-entry load must not spam logs.
|
||||
"""
|
||||
import logging
|
||||
|
||||
import hermes_cli.env_loader as env_loader
|
||||
from hermes_cli.env_loader import _sanitize_env_file_if_needed
|
||||
|
||||
# Isolate process-level seen-set so other tests' paths don't leak in.
|
||||
monkeypatch.setattr(env_loader, "_WARNED_UTF32_PATHS", set())
|
||||
|
||||
env_file = tmp_path / ".env"
|
||||
content = "HERMES_TEST_KEY=hello_utf32\nSECOND_KEY=world\n"
|
||||
raw = codecs.BOM_UTF32_LE + content.encode("utf-32-le")
|
||||
env_file.write_bytes(raw)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="hermes_cli.env_loader"):
|
||||
_sanitize_env_file_if_needed(env_file)
|
||||
_sanitize_env_file_if_needed(env_file)
|
||||
_sanitize_env_file_if_needed(env_file)
|
||||
|
||||
utf32_warnings = [r for r in caplog.records if "UTF-32" in r.message]
|
||||
assert len(utf32_warnings) == 1
|
||||
assert env_file.read_bytes() == raw
|
||||
|
||||
|
||||
def test_leading_replacement_char_does_not_rewrite(tmp_path):
|
||||
"""errors=replace FFFD-on-first-line guard: do not persist mangling.
|
||||
|
||||
Leading 0xFF is not a UTF-16/32 BOM (those need the second BOM byte) but
|
||||
is undecodable as UTF-8, so the replace path would glue U+FFFD onto the
|
||||
key. The guard must leave the on-disk bytes untouched.
|
||||
"""
|
||||
from hermes_cli.env_loader import _sanitize_env_file_if_needed
|
||||
|
||||
env_file = tmp_path / ".env"
|
||||
raw = b"\xffHERMES_TEST_KEY=should-not-rewrite\nSECOND_KEY=ok\n"
|
||||
env_file.write_bytes(raw)
|
||||
|
||||
_sanitize_env_file_if_needed(env_file)
|
||||
|
||||
assert env_file.read_bytes() == raw
|
||||
|
||||
|
||||
def test_plain_utf8_env_regression(tmp_path, monkeypatch):
|
||||
"""Plain UTF-8 .env must keep loading after the UTF-16 sanitize changes."""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
before = b"OPENAI_API_KEY=sk-plain\nSECOND_KEY=ok\n"
|
||||
env_file.write_bytes(before)
|
||||
|
||||
monkeypatch.delenv("OPENAI_API_KEY", raising=False)
|
||||
monkeypatch.delenv("SECOND_KEY", raising=False)
|
||||
|
||||
loaded = load_hermes_dotenv(hermes_home=home)
|
||||
|
||||
assert loaded == [env_file]
|
||||
assert os.getenv("OPENAI_API_KEY") == "sk-plain"
|
||||
assert os.getenv("SECOND_KEY") == "ok"
|
||||
# No spurious rewrite of an already-clean file.
|
||||
assert env_file.read_bytes() == before
|
||||
|
||||
|
||||
def test_cp1252_env_regression_does_not_crash(tmp_path, monkeypatch):
|
||||
"""cp1252/latin-1 body must not crash sanitize; ASCII keys still usable.
|
||||
|
||||
0xE9 is 'é' in cp1252 and incomplete as UTF-8. First line does not begin
|
||||
with U+FFFD, so the FFFD guard must not refuse the whole file.
|
||||
|
||||
Sanitize leaves the file bytes alone when the only "change" is
|
||||
errors=replace on values (original already replace-decoded equals
|
||||
sanitized), so _load_dotenv_with_fallback's latin-1 path recovers café.
|
||||
"""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
before = b"ASCII_KEY=ok\nLATIN1_VALUE=caf\xe9\n"
|
||||
env_file.write_bytes(before)
|
||||
|
||||
monkeypatch.delenv("ASCII_KEY", raising=False)
|
||||
monkeypatch.delenv("LATIN1_VALUE", raising=False)
|
||||
|
||||
loaded = load_hermes_dotenv(hermes_home=home)
|
||||
|
||||
assert loaded == [env_file]
|
||||
assert os.getenv("ASCII_KEY") == "ok"
|
||||
assert os.getenv("LATIN1_VALUE") == "café"
|
||||
# Sanitize must not have rewritten (would have persisted U+FFFD).
|
||||
assert env_file.read_bytes() == before
|
||||
|
|
|
|||
55
tests/test_install_ps1_ascii_only.py
Normal file
55
tests/test_install_ps1_ascii_only.py
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
"""Regression: install.ps1 must stay pure ASCII.
|
||||
|
||||
Issues #66994 / #67000 reported the Windows GUI installer (Hermes-Setup.exe)
|
||||
crashing before it did anything, with a cascade of PowerShell parser errors at
|
||||
lines 1619 / 1770 ("Missing argument in parameter list", "A 'using' statement
|
||||
must appear before any other statements in a script").
|
||||
|
||||
Root cause: ``scripts/install.ps1`` has no UTF-8 BOM, and a commit added two
|
||||
non-ASCII characters *inside double-quoted string literals* (a bullet and an
|
||||
em-dash). Windows PowerShell 5.1 -- which the bootstrap runs the cached script
|
||||
under -- reads a BOM-less ``.ps1`` in the system ANSI code page (e.g. CP1252),
|
||||
not UTF-8. The em-dash's UTF-8 tail byte (0x94) decodes to a "smart" close-quote
|
||||
(U+201D), which the PowerShell tokenizer treats as a string delimiter. That
|
||||
prematurely closes the string and desyncs the parser for the rest of the file,
|
||||
surfacing as unrelated syntax errors far downstream.
|
||||
|
||||
Non-ASCII bytes inside ``#`` comments are harmless (the tokenizer skips a
|
||||
comment to end-of-line regardless of what it contains), which is why the file
|
||||
carried em-dashes in comments for months without breaking -- only a non-ASCII
|
||||
byte in *code* (a string literal) triggers the desync.
|
||||
|
||||
This test is source-level because Linux CI cannot execute the PowerShell
|
||||
installer. Keeping the whole file ASCII-only is the transport-independent
|
||||
invariant: pure ASCII cannot be misdecoded under any code page, BOM or not, so
|
||||
the bug class cannot recur -- in a comment or a string.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
INSTALL_PS1 = REPO_ROOT / "scripts" / "install.ps1"
|
||||
|
||||
|
||||
def test_install_ps1_is_pure_ascii() -> None:
|
||||
raw = INSTALL_PS1.read_bytes()
|
||||
|
||||
offenders = []
|
||||
line_no = 1
|
||||
for byte in raw:
|
||||
if byte == 0x0A:
|
||||
line_no += 1
|
||||
elif byte >= 0x80:
|
||||
offenders.append(line_no)
|
||||
|
||||
assert not offenders, (
|
||||
"scripts/install.ps1 must be pure ASCII so Windows PowerShell 5.1 "
|
||||
"(which reads a BOM-less .ps1 in the system ANSI code page, not UTF-8) "
|
||||
"cannot misdecode a byte into a stray quote and desync the parser "
|
||||
"(issues #66994 / #67000). Non-ASCII bytes found on line(s): "
|
||||
f"{sorted(set(offenders))}. Use ASCII equivalents (em-dash -> '--', "
|
||||
"bullet -> '-')."
|
||||
)
|
||||
68
tests/tools/test_mcp_poll_loop_oom_integration.py
Normal file
68
tests/tools/test_mcp_poll_loop_oom_integration.py
Normal file
|
|
@ -0,0 +1,68 @@
|
|||
"""End-to-end coverage for the MCP poll-loop OOM spin (#63892).
|
||||
|
||||
The unit tests in ``test_mcp_tool.py`` hand-construct completed futures to lock
|
||||
the ``_run_on_mcp_loop`` contract deterministically. This complements them by
|
||||
exercising the actual field trigger through the *live* MCP loop: an inner
|
||||
``asyncio.wait_for`` expiry produces a real ``TimeoutError`` stored on a real
|
||||
future scheduled via ``run_coroutine_threadsafe``. On Python >= 3.8 that class
|
||||
is the builtin ``TimeoutError``, so before the fix the poll loop swallowed the
|
||||
completed future's stored exception and spun with no sleep -- burning CPU,
|
||||
growing the exception traceback ~108 MB/s until the gateway OOM'd, and finally
|
||||
masking the real error behind the generic "MCP call timed out after <full
|
||||
timeout>" wrapper.
|
||||
|
||||
The fixed loop must surface the real exception once, promptly -- long before
|
||||
the outer deadline.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mcp_loop():
|
||||
import tools.mcp_tool as mcp_tool
|
||||
|
||||
mcp_tool._ensure_mcp_loop()
|
||||
yield mcp_tool
|
||||
mcp_tool._stop_mcp_loop()
|
||||
|
||||
|
||||
def test_inner_wait_for_timeout_surfaces_promptly_without_spinning(mcp_loop):
|
||||
async def inner():
|
||||
# Real TimeoutError from wait_for, completing far before the outer
|
||||
# deadline below -- the exact shape of an MCP call_tool wrapped in the
|
||||
# server's configured mcp_servers.<srv>.timeout.
|
||||
await asyncio.wait_for(asyncio.sleep(60), timeout=0.05)
|
||||
|
||||
start = time.monotonic()
|
||||
with pytest.raises(TimeoutError) as exc:
|
||||
mcp_loop._run_on_mcp_loop(inner, timeout=10)
|
||||
elapsed = time.monotonic() - start
|
||||
|
||||
# Fixed: the real inner TimeoutError surfaces within a couple poll ticks.
|
||||
# Broken: the loop swallows it and spins with no sleep until the 10s
|
||||
# deadline, then raises the generic wrapper message instead. A generous
|
||||
# (>= 2s) bound keeps this stable on a slow CI runner while still failing
|
||||
# hard on a spin-to-deadline regression.
|
||||
assert elapsed < 5.0, (
|
||||
f"poll loop spun for {elapsed:.1f}s instead of resolving the completed "
|
||||
"future once (#63892 regression)"
|
||||
)
|
||||
assert "MCP call timed out after" not in str(exc.value), (
|
||||
"the real inner TimeoutError must surface, not the outer poll deadline"
|
||||
)
|
||||
|
||||
|
||||
def test_successful_call_still_returns_through_real_loop(mcp_loop):
|
||||
"""Guard the done-future path against regressing normal success returns."""
|
||||
|
||||
async def inner():
|
||||
await asyncio.sleep(0.25) # first polls time out while still pending
|
||||
return {"ok": True}
|
||||
|
||||
assert mcp_loop._run_on_mcp_loop(inner, timeout=10) == {"ok": True}
|
||||
|
|
@ -4,6 +4,7 @@ All tests use mocks -- no real MCP servers or subprocesses are started.
|
|||
"""
|
||||
|
||||
import asyncio
|
||||
import concurrent.futures
|
||||
import json
|
||||
import threading
|
||||
import time
|
||||
|
|
@ -837,6 +838,23 @@ class TestToolHandler:
|
|||
|
||||
|
||||
class TestRunOnMCPLoopInterrupts:
|
||||
@staticmethod
|
||||
def _run_with_future(mcp_mod, future):
|
||||
loop = MagicMock()
|
||||
loop.is_running.return_value = True
|
||||
|
||||
async def _unused_call():
|
||||
return "unused"
|
||||
|
||||
def _schedule(coro, scheduled_loop, **_kwargs):
|
||||
assert scheduled_loop is loop
|
||||
coro.close()
|
||||
return future
|
||||
|
||||
with patch.object(mcp_mod, "_mcp_loop", loop):
|
||||
with patch("agent.async_utils.safe_schedule_threadsafe", side_effect=_schedule):
|
||||
return mcp_mod._run_on_mcp_loop(_unused_call(), timeout=1)
|
||||
|
||||
def test_interrupt_cancels_waiting_mcp_call(self):
|
||||
import tools.mcp_tool as mcp_mod
|
||||
from tools.interrupt import set_interrupt
|
||||
|
|
@ -922,6 +940,54 @@ class TestRunOnMCPLoopInterrupts:
|
|||
mcp_mod._mcp_loop = old_loop
|
||||
mcp_mod._mcp_thread = old_thread
|
||||
|
||||
def test_completed_future_timeout_is_propagated_once(self):
|
||||
import tools.mcp_tool as mcp_mod
|
||||
|
||||
inner_error = TimeoutError("inner MCP timeout")
|
||||
|
||||
class CompletedWithTimeout(concurrent.futures.Future):
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.result_timeouts = []
|
||||
self.set_exception(inner_error)
|
||||
|
||||
def result(self, timeout=None):
|
||||
self.result_timeouts.append(timeout)
|
||||
return super().result(timeout=timeout)
|
||||
|
||||
future = CompletedWithTimeout()
|
||||
|
||||
with pytest.raises(TimeoutError, match="inner MCP timeout") as exc_info:
|
||||
self._run_with_future(mcp_mod, future)
|
||||
|
||||
assert exc_info.value is inner_error
|
||||
assert len(future.result_timeouts) == 2
|
||||
assert future.result_timeouts[0] is not None
|
||||
assert future.result_timeouts[1] is None
|
||||
|
||||
def test_poll_timeout_racing_success_returns_completed_result(self):
|
||||
import tools.mcp_tool as mcp_mod
|
||||
|
||||
class PollThenSuccess(concurrent.futures.Future):
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.result_timeouts = []
|
||||
|
||||
def result(self, timeout=None):
|
||||
self.result_timeouts.append(timeout)
|
||||
if len(self.result_timeouts) == 1:
|
||||
self.set_result("completed")
|
||||
raise concurrent.futures.TimeoutError
|
||||
|
||||
return super().result(timeout=timeout)
|
||||
|
||||
future = PollThenSuccess()
|
||||
|
||||
assert self._run_with_future(mcp_mod, future) == "completed"
|
||||
assert len(future.result_timeouts) == 2
|
||||
assert future.result_timeouts[0] is not None
|
||||
assert future.result_timeouts[1] is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tool registration (discovery + register)
|
||||
|
|
|
|||
|
|
@ -3909,6 +3909,13 @@ def _run_on_mcp_loop(coro_or_factory, timeout: float = 30):
|
|||
try:
|
||||
return future.result(timeout=wait_timeout)
|
||||
except concurrent.futures.TimeoutError:
|
||||
# On supported Python versions, concurrent.futures.TimeoutError
|
||||
# aliases the built-in TimeoutError, so result(timeout=...) also
|
||||
# raises it for a coroutine's own timeout.
|
||||
# Resolve a done future without a timeout to propagate its stored
|
||||
# outcome, including completion racing with this polling timeout.
|
||||
if future.done():
|
||||
return future.result()
|
||||
continue
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue