refactor(spec)!: v17 close-out of the #3896 line — release notes, tool inert-key removal, ADR-0113 (Proposed) - #3998
Merged
Conversation
The release page had zero mention of the arc: sharing criteria fail-closed (#3929), RLS enabled enforced + priority removed (#3980/#3990), the DynamicLoadingConfig false-compliance removal (#3950), the empty-state gate, and the security-subset ledger re-verification with its two new proof bindings. Added: a Highlights bullet, two breaking-change sections with migration guidance, two dead-cluster table rows, two Spec/kernel bullets, and two upgrade-checklist items. implementation-status gains one sentence each on the sharing-criteria invariant and the RLS corrections. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
…se-out) category, permissions, active and builtIn were authorable and inert — none is part of AIToolDefinition and no execution path read them (#3686 had already corrected all four to dead+authorWarn). Two were misleading in the dangerous direction: permissions promised a capability gate on invocation that nothing enforced (a tool "requiring" capabilities ran for everyone), and active:false read as "withdrawn" while the tool kept reaching the LLM tool set and POST /ai/tools/:name/execute kept running it — the same false-compliance shape as rls.enabled, resolved the same week. The retirement kit, per the requiresConfirmation precedent (#3715): - .strict() ToolSchema rejects each retired key with its own prescription (TOOL_RETIRED_KEY_GUIDANCE); 6 new tests pin the rejections and that the two dangerous ones name the REAL mechanism (action.requiredPermissions / drop from skills-agents). - D2 conversion + D3 chain step (tool-inert-authoring-keys-removed): os migrate meta strips the keys mechanically; lossless by construction. - ToolCategorySchema/ToolCategory removed with the key they typed (zero consumers; action.zod.ts keeps its own inline vocabulary on purpose). - The Studio tool form drops the retired inputs — a form input for an unenforced gate is the UI half of false compliance (objectui#2962 shape). - Ledger entries deleted (#3715 precedent), README tool row updated, authorable-surface + json-schema.manifest baselines edited deliberately, reference docs + v17 release notes regenerated/extended. spec: 6883 tests green, tsc clean, all gates green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
…lumn constraint as its own axis The #3896 pattern generalized: field.required binds three meanings through one knob (record-validator write check, sql-driver.ts:4901 NOT NULL DDL, schema-drift.ts:249 drift expectation), so tightening any invariant on a deployed object is a destructive migration blocked by the very legacy nulls that motivated it. criteria_json's three imperative guards and its missing required marker are the observed cost. Proposed: required keeps the write-time + UI meanings (insert must provide, update may not null out, legacy rows rest); an explicit storage property owns the DDL and the destructive-migration ceremony; drift reads the storage property. criteria_json is the first consumer. Three open questions marked for adjudication — this lands as Proposed, not Accepted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 111 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…l refs regenerated CI caught the two things the tool-key removal missed: the generated skills/objectstack-ai/references/_index.md (gen:skill-refs was the one generator not run), and — found by sweeping hand-written docs the corrected way — the SKILL.md Tool Configuration section, whose prose listed category/permissions/active as optional keys and whose os:check'd defineTool example authored category. That file is published FOR models to read as the authoring contract; with the tombstone it was teaching a parse error. Prose now names the retirement and where each concern actually lives (action.requiredPermissions / drop-from-skills / action.ai.category); the category table is gone with the key. Other sweep hits were false alarms: action.ai.category and skill.active are live, different surfaces. check:skill-refs / skill-docs / skill-examples all green (198 prose examples type-check against the rebuilt spec). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
…h the tool key removal The Type Check job is a serial gauntlet — each fix revealed the next stale projection. This closes the remaining two, verified by running EVERY sub-check of the job locally this time: api-surface.json drops the removed ToolCategory/ToolCategorySchema exports (breaking is intentional, the major changeset is in this PR), and the four metadata-forms translation bundles drop the retired tool-form fields' strings (merge mode — pure deletions, no existing translation touched). check:api-surface, check:react-blocks, check:skill-examples, check:i18n, check:i18n-coverage all green locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
…ts ledger entry Test Core caught the one consumer of the deleted ledger entry: the CLI authorWarn lint is ledger-driven, and a test pinned the warning for authored tool.permissions. Enforcement moved a layer down in this PR — the strict parse rejects the key with its prescription before any lint could run — so the test now pins the absence (same shape as tenancy #2763 and contextVariables ADR-0105 D11, which sit in the same test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
os-zhuang
marked this pull request as ready for review
July 30, 2026 03:08
…schema-bypass-h4c7xr
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
2 participants
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 three follow-ups approved after #3990, one commit each: the v17 release documentation for the whole security-hardening arc, the tool-surface false-capability removal while the breaking window is open, and the ADR that generalizes the structural lesson — left as Proposed for adjudication.
1. v17 release notes carry the security-hardening line (docs only)
releases/v17.mdxhad zero mention of the arc. Added, in the page's own voice:enabled/prioritypair.DynamicLoadingConfig(refactor(spec)!: remove the plugin sandboxing / integrity / approval config that never existed (#3896 follow-up) #3950) andrls.priority(refactor(spec)!: close the three #3980 triage items — remove rls.priority, bind tabPermissions + writeScope proofs #3990); the dead-sweep Highlights bullet now names the four tool keys (part 2 below).check:empty-stategate.implementation-status.mdx: one sentence each on the sharing-criteria invariant and the RLS corrections.2. The four inert tool authoring keys are removed (breaking, changeset: major)
tool.category/permissions/active/builtInwere authorable and inert — none is part ofAIToolDefinition, no execution path read them (the ledger corrected all four to dead+authorWarn in #3686). Two were misleading in the dangerous direction, which is what makes this the v17 move rather than backlog:permissionspromised a capability gate on invocation. Nothing enforced it — a tool "requiring" capabilities ran for everyone. Real gates:action.requiredPermissions(ADR-0066), permission sets on the touched objects.active: falseread as "withdrawn" and withdrew nothing:ToolRegistry.getAll()returns everything, the tool kept reaching the LLM tool set, andPOST /ai/tools/:name/executekept running it — unlikeagent.active/skill.active, which are enforced. The same false-compliance shape asrls.enabled, resolved the same week.Retirement kit, per the
requiresConfirmationprecedent (#3715): the.strict()ToolSchema rejects each key with its own prescription (6 new tests pin the rejections and that the two dangerous ones name the real mechanism); D2 conversion + D3 chain step (tool-inert-authoring-keys-removed) soos migrate metastrips them mechanically;ToolCategorySchema/ToolCategoryremoved with the key they typed (zero consumers —action.zod.tskeeps its own inline vocabulary on purpose); the Studio tool form drops the retired inputs (a form input for an unenforced gate is the UI half of false compliance — the objectui#2962 shape); ledger entries deleted, README tool row updated, both baselines edited deliberately.3. ADR-0113 (Proposed) —
requiredas a write contract, the column constraint as its own axisThe structural lesson behind #3929's three imperative guards, written up for decision — not implemented. The tri-binding is cited at its exact sites:
requiredtoday means the write check (record-validator.ts), theNOT NULLDDL (sql-driver.ts:4901), and the drift expectation (schema-drift.ts:249) through one knob, so tightening any invariant on a deployed object is a destructive migration blocked by the very legacy nulls that motivated it.Proposed rulings:
requiredkeeps the write-time + UI meanings (insert must provide, update may not null out, legacy rows rest — the semantics #3929 hand-built, promoted to one word); an explicitstorage.notNullowns the DDL and keeps the destructive-migration ceremony; drift reads the storage property;criteria_jsonis the first consumer and gets its required marker back. Three open questions are flagged for the deciders (spelling; the new-field default;requiredWheninterplay). Merging this PR accepts the document, not the design — the Status line says Proposed.Verification
packages/spec: 6894 tests green (chain-replay gate composes the new conversion with feat(runtime)!: the undeclared-action refusal has no opt-out — dropOS_ALLOW_UNDECLARED_ACTIONS(ADR-0110 D3 revised) #3987's and everything since).tscclean.check:liveness,check:empty-state,check:authorable-surface(baselines deliberate),check:spec-changes,check:upgrade-guide,check:docs(251 files in sync).Generated by Claude Code