fix: address Hermes runtime PR review feedback
This commit is contained in:
7
.changeset/proud-horses-chat.md
Normal file
7
.changeset/proud-horses-chat.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Preserve Hermes chat session state and project runtime routing more reliably.
|
||||||
|
category: fix
|
||||||
|
dev: Refreshes cached project chat plugin runners and hardens Hermes CLI session/error handling.
|
||||||
@@ -90,10 +90,16 @@ export function getOrCreateScopedChatManager(
|
|||||||
store: TaskStore,
|
store: TaskStore,
|
||||||
chatStore: ChatStore,
|
chatStore: ChatStore,
|
||||||
pluginRunner?: ConstructorParameters<typeof ChatManager>[3],
|
pluginRunner?: ConstructorParameters<typeof ChatManager>[3],
|
||||||
|
refreshPluginRunner = false,
|
||||||
): ChatManager {
|
): ChatManager {
|
||||||
const key = store.getFusionDir();
|
const key = store.getFusionDir();
|
||||||
const cached = scopedChatManagerCache.get(key);
|
const cached = scopedChatManagerCache.get(key);
|
||||||
if (cached) return cached;
|
if (cached) {
|
||||||
|
if (refreshPluginRunner && pluginRunner) {
|
||||||
|
cached.setPluginRunner(pluginRunner);
|
||||||
|
}
|
||||||
|
return cached;
|
||||||
|
}
|
||||||
const agentStore = new AgentStore({ rootDir: store.getFusionDir() });
|
const agentStore = new AgentStore({ rootDir: store.getFusionDir() });
|
||||||
const manager = new ChatManager(
|
const manager = new ChatManager(
|
||||||
chatStore,
|
chatStore,
|
||||||
|
|||||||
@@ -1075,6 +1075,14 @@ export class ChatManager {
|
|||||||
private taskStore?: TaskStore,
|
private taskStore?: TaskStore,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* FNXC:ProjectChatRuntime 2026-07-05-18:10:
|
||||||
|
* Project chat managers can be created before a project engine finishes booting. Refreshing the plugin runner after construction prevents early requests from permanently binding Hermes/runtime hints to the global fallback runner; callers must only refresh from a confirmed project runner so transient engine unavailability cannot downgrade a scoped manager.
|
||||||
|
*/
|
||||||
|
setPluginRunner(pluginRunner: ChatManager["pluginRunner"] | undefined): void {
|
||||||
|
this.pluginRunner = pluginRunner;
|
||||||
|
}
|
||||||
|
|
||||||
private getPluginRunnerForSkillSelection(): Parameters<typeof buildSessionSkillContextSync>[3] {
|
private getPluginRunnerForSkillSelection(): Parameters<typeof buildSessionSkillContextSync>[3] {
|
||||||
return this.pluginRunner?.getPluginSkills
|
return this.pluginRunner?.getPluginSkills
|
||||||
? (this.pluginRunner as unknown as Parameters<typeof buildSessionSkillContextSync>[3])
|
? (this.pluginRunner as unknown as Parameters<typeof buildSessionSkillContextSync>[3])
|
||||||
|
|||||||
@@ -116,8 +116,9 @@ export function registerChatRoutes(ctx: ApiRoutesContext, deps: ChatRouteDeps):
|
|||||||
const projectStore = await getOrCreateProjectStore(projectId);
|
const projectStore = await getOrCreateProjectStore(projectId);
|
||||||
const chatStore = getOrCreateScopedChatStore(projectStore);
|
const chatStore = getOrCreateScopedChatStore(projectStore);
|
||||||
const engine = options?.engineManager?.getEngine(projectId);
|
const engine = options?.engineManager?.getEngine(projectId);
|
||||||
const pluginRunner = engine?.getPluginRunner?.() ?? options?.pluginRunner;
|
const projectPluginRunner = engine?.getPluginRunner?.();
|
||||||
return getOrCreateScopedChatManager(projectStore, chatStore, pluginRunner);
|
const pluginRunner = projectPluginRunner ?? options?.pluginRunner;
|
||||||
|
return getOrCreateScopedChatManager(projectStore, chatStore, pluginRunner, Boolean(projectPluginRunner));
|
||||||
}
|
}
|
||||||
function validateModelPair(modelProvider: unknown, modelId: unknown): { modelProvider?: string; modelId?: string } {
|
function validateModelPair(modelProvider: unknown, modelId: unknown): { modelProvider?: string; modelId?: string } {
|
||||||
let normalizedProvider: string | undefined;
|
let normalizedProvider: string | undefined;
|
||||||
|
|||||||
@@ -230,6 +230,12 @@ describe("parseHermesOutput", () => {
|
|||||||
expect(result.body).toBe("OK");
|
expect(result.body).toBe("OK");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("strips a leading stdout session_id marker from an empty assistant body", () => {
|
||||||
|
const result = parseHermesOutput("session_id: 20260427_120000_abcd12\n", "");
|
||||||
|
expect(result.sessionId).toBe("20260427_120000_abcd12");
|
||||||
|
expect(result.body).toBe("");
|
||||||
|
});
|
||||||
|
|
||||||
it("throws when session_id line is missing", () => {
|
it("throws when session_id line is missing", () => {
|
||||||
expect(() => parseHermesOutput("some output without id", "stderr text")).toThrow(
|
expect(() => parseHermesOutput("some output without id", "stderr text")).toThrow(
|
||||||
/missing session_id/,
|
/missing session_id/,
|
||||||
|
|||||||
@@ -68,6 +68,12 @@ describe("HermesRuntimeAdapter — promptWithFallback", () => {
|
|||||||
expect(onText).toHaveBeenCalledWith("hello from hermes");
|
expect(onText).toHaveBeenCalledWith("hello from hermes");
|
||||||
// Session id is captured for next call
|
// Session id is captured for next call
|
||||||
expect(session.sessionId).toBe("20260427_120000_abc123");
|
expect(session.sessionId).toBe("20260427_120000_abc123");
|
||||||
|
expect(session.messages).toEqual([
|
||||||
|
{ role: "user", content: "first prompt" },
|
||||||
|
{ role: "assistant", content: "hello from hermes" },
|
||||||
|
]);
|
||||||
|
expect(session.state.messages).toBe(session.messages);
|
||||||
|
expect(session.state.errorMessage).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("passes captured session id as --resume on subsequent calls", async () => {
|
it("passes captured session id as --resume on subsequent calls", async () => {
|
||||||
@@ -94,6 +100,8 @@ describe("HermesRuntimeAdapter — promptWithFallback", () => {
|
|||||||
await expect(adapter.promptWithFallback(session, "p")).rejects.toThrow(
|
await expect(adapter.promptWithFallback(session, "p")).rejects.toThrow(
|
||||||
/missing session_id/,
|
/missing session_id/,
|
||||||
);
|
);
|
||||||
|
expect(session.messages).toEqual([]);
|
||||||
|
expect(session.state.errorMessage).toBe("hermes: missing session_id");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does NOT call onText when body is empty", async () => {
|
it("does NOT call onText when body is empty", async () => {
|
||||||
@@ -110,6 +118,11 @@ describe("HermesRuntimeAdapter — promptWithFallback", () => {
|
|||||||
});
|
});
|
||||||
await adapter.promptWithFallback(session, "p");
|
await adapter.promptWithFallback(session, "p");
|
||||||
expect(onText).not.toHaveBeenCalled();
|
expect(onText).not.toHaveBeenCalled();
|
||||||
|
expect(session.messages).toEqual([
|
||||||
|
{ role: "user", content: "p" },
|
||||||
|
{ role: "assistant", content: "" },
|
||||||
|
]);
|
||||||
|
expect(session.state.errorMessage).toBeUndefined();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -311,7 +311,8 @@ function stripChrome(text: string): string {
|
|||||||
export function parseHermesOutput(rawStdout: string, rawStderr: string): HermesCliResult {
|
export function parseHermesOutput(rawStdout: string, rawStderr: string): HermesCliResult {
|
||||||
const cleanedStdout = cleanText(rawStdout);
|
const cleanedStdout = cleanText(rawStdout);
|
||||||
const cleanedCombined = cleanText([rawStdout, rawStderr].filter(Boolean).join("\n"));
|
const cleanedCombined = cleanText([rawStdout, rawStderr].filter(Boolean).join("\n"));
|
||||||
const match = SESSION_ID_RE.exec(cleanedStdout) ?? SESSION_ID_RE.exec(cleanedCombined);
|
const stdoutMatch = SESSION_ID_RE.exec(cleanedStdout);
|
||||||
|
const match = stdoutMatch ?? SESSION_ID_RE.exec(cleanedCombined);
|
||||||
|
|
||||||
if (!match) {
|
if (!match) {
|
||||||
throw new Error(`hermes: missing session_id in output.\n${cleanedCombined}`);
|
throw new Error(`hermes: missing session_id in output.\n${cleanedCombined}`);
|
||||||
@@ -320,8 +321,7 @@ export function parseHermesOutput(rawStdout: string, rawStderr: string): HermesC
|
|||||||
const sessionId = match[1]!;
|
const sessionId = match[1]!;
|
||||||
// Body is stdout before the session_id line. Recent Hermes builds can emit
|
// Body is stdout before the session_id line. Recent Hermes builds can emit
|
||||||
// the session_id marker on stderr, so do not treat stderr as assistant text.
|
// the session_id marker on stderr, so do not treat stderr as assistant text.
|
||||||
const sessionIdLineStart = cleanedStdout.lastIndexOf("\nsession_id:");
|
const bodyRaw = stdoutMatch ? cleanedStdout.slice(0, stdoutMatch.index) : cleanedStdout;
|
||||||
const bodyRaw = sessionIdLineStart >= 0 ? cleanedStdout.slice(0, sessionIdLineStart) : cleanedStdout;
|
|
||||||
const body = stripChrome(bodyRaw);
|
const body = stripChrome(bodyRaw);
|
||||||
|
|
||||||
return { body, sessionId };
|
return { body, sessionId };
|
||||||
|
|||||||
@@ -84,14 +84,23 @@ export class HermesRuntimeAdapter implements AgentRuntime {
|
|||||||
const promptWithContext = resumeId
|
const promptWithContext = resumeId
|
||||||
? prompt
|
? prompt
|
||||||
: `${session.fusedSystemPrompt}\n\nUser request:\n${prompt}`;
|
: `${session.fusedSystemPrompt}\n\nUser request:\n${prompt}`;
|
||||||
session.messages.push({ role: "user", content: prompt });
|
const userMessage = { role: "user", content: prompt };
|
||||||
const result = await invokeHermesCli(promptWithContext, this.settings, resumeId);
|
session.messages.push(userMessage);
|
||||||
|
let result: Awaited<ReturnType<typeof invokeHermesCli>>;
|
||||||
|
try {
|
||||||
|
result = await invokeHermesCli(promptWithContext, this.settings, resumeId);
|
||||||
|
session.state.errorMessage = undefined;
|
||||||
|
} catch (err) {
|
||||||
|
session.messages.pop();
|
||||||
|
session.state.errorMessage = err instanceof Error ? err.message : String(err);
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
|
||||||
session.sessionId = result.sessionId;
|
session.sessionId = result.sessionId;
|
||||||
session.lastModelDescription = this.describeFromSettings();
|
session.lastModelDescription = this.describeFromSettings();
|
||||||
|
|
||||||
|
session.messages.push({ role: "assistant", content: result.body });
|
||||||
if (result.body) {
|
if (result.body) {
|
||||||
session.messages.push({ role: "assistant", content: result.body });
|
|
||||||
session.callbacks.onText?.(result.body);
|
session.callbacks.onText?.(result.body);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user