Repository navigation
Load agent catalogs each turn instead of caching them on the chat - #267
Conversation
e80909d to
399bf68
Compare
399bf68 to
820fb5a
Compare
Preview:
|
|
Submitted 2 actionable inline findings. |
820fb5a to
e155191
Compare
|
Submitted 1 actionable inline finding. |
e155191 to
482af19
Compare
|
482af19 to
fd0e45c
Compare
|
Submitted 1 actionable inline finding. |
fd0e45c to
9202edc
Compare
|
Submitted 1 actionable inline finding. |
9202edc to
4566159
Compare
|
e51cf65 to
de95ab3
Compare
|
|
Replies to the two Bonk findings on de95ab3. Hidden ambient catalogs are still loaded. The premise does not hold. The ambient fold ( Do not reinterpret |
A chat cached its agent catalogs and never loaded them again, so the agent could not see a skill added after the chat opened. A failed load was cached too, as null, which means "this gatekeeper has no catalog" -- so one failure read as an empty library for the rest of the chat. The cache was the cause of both. A catalog states what a session can reach now, which is not a fact a chat can hold. Loading it per turn removes the stale window and the cached failure together, and deletes the snapshot type, the completer, the chat field that stored it, and their tests. A connection blocked pending a scope-widening restart is still skipped. The cost is one call per ambient gatekeeper per runAgent invocation. An automatic compaction reruns the turn, so that turn loads twice.
Loading catalogs each turn made every pass call getAgentCatalog() on every
ambient connection, including Scheduler, which answers null on every call. The
Workshop cannot tell which ambient gatekeepers have a catalog: the method is
optional on Gatekeeper, nothing declares it, and over a stub every method looks
callable.
Sharpen the existing null return instead of adding a flag. null now means the
gatekeeper has no catalog, and the Workshop remembers that per connection in
memory for the life of the workspace's Durable Object. A gatekeeper that gains
a catalog in a later version is asked again on the next activation, so every
chat converges. An empty catalog is {entries: []} and is still asked for every
turn. A failed load is not remembered.
Context never returns null and Scheduler always does, so neither changes. The
integration fixture's ambient gatekeeper had no getAgentCatalog at all and took
the failure path on every pass; it now answers null like Scheduler.
The Workshop loads the catalog into every chat's prompt on every turn, so it is expected and mandatory. Treating it as an observation recorded an action and a transcript entry per turn, and Context's excludeObservers check could refuse the whole catalog in a shared workspace once the owner added a private collection. The catalog is a discovery index of titles and descriptions; it must not hold anything that needs observer verification, or every workspace would be affected at once. Reading an item through the session remains an observation. getAgentCatalog() no longer takes an authorizer, and the Workshop no longer builds one for the load. Context drops the observation and observer tracking from its catalog; Scheduler and the integration fixture drop the unused parameter.
de95ab3 to
6736658
Compare
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
|
On the two P1 findings (Bonk, Devin) that private collection titles, descriptions, and skill names now reach collaborators through the prompt without an observer check: this is the intended change, decided by @kentonv. Receiving the catalog is expected and mandatory, so it is not an observation, and the catalog must not contain anything sensitive; if it did, every workspace would be affected at once. The |
# By Maximo Guk (9) and others # Via GitHub * origin: (33 commits) Bump the react-and-ui group across 1 directory with 4 updates (cloudflare#515) Bump the editor-codemirror group across 1 directory with 5 updates (cloudflare#502) Bump the remaining-npm group across 1 directory with 5 updates (cloudflare#501) Bump the github-actions group with 2 updates (cloudflare#499) Fix blueprint configurator readiness (cloudflare#505) fix sizing issues with user search ui (cloudflare#511) gatekeeper-confluence: stop reporting the access-token expiry as the credential expiry (cloudflare#509) Review Workshop eval trajectory changes (cloudflare#477) Compare Workshop evals on pull requests (cloudflare#476) Re-check compaction after every agent step (cloudflare#493) Write the bundled format blueprints in TypeScript (cloudflare#466) add deployment-wide user directory for user search (cloudflare#474) Load agent catalogs each turn instead of caching them on the chat (cloudflare#267) Updated agent prompt: avoid unnecessary implementation details in response, don't always create gadgets (cloudflare#489) Fix Anthropic streams in the local eval target; default evals to GLM 5.3 Flash (cloudflare#495) Make spawned agents persistent across restarts (cloudflare#492) Bump vitest from 4.1.10 to 4.1.11 (cloudflare#470) Restricted data UI - share modal stays usable for restricted workspaces (cloudflare#308) Restricted data: govern restricted reads by observer verification (cloudflare#382) Rename prohibitAllSharing -> containsRestrictedData (cloudflare#381) ... # Conflicts: # packages/workshop-backend/src/user.ts # packages/workshop-shared/src/api.ts
Second upstream catch-up, 08afe05..bfe217f: 31 upstream commits, 13 days. Merged, never rebased: pointfive-os pins p5 commits by SHA in its submodule gitlink, so rewriting them would orphan the commit past deployments were built from. Merged clean. All four PointFive changes survive; upstream's rename of build-browser-runtime.mjs to scripts/build-browser-runtime.ts carried the Gadget bundling steps along. One downstream fix is needed in pointfive-os: Gatekeeper.getAgentCatalog() lost its authorizer parameter (cloudflare#267).
…prompt rebuild (port cloudflare/cloudflare-os#267) The persisted <available_skills> index is reused byte-for-byte across turns so the provider prefix cache stays warm (#104414). A skill that lands after the prompt was built (hub install, skill_manage from another session, org sync, curator) therefore never reached the index the model actually routes on until compaction rebuilt the prompt: skills_list is live, but the prompt tells the model to scan the index, not to call it. cloudflare-os#267 found the same fault in their chat-cached catalog ("a catalog states what a session can reach now, which is not a fact a chat can hold"). Hermes' idiom for stored-prompt drift is a one-shot note behind the cached prefix, not a rebuild, so agent/skills_index_delta.py stages the delta on the same per-turn user-message channel the surface-switch note rides: the added skills' own index lines (name + description, the routing signal), the removed names, cumulative against the stored prompt and re-staged only when the delta changes (read back from the api_content sidecar, so the gateway's fresh agent per turn does not stack copies). An undone delta retires the stale note once. The current index comes from the same cached builder the prompt did, so an unchanged turn costs one LRU hit.
…prompt rebuild (port cloudflare/cloudflare-os#267) The persisted <available_skills> index is reused byte-for-byte across turns so the provider prefix cache stays warm (#104414). A skill that lands after the prompt was built (hub install, skill_manage from another session, org sync, curator) therefore never reached the index the model actually routes on until compaction rebuilt the prompt: skills_list is live, but the prompt tells the model to scan the index, not to call it. cloudflare-os#267 found the same fault in their chat-cached catalog ("a catalog states what a session can reach now, which is not a fact a chat can hold"). Hermes' idiom for stored-prompt drift is a one-shot note behind the cached prefix, not a rebuild, so agent/skills_index_delta.py stages the delta on the same per-turn user-message channel the surface-switch note rides: the added skills' own index lines (name + description, the routing signal), the removed names, cumulative against the stored prompt and re-staged only when the delta changes (read back from the api_content sidecar, so the gateway's fresh agent per turn does not stack copies). An undone delta retires the stale note once. The current index comes from the same cached builder the prompt did, so an unchanged turn costs one LRU hit.
…prompt rebuild (port cloudflare/cloudflare-os#267) The persisted <available_skills> index is reused byte-for-byte across turns so the provider prefix cache stays warm (NousResearch#104414). A skill that lands after the prompt was built (hub install, skill_manage from another session, org sync, curator) therefore never reached the index the model actually routes on until compaction rebuilt the prompt: skills_list is live, but the prompt tells the model to scan the index, not to call it. cloudflare-os#267 found the same fault in their chat-cached catalog ("a catalog states what a session can reach now, which is not a fact a chat can hold"). Hermes' idiom for stored-prompt drift is a one-shot note behind the cached prefix, not a rebuild, so agent/skills_index_delta.py stages the delta on the same per-turn user-message channel the surface-switch note rides: the added skills' own index lines (name + description, the routing signal), the removed names, cumulative against the stored prompt and re-staged only when the delta changes (read back from the api_content sidecar, so the gateway's fresh agent per turn does not stack copies). An undone delta retires the stale note once. The current index comes from the same cached builder the prompt did, so an unchanged turn costs one LRU hit.
What does this change?
Three commits, one concern each.
Load catalogs each turn. A chat cached its agent catalogs and never loaded them again. The
agent could not see a skill added after the chat opened. A new skill reached the slash-command
picker at once, because that list is live, but it never reached the agent's catalog. The user had
to open a new chat. A failed load was cached the same way, as
null, so one failure read as anempty library for the rest of the chat. The cache caused both faults. A catalog states what a
session can reach now, which is not a fact a chat can hold. This loads it on every
runAgentpass and deletes the snapshot type, the completer, the chat field that held it, and their tests.
Stop asking a connection that has no catalog. Scheduler answers
nullon every call, and theWorkshop cannot tell which ambient gatekeepers have a catalog without asking.
nullnow means thegatekeeper has no catalog, and the Workshop remembers that per connection, in memory only, so a
gatekeeper that gains a catalog in a later version is asked again on the next activation of the
workspace. An empty catalog is
{entries: []}and is loaded every turn. A failed load is notremembered.
Receiving the catalog is not an observation. The catalog reaches every chat's prompt
automatically, so it is expected and mandatory, and it must not contain anything that needs
observer verification or every workspace would be affected at once. Treating it as one recorded
an action and a transcript entry per turn, and Context's
excludeObserverscheck could refusethe whole catalog in a shared workspace once the owner added a private collection.
getAgentCatalog()drops its authorizer parameter, Context drops the observation and observertracking from its catalog, and Scheduler and the integration fixture drop the unused parameter.
Reading an item through the session is still an observation.
Cost
One
getAgentCatalogcall per ambient gatekeeper that has a catalog, each timerunAgentruns.An automatic compaction reruns the turn, so that turn loads twice. No action records, no
transcript entries.
A failure costs one turn's catalog. The next turn loads it again.
Checklist
Checking every item does not guarantee acceptance. Maintainers determine whether
a pull request meets the contribution policy.