-
Notifications
You must be signed in to change notification settings - Fork 91
fix(opencode): move volatile date out of cached system prefix #950
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
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 |
|---|---|---|
|
|
@@ -969,6 +969,24 @@ export namespace SessionPrompt { | |
|
|
||
| await Plugin.trigger("experimental.chat.messages.transform", {}, { messages: msgs }) | ||
|
|
||
| // altimate_change start — upstream_fix: carry the date in the trailing user message | ||
| // so it stays out of the cache-controlled system prefix (see session/system.ts). | ||
| const lastUserForDate = msgs.findLast((m) => m.info.role === "user") | ||
| if (lastUserForDate) { | ||
| lastUserForDate.parts = [ | ||
| ...lastUserForDate.parts, | ||
| { | ||
| type: "text" as const, | ||
| id: PartID.ascending(), | ||
| sessionID, | ||
| messageID: lastUserForDate.info.id, | ||
| text: `\n\n${SystemPrompt.currentDate()}`, | ||
| synthetic: true, | ||
| }, | ||
| ] | ||
| } | ||
|
Comment on lines
+974
to
+987
Author
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. loop() already throws "No user message found" earlier if msgs has no user entry, so there's always a trailing user message to append to at this point. The date can't be silently dropped. |
||
| // altimate_change end | ||
|
|
||
| // Build system prompt, adding structured output instruction if needed | ||
| const skills = await SystemPrompt.skills(agent) | ||
| // altimate_change start - unified context-aware injection for memory + training | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { describe, expect, test } from "bun:test" | ||
| import { describe, expect, setSystemTime, test } from "bun:test" | ||
| import path from "path" | ||
| import { Agent } from "../../src/agent/agent" | ||
| import { Instance } from "../../src/project/instance" | ||
|
|
@@ -160,4 +160,27 @@ description: ${description} | |
| process.env.OPENCODE_TEST_HOME = home | ||
| } | ||
| }) | ||
|
|
||
| test("environment() keeps the volatile date out of the cached system prefix", async () => { | ||
| // Freeze the clock so the captured `today` and the `new Date()` inside | ||
| // currentDate() read the same instant — otherwise the assertion can race | ||
| // across midnight. | ||
| setSystemTime(new Date("2026-06-17T12:00:00.000Z")) | ||
| try { | ||
| await using tmp = await tmpdir({ git: true }) | ||
| await Instance.provide({ | ||
| directory: tmp.path, | ||
| fn: async () => { | ||
| const today = new Date().toDateString() | ||
| const [env] = await SystemPrompt.environment(makeModel({ apiId: "claude-3-7-sonnet" })) | ||
| expect(env).toMatch(/<env>/) | ||
| expect(env).not.toMatch(/Today's date/) | ||
| expect(env).not.toContain(today) | ||
| expect(SystemPrompt.currentDate()).toContain(today) | ||
| }, | ||
| }) | ||
| } finally { | ||
| setSystemTime() | ||
| } | ||
| }) | ||
|
Comment on lines
+164
to
+185
Author
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. Good catch - captured the date once at the top of the test so all three assertions compare the same value. |
||
| }) | ||
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.
findLast is already used elsewhere in this file (the reminder-wrapping code just above), so the build target already supports it - kept it for consistency.