Move the instruction a coworker actually follows when somebody edits it - #32
Merged
Conversation
A coworker with no address runs on `configuration.systemPrompt` and on nothing else: `registeredAgentFromRow` gives a `built_in` agent that column and no standing role message, so `agentProfiles.roleDescription` never reaches it. `create` writes both. `update` wrote only the profile. So editing such a coworker's role description changed every screen and nothing the Bot follows. The edit reported success, the text was there when you came back, and the coworker went on answering from the instruction it was made with — permanently, with nothing anywhere to say so. It is the same shape as the endpoint bug the comment above this code already warns about, arriving through the other field. Live rather than theoretical: production runs with no managed Bot, so every coworker created without an address takes the `built_in` branch — which since the bot-creator shipped is most of them, because every coworker that card makes is endpoint-less. `update` now carries the role description into the prompt for `built_in` rows and only those. A remote Bot must not acquire a prompt it never had: its instruction travels as the standing role message, and a `systemPrompt` in its configuration would be a second source for the same thing — and the one the runtime prefers if that row's type ever changed. The tenant package's own Bots, whose `system_prompt` is deliberately not their `role_description`, cannot reach `update` at all, because `requireManageable` throws `ProtectedAgentError` for anything the package owns. Verification: 2897 pass, 23 skip, 0 fail across 228 files; format, lint and typecheck clean. Two integration tests, because the pair can only drift silently — one that an edit moves the prompt a coworker running here follows, one that a coworker at its own address never acquires one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
A coworker with no address runs on
configuration.systemPromptand on nothing else.registeredAgentFromRowgives abuilt_inagent that column and no standing role message, soagentProfiles.roleDescription— the field every screen shows and the edit form writes — neverreaches it.
createwrites both.updatewrote only the profile.So editing such a coworker's role description changed everything a person can see and nothing the Bot
follows. The edit reports success, the new text is there when you come back, and the coworker goes on
answering from the instruction it was made with, permanently, with nothing anywhere to say so. It is
the same shape as the endpoint bug the comment directly above this code already warns about —
"the screen reported success and the Bot kept answering at the old address" — arriving through the
other field.
This is live, not theoretical. Production has no
MANAGED_AGENT_AG_UI_URL, so every coworkercreated without an address takes the
built_inbranch. Since #31that is most of them: every coworker the bot-creator card makes is endpoint-less by design.
The approach
updatecarries the role description intoconfiguration.systemPromptforbuilt_inrows, and onlythose.
Only
built_in, and the two exclusions are the point. A remote Bot must not acquire a prompt itnever had — its instruction travels as the standing role message built from the profile, so a
systemPromptin its configuration would be a second source for the same thing, and the one theruntime prefers if that row's type ever changed. And the tenant package's own Bots, whose
system_promptis deliberately not theirrole_description(General Assistant carries "You are ahelpful general assistant…" against a role of "Help with everyday work…"), cannot reach
updateatall:
requireManageablethrowsProtectedAgentErrorfor anything the package owns. I checked thatbefore writing the condition, because a blanket sync would have overwritten those.
What is not covered
their prompt another, and nothing here reconciles the two retrospectively. A person can restore it
by saving the role description once more, and that now takes effect. Not backfilled, because
guessing which of the two texts somebody meant is not a migration's decision to make.
but that is a property of this code being right rather than something the UI would notice if it
regressed. The tests are what hold it.
Verification
format:check,lintandtypecheckclean. 2897 pass, 23 skip, 0 fail across 228 files.an edit moves the instruction a coworker that runs here actually followsconfiguration.systemPromptand the profile carry the new textan edit never gives a coworker at its own address a system promptsystemPromptappears in its configurationBoth are integration tests against a real database rather than unit tests over the merge, because the
bug was in what got written and a test over the object would have agreed with it.
One note on running the suite locally: a database migrated from a branch without the Slack migrations
reports 47 failures in the external-link and external-thread stores that have nothing to do with this
change. They reproduce identically on a clean
main. Migrate a database on this branch beforebelieving a failure in those files.
Merge notes
The same hunk is in CopilotKit/OpenBot#331, which
carries the create half and this update half together for upstream. Nothing else here overlaps, and
there is no migration.