Skip to content

chore(authz): remove legacy topic-memory owner rows at startup - #1849

Merged
Teingi merged 2 commits into
oceanbase:masterfrom
Lkx-JY:chore/1848-remove-legacy-topic-memory-owners
Oct 5, 2026
Merged

Teingi merged 2 commits into
oceanbase:masterfrom
Lkx-JY:chore/1848-remove-legacy-topic-memory-owners

Conversation

@Lkx-JY

@Lkx-JY Lkx-JY commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1848

Rationale for this change

Topic Memory is Scope-owned: it registers no Artifact Family Access Profile and
never establishes a per-topic Artifact owner. The pre-#1794 Worker nevertheless
called establish_artifact_owner() on the create branch of its Topic
publication, so every Topic created by an older version left a row in
pc_access_owners with owner_kind = 'artifact' and family = 'topic-memory'.

#1794 removed that call, so no new rows are written, but rows persisted by older
versions remain.

These rows are not inert. resolve_resource_filter reads ownership through the
decision snapshot (list_owned_resources filters only by owner, not by family),
and _derive_authorized_resource_filter merges those resources into
AuthorizedResourceFilter.exact_resources. A legacy row therefore hands its
owner an exact authorized entry for a resource whose family is Scope-owned and
is not supposed to have an owner at all.

What changes are included in this PR?

  • Add RelationalAccessRepository.delete_legacy_topic_memory_owners(), which
    deletes only legacy Topic Memory Artifact owner rows:

    DELETE FROM pc_access_owners
    WHERE owner_kind = 'artifact' AND family = 'topic-memory';

    The owner_kind predicate keeps this from ever matching candidate attestation
    rows (owner_kind = 'candidate'), which serve other families.

  • The deletion is a relationship mutation, so it respects the protocol the rest
    of this table already follows. It takes the shared pc_access_policy_heads
    row lock before touching owner rows (the same order as
    establish_artifact_owner), and it increments the policy revision so that the
    revision-stability check behind decision_snapshot can observe the change
    instead of accepting a torn snapshot. When there is nothing to delete, no
    revision is incremented, so an ordinary restart cannot invalidate outstanding
    signed cursors.

  • Run it once at Server startup, next to the existing receipt migration, via a
    guarded helper _remove_legacy_topic_owners(). It only acts while a real
    service is configured and the access tables are open, and it logs
    server.topic_owner_cleanup with the removed count when it deletes something.

  • Tests:

    • A repository-level regression test that seeds a legacy Topic Memory owner, a
      normal Skill owner, and an Experience candidate attestation through the real
      writer APIs, then asserts the first is removed, the other two survive, the
      policy revision moves for the deletion, and a repeated run both removes
      nothing and leaves the revision untouched.
    • A startup regression test that seeds the same relations, restarts the Server
      against the same database file, and asserts the legacy row is gone from
      pc_access_owners while the other two remain (and that a further restart
      changes nothing).

Are there any user-facing changes?

  • No API change. The one observable behavior change is the intended one: a
    legacy Topic Memory owner row no longer puts that resource into its owner's
    authorized resource list.

  • Deployments that ran an older version are cleaned at the next startup. A
    deployment currently running with Access Control disabled can still carry the
    rows, because the access tables are only open while a service is configured;
    the equivalent operator statements are:

    DELETE FROM pc_access_owners
    WHERE owner_kind = 'artifact' AND family = 'topic-memory';

How was this change tested?

  • pytest tests/test_access_control.py -q (16 passed)
  • pytest tests/test_access_control.py \ tests/e2e/test_access_control_regressions.py \ tests/e2e/test_access_control_http.py \ tests/test_access_http.py \ tests/test_server.py \ tests/test_access_adapters.py -q (137 passed)
  • pytest tests/test_processing_security.py tests/test_access_mcp.py \ tests/test_authorization.py tests/test_access_snapshot_boundary.py \ tests/test_topic_memory_server.py tests/e2e/test_artifact_dreaming.py -q
    (84 passed, 35 skipped)
  • Both new tests were verified to fail before the change. The repository test
    failed with AttributeError: 'RelationalAccessRepository' object has no attribute 'delete_legacy_topic_memory_owners'; the startup test failed with
    the legacy row still present in pc_access_owners after a restart once the
    startup call was temporarily removed, confirming it protects the wiring and
    not only the deletion.
  • make check (lock file, all prek hooks incl. ruff check/format and ty check,
    and the integration manifest suite — passed)

AI usage statement

Assisted analysis and implementation with DeepSeek Harness (deepseek-v4-pro).

Topic Memory is Scope-owned and no read path has ever consulted an
Artifact owner relation, but the pre-oceanbase#1794 Worker established one for
every newly created Topic. oceanbase#1794 stopped writing them; rows persisted
by older versions remain as dead state.

Delete them once at startup while the access tables are open. The
predicate keeps owner_kind='artifact' so candidate attestations are
never matched.

Refs oceanbase#1848
"""
async with self._database.connection(self._bound_connection) as connection:
result = await connection.execute(
delete(ACCESS_OWNERS_TABLE).where(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 删除 artifact owner 行时未推进 policy revision,绕过了同表的锁序与快照一致性协议

新增方法只执行 DELETE ... WHERE owner_kind='artifact' AND family='topic-memory',没有调用 _increment_policy_revision(connection)。而同表中其它 artifact-owner / Binding 写路径都会调用它(establish_artifact_owner 见 repository.py:485,Binding 写路径见 :710/:750/:818),并且 _increment_policy_revision 自带注释(:919-921)明确写「所有变更先取 pc_access_policy_heads 行锁」是统一锁序。

DecisionState 的一致性读取(service.py:410-421)是「读 revision → 逐个读 artifact owner → 再读 revision,未变才接受快照」,其成立前提正是 artifact owner 的任何变化都会改变 revision。删除窗口内并发读取(多副本,或滚动重启期间另一个进程在同一数据库上决策)可能拿到 revision 未变、但 owners 已被部分删除的撕裂快照,且重试逻辑不会触发。

另外 docstring 中「no read path has ever consulted one」并不成立:list_owned_resources(:579-594)会把该表 owner_kind='artifact' 的行读回,_derive_authorized_resource_filter(service.py:453-459)再把 owned_resources 并入 AuthorizedResourceFilter.exact_resources。也就是说这次删除确实改变了授权派生结果——这正是必须 bump revision 的原因。

建议:在同一事务内按既有锁序先 await self._increment_policy_revision(connection) 再删除(或明确注释为何本例免 bump),并补一条断言 policy revision 变化的测试。

…ners

Owner rows feed the decision snapshot through list_owned_resources and
the owner-derived authorized resource filter, so removing them is a
relationship mutation: take the shared policy-head lock first and
increment the revision. Skip both when there is nothing to delete, so a
restart cannot invalidate outstanding signed cursors.

Also correct the claim that no read path consults these rows.

Refs oceanbase#1848
@Teingi
Teingi merged commit 5fbd918 into oceanbase:master Oct 5, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: remove legacy topic-memory owner rows from pc_access_owners

2 participants