mirror of
https://github.com/lobehub/lobehub.git
synced 2026-09-20 04:56:13 +08:00
Arvin Xu
abdd7ff74f
* ♻️ refactor(context-engine): let the engine assemble the system message The server composed the final system prompt by string-appending onto `agentConfig.systemRole` from three files across the pipeline: the resolver, `operationPrep` (project instructions), and `toolDiscovery` (borrowed-connector attribution). That is why the resolved config had to stay mutable all the way through, and why each append site carried a comment vouching for the aliasing — "appending here still reaches the LLM because `createOperation` reads `agentConfig` below". Correctness depended on nobody cloning the object, which the `do not clone` warning enforced by convention rather than by type. The mechanism for doing this properly already existed and was being bypassed. `BaseSystemRoleProvider` is purpose-built — subclasses return content, the base finds or creates the system message and joins with `\n\n` — and the engine has a declared "Phase 2: System Message Assembly" where a dozen injectors already use it. So the two late contributions become injectors. `ProjectInstructionsInjector` takes the files rather than a rendered block, because wrapping them is presentation and belongs to whoever owns the system message. `ConnectorOwnershipInjector` takes a rendered note, because naming the authorizers needs a user lookup the engine cannot perform — the same split `EvalContextSystemInjector` makes with `envPrompt`. Both run immediately after `SystemRoleInjector`, which is exactly where their text used to land. Placing them later would silently reorder the prompt behind every other Phase 2 provider; keeping the position keeps it byte-identical. The data reaches the engine on `AgentConfigWithId`, which already carries run-scoped fields stamped by one stage for another (`subAgentChatConfigOverride`). Same transport as before, structured payload instead of a concatenated string — no new plumbing through the state blob, and no size change, since the same text already travelled inside `systemRole`. Not included: the persona-level appends inside the resolver (builtin runtime role, page/task agent, per-call instructions). Those compose what the agent IS, which the resolve phase owns; these two are run context, which the engine owns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ♻️ refactor(agent): carry system-message context on the operation, not the config Follow-up to the previous commit, which moved the two late system-prompt contributions into context-engine injectors but carried their payloads on `AgentConfigWithId`. That was the same category error in a better shape: it removed the string concatenation and kept smuggling run context through the agent's configuration. Project instructions come from the workspace and the connector note from tool discovery — neither is part of what the agent IS. They now travel the way every other piece of run context does. There is an established family of them on the operation metadata — `agentShareVisitor`, `botContext`, `botPlatformContext`, `discordContext`, `userTimezone`, `evalContext` — each mapped onto `RuntimeExecutorContext` and consumed downstream, and `evalContext` in particular already feeds an injector. `expertise` is the exact precedent and this follows it end to end: `prepareOperation` and `discoverTools` return the values instead of mutating the config, `startOperation` puts them on the operation, `createOperation` persists them into `state.metadata`, `AgentRuntimeService` maps them onto the executor context, and the context builder reads them from there. Persisting matters rather than being incidental: steps can be claimed by different workers, so anything the engine needs has to survive on the operation. `createOperation` builds `state.metadata` from an explicit field list, so a value not named there is dropped without a word — the same shape of trap as the zod schema that silently stripped `createdAt`. The two fields added to `AgentConfigWithId` in the previous commit are removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * 🐛 fix(agent): forward the system-message context, and restore its order Two from review, and the first one meant the whole change was dead on the server. `serverMessagesEngine` destructures a fixed parameter list, so the two new fields never reached `MessagesEngine` and both injectors always saw `undefined`. The old concatenation was already gone, so every server run would have lost its project instructions and connector attribution outright — `AGENTS.md` silently absent from the prompt. Neither the injector unit tests nor the type checker caught it: the tests exercised the injectors directly, and the call site passes an intermediate object, which disables excess-property checking. So the regression test lives at the wrapper-to-engine seam, not on the injectors. Verified it fails without the forwarding. The injector order was also wrong. `discoverTools` runs before `prepareOperation`, so the old appends produced persona → connector note → project instructions; the injectors had them the other way round, against an adjacent comment claiming the prompt was byte-identical. Connector attribution now precedes the project instructions, with a test pinning it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✅ test(agent): cover every link the system-message context travels The forwarding bug shipped with green injector tests because they exercised the injectors directly. The wrapper test added with the fix only covers the last link; a value dropped anywhere earlier is just as silent and just as total. Every failure this feature has had — the wrapper's fixed parameter list, and `createOperation` assembling `state.metadata` from an explicit field list — was a name missing from a list rather than broken logic. Nothing about that shape surfaces in behaviour: the injectors keep working, they simply never receive anything. So each link now has an assertion of its own, and each was verified to fail with its forwarding removed: createOperation params → state.metadata (AgentRuntimeService.test) executor context → engine input (serverCallLlmContextBuilder.test) engine input → system message (serverMessagesEngine.test) One link is still covered by reading rather than by a test: `state.metadata` → executor context. And a true end-to-end run through `execAgent` is disproportionately expensive here, because the project instructions originate in the device workspace-scan path — reaching them needs an activeDeviceId, a device row and a scan result, none of which is the thing under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✅ test(agent): pin the stage order the system message depends on Drives the real `execAgent` pipeline and asserts that tool discovery runs before operation prep. That order is the reason connector attribution precedes the project instructions in the assembled system message, and until now it rested on a comment citing two line numbers — the same kind of claim that was wrong the first time this ordering was written down. Carrying a real payload end to end through this harness turned out to cost more than it returns. The project instructions originate in the device workspace-scan path, so reaching them needs an active device, a device row, a fresh cached scan, and the agent-skill lookup that happens to share the same `try` — each mock revealing the next, and none of it the thing under test. The payload is therefore covered link by link instead, which also matches the failure shape: every regression it has had was a name missing from an explicit field list, not broken logic. Worth recording separately: project instructions and the project skill lookup share one `try`, so a skill-model failure silently drops the instructions too. Pre-existing, unrelated to this change, and not fixed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>