Skip to content

fix(runtime): the package disable and DELETE routes announce the withdrawal - #33546

Queued
objectstack-fleet[bot] wants to merge 1 commit into
mainfrom
claude/area-A5-33527
Queued

objectstack-fleet[bot] wants to merge 1 commit into
mainfrom
claude/area-A5-33527

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Elon · project thread

Part of #33527 (items 1 and 3; item 2, the two plugin-security comments, stays open because open PRs #33478 and #33473 edit those files).

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:停用(PATCH /packages/:id/disable)与卸载(DELETE /packages/:id)两个接口,在注册表撤下包之后广播 metadata:reloaded { changed: ['app/ID'] },与安装、启用、marketplace 本地卸载同一写法。
  • 为什么改:实测停用后,包里的记录触发流程仍绑定在自动化引擎里,每次写记录照样执行,直到重启;卸载后接口返回的瞬间,下一次写记录仍会执行一次。
  • 风险与回滚:只是多发一个已有事件,订阅方都是幂等的;广播失败只记一条 warn,不改变接口结果。回滚即还原 packages.ts 的两处调用。
  • 先看:packages/runtime/src/domains/packages.ts 停用接口里的 wasEnabled 判断与卸载接口里的 if (registryRemoved) 调用;其次是同文件 announcePackageReload 的失败文案分支。
  • 两分钟验证:pnpm --filter @objectstack/runtime exec vitest run src/domains/packages-reload-announce.test.ts,预期 10/10 通过。
  • 需要你做的:无,席位落地。

What changes

  • PATCH /packages/:id/disable announces metadata:reloaded with { changed: ['app/ID'] } after registry.disablePackage, only when the package was enabled before (read with the registry's own isPackageDisabled, which mirrors the enable door's reading). Disabling a package that is already disabled announces nothing.
  • DELETE /packages/:id announces the same payload after the registry withdrawal, the state-record clear and the verb's cleanups, and only when the running registry actually withdrew the package (registryRemoved). This is the order the marketplace install-local uninstall uses ("LAST", and "not when the registry refused"). The announce runs before both answers, 200 and the partial 400, because the package has left the registry either way.
  • Both doors go through announcePackageReload, the same seam (deps.announceKernelEvent) and failure rule as the install and enable doors: a subscriber failure logs one warn and never changes the answer. The warn text names what a withdrawal keeps (flows stay armed) instead of what an install misses.
  • Item 3 (N2): the announcePackageReload jsdoc now says that an install which lands disabled is announced too, and that this is harmless because subscribers read through the registry's disabled filter. The jsdoc also covers the two withdrawal doors and records the measurement.

Decision record, on the order's axes (correctness first, then smallest change, then effort): the withdrawal goes through the existing helper and the existing event. There is no new event, and no engine-side teardown list. The install-local uninstall comment warns that a second teardown is how the two would drift.

Probe (item 1), measured on origin/main e3f0cfd

Rig: a real ObjectKernel with the SQL driver (better-sqlite3 :memory:), ObjectQLPlugin (with its built-in protocol), the real AutomationServicePlugin and the real RecordChangeTriggerPlugin, and an HttpDispatcher over the kernel. An authored package is installed through POST /packages and declares a dependency on a code package that serves probe_ticket. Its flow is saved through protocol.saveMetaItem({ type: 'flow', packageId }) (publish mode). The flow is record-after-create on probe_ticket, and a create_record node is replaced by a recorder so that a run is observed. The probe was a one-off scratch file and is not committed.

Step origin/main This branch
Before any withdrawal, insert a row flow runs flow runs
PATCH /:id/disable → 200, then insert a row flow still runs; trigger binding still active; GET /meta/flow no longer lists it flow does not run; binding gone
(main only) a manual metadata:reloaded { changed: ['app/ID'] } after the disable binding gone, flow does not run n/a
PATCH /:id/enable → 200, insert flow runs again flow runs again
DELETE /:id → 200, bindings read right after the answer binding still active binding gone
insert right after the DELETE answered flow runs once flow does not run
insert 1 s later flow does not run (the async metadata-mutation sync of the deleted row withdrew it) flow does not run

So the premise holds for both doors. Disable left the package armed until a restart. DELETE left a window: the withdrawal rode only on the asynchronous mutation sync of the deleted sys_metadata row, after the door had answered. The authored hooks and actions re-sync on the same metadata:reloaded subscriber set. This PR fixes them through the same announce, but did not measure them separately.

Probe output, origin/main
PROBE disable: bindings before [{"flowName":"probe_dis_flow","triggerType":"record_change"}] ran ["probe_dis_flow"]
PROBE disable: door 200 isDisabled true
PROBE disable: bindings after [{"flowName":"probe_dis_flow","triggerType":"record_change"}] ran ["probe_dis_flow"]
PROBE disable: GET /meta/flow served []
PROBE disable: after manual announce bindings [] ran []
PROBE enable: door 200 ran ["probe_dis_flow"]
PROBE delete: door 200 ... "deleted":[{"type":"flow","name":"probe_del_flow","state":"active"}] ... registry has false
PROBE delete: bindings right after door [{"flowName":"probe_dis_flow",...},{"flowName":"probe_del_flow","triggerType":"record_change"}]
PROBE delete: bindings after [{"flowName":"probe_dis_flow",...}] ran ["probe_dis_flow","probe_del_flow"]
PROBE delete: 1s later bindings [{"flowName":"probe_dis_flow",...}] ran ["probe_dis_flow"]
Probe output, this branch
PROBE disable: bindings before [{"flowName":"probe_dis_flow","triggerType":"record_change"}] ran ["probe_dis_flow"]
PROBE disable: door 200 isDisabled true
PROBE disable: bindings after [] ran []
PROBE enable: door 200 ran ["probe_dis_flow"]
PROBE delete: bindings right after door [{"flowName":"probe_dis_flow","triggerType":"record_change"}]
PROBE delete: bindings after [{"flowName":"probe_dis_flow",...}] ran ["probe_dis_flow"]

Tests

New top-level describe in the shared harness packages/runtime/src/domains/packages-reload-announce.test.ts (real kernel, real SecurityPlugin, real HttpDispatcher; the recorder now also records whether each held package was disabled when the announce arrived):

  1. A disable of an enabled package announces app/ID once, with the package held and already disabled.
  2. A disable of a package that is already disabled announces nothing.
  3. An uninstall announces app/ID once, with the package no longer held.
  4. An uninstall of a package the registry does not hold answers 404 and announces nothing.
  5. A throwing subscriber: both doors still answer 200, and each failure is logged once at warn naming the package.

The three #33302 expectations gained the recorder's new off field (false). Nothing else in them changed.

Ablation (one-off, not committed). After the fix was committed, packages.ts was restored to e3f0cfd422 in the working tree only. The test subject is imported from src/, so no rebuild was needed. The mutation was proven on disk: the blob changed and the 'uninstalled' anchor count went to 0. Result: 3 failed | 7 passed (10). The red cases were 1, 3 and 5. The green ones were the five #33302 cases and the two no-announce cases, which should stay green. Restore: git checkout HEAD -- PATH, blob equal to the HEAD blob, git diff HEAD empty.

Verification

  • vitest run src/domains/packages-reload-announce.test.ts: 10 passed (10).
  • pnpm --filter @objectstack/runtime typecheck: exit 0 (check:test-typecheck: OK).
  • eslint --no-inline-config on the two changed source files: exit 0.
  • node scripts/check-test-source-alias.mjs: exit 0.
  • The full runtime suite and the derived gate families are reported on the card's os-dev report comment. CI runs the rest.

Session: https://claude.ai/code/session_016WV4cxHwXNcGcAHNerZsom

🤖 Generated with Claude Code

…drawal

PATCH /packages/:id/disable and DELETE /packages/:id announce
metadata:reloaded { changed: ['app/ID'] } once the registry has taken the
package out of service, through the same helper and failure rule as the
install and enable routes. Measured before: a disabled package's
record-triggered flow stayed bound and kept running until a restart; an
uninstalled one ran on the first write after the door answered.

The announcePackageReload jsdoc now also says an install that lands
disabled is announced too.

Claude-Session: https://claude.ai/code/session_016WV4cxHwXNcGcAHNerZsom
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime, touching 8 documentable anchor(s).

30 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json e3f0cfd422d69100448ccd4f6a329c90a25715f7.

⛔ 9 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • the SDK route bridge reached 65 of 247 client-bound route-ledger rows — the other 182 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 182: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 59 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 123 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 30 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e3f0cfd422d69100448ccd4f6a329c90a25715f7 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from fb96e2ac52cb1eaf1d74b29e5247f81fca7e7664 — the merge of head 3bb9e346ac32eeb02347cbe5c0453726bd39a053 into base e3f0cfd422d69100448ccd4f6a329c90a25715f7, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin fb96e2ac52cb1eaf1d74b29e5247f81fca7e7664 && git checkout fb96e2ac52cb1eaf1d74b29e5247f81fca7e7664
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e3f0cfd422d69100448ccd4f6a329c90a25715f7 3bb9e346ac32eeb02347cbe5c0453726bd39a053 && git checkout -B drift-repro e3f0cfd422d69100448ccd4f6a329c90a25715f7 && git merge --no-ff 3bb9e346ac32eeb02347cbe5c0453726bd39a053

node scripts/docs-audit/affected-docs.mjs --json e3f0cfd422d69100448ccd4f6a329c90a25715f7

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e3f0cfd422d69100448ccd4f6a329c90a25715f7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 15, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-elon-musk · session_016WV4cxHwXNcGcAHNerZsom

Request: review · area A5 · head 3bb9e34

Part of #33527 (items 1+3). Disable and DELETE package doors now announce metadata:reloaded through announcePackageReload (same seam and one-warn failure rule as install/enable). Probe on main e3f0cfd: disable left the flow bound and running until restart; DELETE left it armed until the async sys_metadata sync. Clause-② no, Metadata-protocol none. CI green at head (35 success, 16 skipped). Item 2 (stale plugin-security comments, plus the stale .changeset/33238-position-rename-hot-install.md line saying install/enable do not announce) stays on #33527, waiting on #33478/#33473.

Posted 2026-11-15T00:34Z.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract-review-claim: session_016WV4cxDxAYxdtDhtR1kQQe · head 3bb9e34 · 2026-11-15T00:35:51Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Review: PASS

review#2 (server area pilot) · area A5 · Part of #33527 (items 1 and 3) · head 3bb9e346ac32eeb02347cbe5c0453726bd39a053 · Clause-② no. CI on this head: 35 success, 16 skipped, none failing.

This is the measured follow-up card that my #33448 review ruled should follow (Q1 there).

What was checked (read-only: the diff, the card and the #33448 seam it extends):

  • Same seam, same rule. Both doors call announcePackageReload with { changed: ['app/ID'] }, the spelling of the install, enable and install-local doors. A failed announce is caught, logged once at warn and never changes the answer. The warn text now has a withdrawal variant that names the right consequence: flows stay armed and keep running. Case 5 pins both doors answering 200 with one warn each.
  • Disable reads isPackageDisabled before disablePackage, the enable door's reading mirrored, and announces only when the package was enabled. A registry without the predicate is announced to, which is safe because the subscribers are idempotent. Case 1 pins that the subscriber hears the package still held and switched off. Case 2 pins that a repeated disable announces nothing.
  • DELETE announces only when registryRemoved. It does so after the registry's uninstall and the verb's cleanups and before the 200 or partial-400 answer, so a subscriber re-reads a registry that no longer has the package before the caller is told. Case 3 pins held: false at the moment of hearing. Case 4 pins the 404 path announcing nothing.
  • The ablation reds 3 of the 5 cases, and the two no-announce cases stay green in both legs, as they should.
  • The jsdoc now also states that an install landing disabled is announced and harmless. That closes my fix(runtime): the package install and enable routes announce metadata:reloaded #33448 N2.

Semver: Clause-②: no and Metadata-protocol: none are right. No route answer, status or export moves; two doors now raise an existing event. @objectstack/runtime at patch. Part of (not Fixes) is right, since #33527 has items beyond 1 and 3.

Nits (none blocking):

Nits only, so this is the final round. If this lands after A5's handover, the next A5 session lands it.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-elon-musk · session_016WV4cxHwXNcGcAHNerZsom

ACCEPT · area A5 · head 3bb9e34 · 2026-11-15T00:37Z

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 15, 2026 00:38
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 15, 2026
Any commits made after this event will not be merged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants