-
Notifications
You must be signed in to change notification settings - Fork 404
Improve chat queue management and context meter freshness #2495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
489b62f
b333e0d
481bbc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@agent-native/core": patch | ||
| --- | ||
|
|
||
| The agent now hands over files it creates instead of describing where to find them. A new `offer-download` action resolves a workspace file to an access-scoped download URL and renders it as a compact download card in chat, the resources route supports `?download` for a real save-to-disk response, and a framework rule tells the agent to hand over artifacts rather than give app-navigation directions. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@agent-native/core": patch | ||
| "@agent-native/toolkit": patch | ||
| --- | ||
|
|
||
| Queued chat messages are now manageable: each shows a Queued chip and its position, and can be edited in place, reordered, removed, or sent immediately (interrupting the active run). A turn that ended in an error keeps a compact error marker in the transcript instead of losing it as soon as the next message is sent, and the model picker states when a family is listed cheapest first and notes an in-session model switch in the transcript. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@agent-native/core": patch | ||
| --- | ||
|
|
||
| Chat transcript now reports work accurately. Each work group in a turn shows its own duration instead of every group repeating the whole-turn time, a `tool_done` that matches no in-flight card settles that card rather than being dropped (which left tools spinning "Still working" for the rest of the turn), and a tool that never reported back renders as an unknown outcome instead of a spinner or a clean success. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- | ||
| "@agent-native/core": patch | ||
| "@agent-native/toolkit": patch | ||
| --- | ||
|
|
||
| Context meter now reports its own freshness. `context-manifest-get` returns the | ||
| thread's newest turn id, `writeContextManifest` returns a typed persist outcome | ||
| instead of a swallowed rejection, and the meter dims with an explanation when | ||
| the manifest trails the running turn or shows an em dash when no usable reading | ||
| exists. The meter also refetches while a run is streaming, which is what kept it | ||
| frozen on the first turn's percentage. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,9 @@ import { | |
| type ContextManifest, | ||
| } from "../../../shared/context-xray.js"; | ||
| import { callerOwnsThread } from "../../run-ownership.js"; | ||
| import { getRunByThread } from "../../run-store.js"; | ||
| import { readContextManifest } from "../directives-store.js"; | ||
| import { getContextManifestWriteOutcome } from "../manifest.js"; | ||
| import { | ||
| contextXrayAuthError, | ||
| contextXrayThreadNotFoundError, | ||
|
|
@@ -62,14 +64,29 @@ export default defineAction({ | |
| if (!ownerEmail) throw contextXrayAuthError(); | ||
| const ownsThread = await callerOwnsThread(ownerEmail, args.threadId); | ||
| if (!ownsThread) throw contextXrayThreadNotFoundError(); | ||
| const stored = await readContextManifest(args.threadId); | ||
| const manifest = | ||
| (await readContextManifest(args.threadId)) ?? | ||
| emptyContextManifest(args.threadId, { enforceable: true }); | ||
| stored ?? emptyContextManifest(args.threadId, { enforceable: true }); | ||
| const latestRun = await getRunByThread(args.threadId, { | ||
| includeTerminal: true, | ||
| }); | ||
| const latestTurnId = latestRun | ||
| ? (latestRun.turnId ?? latestRun.id) | ||
| : undefined; | ||
| // A persist failure recorded after the stored manifest was written means | ||
| // the stored manifest describes an older turn than the one that just ran. | ||
| const outcome = getContextManifestWriteOutcome(args.threadId); | ||
| const writeFailedAfterStored = | ||
| outcome?.status === "failed" && | ||
| outcome.failedAt > (stored?.updatedAt ?? 0); | ||
|
Comment on lines
+79
to
+81
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Do not mark an empty manifest as a failed persisted readingWhen there is no stored manifest, Additional Info |
||
| return { | ||
| ...manifest, | ||
| url: contextXrayDeepLink(args.threadId), | ||
| enforceable: manifest.enforceable ?? true, | ||
| source: manifest.source ?? "structured", | ||
| ...(latestTurnId ? { latestTurnId } : {}), | ||
| ...(latestRun ? { latestTurnStartedAt: latestRun.startedAt } : {}), | ||
| ...(writeFailedAfterStored ? { writeStatus: "failed" as const } : {}), | ||
| }; | ||
| }, | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { | ||
| _resetContextManifestWriteOutcomes, | ||
| buildManifest, | ||
| getContextManifestWriteOutcome, | ||
| writeContextManifest, | ||
| } from "./manifest.js"; | ||
|
|
||
| const appStatePut = vi.hoisted(() => vi.fn(async () => {})); | ||
|
|
||
| vi.mock("../../application-state/store.js", () => ({ appStatePut })); | ||
|
|
||
| describe("writeContextManifest outcomes", () => { | ||
| it("reports a failed persist instead of looking like a completed one", async () => { | ||
| _resetContextManifestWriteOutcomes(); | ||
| appStatePut.mockRejectedValueOnce(new Error("store unavailable")); | ||
| const manifest = await buildManifest({ | ||
| threadId: "thread-write", | ||
| turnId: "turn-2", | ||
| rawMessages: [], | ||
| sentMessages: [], | ||
| appliedStatus: new Map(), | ||
| directives: new Map(), | ||
| }); | ||
|
|
||
| const failed = await writeContextManifest("thread-write", manifest); | ||
| expect(failed).toMatchObject({ | ||
| status: "failed", | ||
| turnId: "turn-2", | ||
| error: "store unavailable", | ||
| }); | ||
| expect(getContextManifestWriteOutcome("thread-write")).toBe(failed); | ||
|
|
||
| const written = await writeContextManifest("thread-write", manifest); | ||
| expect(written.status).toBe("written"); | ||
| expect(appStatePut).toHaveBeenLastCalledWith( | ||
| "thread-write", | ||
| expect.any(String), | ||
| expect.objectContaining({ writeStatus: "written", turnId: "turn-2" }), | ||
| expect.objectContaining({ requestSource: "context-xray" }), | ||
| ); | ||
| _resetContextManifestWriteOutcomes(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -239,18 +239,101 @@ export async function buildManifest( | |
| }; | ||
| } | ||
|
|
||
| export type ContextManifestWriteOutcome = | ||
| | { status: "written"; turnId?: string; updatedAt: number } | ||
| | { status: "failed"; turnId?: string; failedAt: number; error: string }; | ||
|
|
||
| /** | ||
| * Last persist outcome per thread. A failed write leaves the previous turn's | ||
| * manifest in application state, which reads back as a perfectly plausible | ||
| * manifest — this map is the only thing that can tell a reader the newest | ||
| * turn never made it to storage. | ||
| */ | ||
| const writeOutcomes = new Map<string, ContextManifestWriteOutcome>(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Persist manifest write failures outside the process-local outcome mapThe failure outcome is retained only in an in-memory map, while the manifest and run lookup are durable. If the write runs in a different worker, or the process restarts before Additional Info |
||
| const MAX_TRACKED_WRITE_OUTCOMES = 500; | ||
|
|
||
| function rememberWriteOutcome( | ||
| threadId: string, | ||
| outcome: ContextManifestWriteOutcome, | ||
| ): void { | ||
| writeOutcomes.delete(threadId); | ||
| writeOutcomes.set(threadId, outcome); | ||
| while (writeOutcomes.size > MAX_TRACKED_WRITE_OUTCOMES) { | ||
| const oldest = writeOutcomes.keys().next(); | ||
| if (oldest.done) break; | ||
| writeOutcomes.delete(oldest.value); | ||
| } | ||
| } | ||
|
|
||
| export function getContextManifestWriteOutcome( | ||
| threadId: string, | ||
| ): ContextManifestWriteOutcome | undefined { | ||
| return writeOutcomes.get(threadId); | ||
| } | ||
|
|
||
| /** | ||
| * Record that no manifest could be produced for a turn (the transform threw | ||
| * before it ever reached `writeContextManifest`). | ||
| */ | ||
| export function recordContextManifestWriteFailure( | ||
| threadId: string, | ||
| error: unknown, | ||
| turnId?: string, | ||
| ): void { | ||
| rememberWriteOutcome(threadId, { | ||
| status: "failed", | ||
| ...(turnId ? { turnId } : {}), | ||
| failedAt: Date.now(), | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
|
|
||
| /** @internal test helper */ | ||
| export function _resetContextManifestWriteOutcomes(): void { | ||
| writeOutcomes.clear(); | ||
| } | ||
|
|
||
| /** | ||
| * Persist a manifest and report the outcome. Returns a typed failure instead | ||
| * of throwing so the fire-and-forget caller cannot drop it silently, and so | ||
| * `context-manifest-get` can tell readers the meter is showing an older turn. | ||
| */ | ||
| export async function writeContextManifest( | ||
| threadId: string, | ||
| manifest: ContextManifest, | ||
| ): Promise<void> { | ||
| await appStatePut( | ||
| threadId, | ||
| CONTEXT_XRAY_MANIFEST_KEY, | ||
| manifest as unknown as Record<string, unknown>, | ||
| { | ||
| requestSource: "context-xray", | ||
| }, | ||
| ); | ||
| ): Promise<ContextManifestWriteOutcome> { | ||
| const updatedAt = Date.now(); | ||
| const persisted: ContextManifest = { | ||
| ...manifest, | ||
| updatedAt, | ||
| writeStatus: "written", | ||
| }; | ||
| try { | ||
| await appStatePut( | ||
| threadId, | ||
| CONTEXT_XRAY_MANIFEST_KEY, | ||
| persisted as unknown as Record<string, unknown>, | ||
| { | ||
| requestSource: "context-xray", | ||
| }, | ||
| ); | ||
| } catch (err) { | ||
| const outcome: ContextManifestWriteOutcome = { | ||
| status: "failed", | ||
| ...(manifest.turnId ? { turnId: manifest.turnId } : {}), | ||
| failedAt: Date.now(), | ||
| error: err instanceof Error ? err.message : String(err), | ||
| }; | ||
| rememberWriteOutcome(threadId, outcome); | ||
| return outcome; | ||
| } | ||
| const outcome: ContextManifestWriteOutcome = { | ||
| status: "written", | ||
| ...(manifest.turnId ? { turnId: manifest.turnId } : {}), | ||
| updatedAt, | ||
| }; | ||
| rememberWriteOutcome(threadId, outcome); | ||
| return outcome; | ||
| } | ||
|
|
||
| export function updateManifestSegmentStatus( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1045,7 +1045,11 @@ export function startRun( | |
| const send = (event: AgentChatEvent) => { | ||
| if (run.status === "aborted" && abort.signal.aborted) return; | ||
|
|
||
| const runEvent: RunEvent = { seq: run.events.length, event }; | ||
| const runEvent: RunEvent = { | ||
| seq: run.events.length, | ||
| event, | ||
| at: Date.now(), | ||
|
Comment on lines
+1048
to
+1051
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Persist event timestamps with the run events
Additional Info |
||
| }; | ||
| if (isTerminalRunEvent(event)) { | ||
| pendingTerminalEvent = runEvent; | ||
| return; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Do not let an older async write failure invalidate a newer manifest
Manifest writes are intentionally fire-and-forget, so an older iteration can fail after a later iteration has successfully persisted. That older failure becomes the latest process-local outcome and is then surfaced against the newer stored manifest, causing the meter to show unavailable incorrectly. Track write generation/ownership and only surface a failure when it applies to the stored manifest.
Additional Info