{{ message }}
Studio: repin the permission-mode contract to behaviour, not a declaration - #8986
Merged
Conversation
…ation #8686 hoisted the per-call `const permissionMode = loadPermissionMode();` in chat-runtime-store.ts up to a module-level INITIAL_PERMISSION_MODE. The store still reads the persisted level and still refuses to restore "full", so the behaviour is unchanged, but test_research_presentation_is_integrated pinned the old declaration's exact text and went red on main. Assert the contract instead: the level is read through loadPermissionMode(), and the store's initial permissionMode binds to that read rather than a hardcoded level. That also covers a case the literal never did.
for more information, see https://pre-commit.ci
Re-checked what #8686 actually did. It did not hoist the per-call read: it put a chat-scoped override in front of it, const permissionMode = threadScopedOverride("permissionMode") ?? loadPermissionMode(); at both call sites, so the read is still per call. The module-level INITIAL_PERMISSION_MODE it also added is a separate thing, used only for the store's initial state. That matters here. The assertion this file lost sat in the deep-research block and guarded setDeepResearchEnabled re-resolving the level. My first replacement asserted the initial state instead, which is a different property: it would still pass if the setter stopped re-reading and used the cached constant. Scope the assertion to setDeepResearchEnabled and pin the read that is actually the contract. Mutation checked against the real file: permissionMode = INITIAL_PERMISSION_MODE caught (missed before) threadScopedOverride(...) ?? "ask" caught
Member
Author
|
@codex review |
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
1 participant
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.
Repo tests (CPU)is red on main with a single failure:Cause
#8686 (
5bb0bc6f7) hoisted the two per-callconst permissionMode = loadPermissionMode();reads inchat-runtime-store.tsup to a single module-level constant:The store still reads the persisted level out of storage, and still refuses to restore "full" across sessions, so nothing the assertion was written to protect changed. #8686 is correct; the test was pinned to the spelling of a declaration rather than to the behaviour, so a pure refactor turned it red.
Fix
Assert the contract:
const <name> = loadPermissionMode();exists, so the level comes out of storagepermissionModebinds to that constant (or callsloadPermissionMode()inline), so it is never a hardcoded levelThis is strictly stronger than what it replaces. Both are mutation checked against the real file:
const INITIAL_PERMISSION_MODE = "ask";(drops the storage read)permissionMode: "ask",in the initial stateVerification
tests/studiolocally on a CPU-only shape: 3877 passed, 6 skipped. The 8 errors intest_chat_thread_title_cas.pyare a missingstructlogin my local venv, not related; CI installs it and that file is green on main.