fix(agent): respect explicit accessPolicy="public" over allowlist heuristic (forward-port #873 → main)#897
Merged
Conversation
This was referenced Jun 1, 2026
…ristic (#865) Pre-fix, `DKGAgent.isPrivateContextGraph` fell through to an allowlist heuristic whenever the explicit accessPolicy triple was anything other than "private". The moment any `DKG_ALLOWED_AGENT` / `DKG_ALLOWED_PEER` quad landed in `_meta` (via inviteToContextGraph / inviteAgentToContextGraph), the function flipped the CG to private even when the curator had explicitly chosen `accessPolicy="public"` in the Create Project modal. That mis-classification then routed the next VM publish through the LU-5 / LU-11 curated path, which hangs waiting for V2 ACKs from invitees that never arrive on a CG cores treat as public — the symptom the user reports on the rc.12 edge node ("publish never submits, no TRAC spent, no in-product recovery"). Fix: isPrivateContextGraph now resolves the explicit policy first. "public" returns false; "private" returns true; absent/unknown falls through to the legacy allowlist heuristic (preserved so discovered-via- gossip CGs without an ontology bootstrap still classify correctly). Also adds a `warnIfAllowlistWriteOnPublicCg` observability hook on the two invite write paths so operators get a clear breadcrumb when an allowlist lands on a public CG (legitimate for publishPolicy=curated + accessPolicy=public, but worth surfacing so the next publisher behavior is unsurprising). Test updates: - New `test/issue-865-public-cg-allowlist.test.ts` — 4 cases covering explicit-public + agent invite, explicit-public + peer invite, explicit-private stays curated, and legacy fallback for CGs without explicit accessPolicy. - `test/agent.test.ts` "maps local access policy to EVM publish policy" — the `register-agent-allowlist-policy` case used to silently rely on the "invite auto-promotes to curated" inference this fix removes. Updated to express curated intent up-front via `accessPolicy: 1` so the test still asserts the original on-chain contract. Co-authored-by: Cursor <cursoragent@cursor.com>
Two follow-ups from the Codex review on #873: 1. Extract `getExplicitAccessPolicy()` — both `isPrivateContextGraph` (read-path routing) and `warnIfAllowlistWriteOnPublicCg` (write-path observability) were running the same SPARQL SELECT against ONTOLOGY + cgMeta for the `DKG_ACCESS_POLICY` triple, with the same literal-string comparisons (`"public"` / `"private"`). Codex flagged that if the policy-resolution rules change again, the warn can drift from the routing logic and silently mislead operators. New private helper is the single source of truth for "what does this CG's explicit accessPolicy actually say"; both consumers go through it. Returns `'public' | 'private' | null`, where `null` is the "no explicit policy" signal both callers already needed. 2. Move `warnIfAllowlistWriteOnPublicCg` AFTER the idempotency early-return in `inviteToContextGraph` (peer path). Pre-fix, the warn fired BEFORE the `existingAllowlist?.includes(peerId)` check — re-inviting a peer that was already allowed logged "writing allowlist quad" even though the method returned without inserting anything, polluting operators' audit trails with phantom writes. New ordering only emits the warn when an actual insert is about to happen. Code comment pins the contract so a future refactor doesn't accidentally move it back. Test additions: - New `test/issue-865-public-cg-allowlist.test.ts` case ("warn must not fire on no-op peer re-invite") uses `Logger.setSink` to capture warn lines, calls `inviteToContextGraph` twice with the same peer on a public CG, asserts the public-CG warning is logged EXACTLY ONCE (first call inserts, second is idempotent and silent). Co-authored-by: Cursor <cursoragent@cursor.com>
…, codex line 14061) Sibling fix to the round-2 peer-path commit (2ecd15b). Codex's third finding lived on the agent path: `inviteAgentToContextGraph` had no idempotency early-return at all AND emitted the public-CG warn before deciding whether the allowlist was actually growing. Re-inviting an already-allowed agent therefore re-pushed the duplicate DKG_ALLOWED_AGENT triple (oxigraph dedupes the quad but the side effects — `upsertContextGraphMember`, `store.insert`, the warn line itself — all still fired) and logged a phantom "writing allowlist" breadcrumb. @branarakic empirically reproduced this against the patched daemon (combined #873 + #874 worktree at `704b49cf`) — the smoke test in the PR description's table shows the second `add-participant` for the same agent address firing the warn a second time even though no new quad lands. Fix mirrors the peer-path branch at ~line 13967: * Hoist `getPrivateContextGraphParticipants(...)` to the top of the method so we know `alreadyAllowed` before any side effect. * Early-return when `alreadyAllowed && !delegation` (no-op re-invite case — the only thing that runs is `upsertContextGraphMember` for bookkeeping parity with the peer path, plus an info log). * When a delegation IS supplied, fall through but skip both the warn AND the duplicate DKG_ALLOWED_AGENT push — only the delegation node is overwritten, which is the documented "re-approve refreshes the delegation" semantics from the existing comment at the delegation block. * When `!alreadyAllowed`, behavior is unchanged: warn + push allowlist quad + push delegation quads (if any) + insert. Pinning test mirrors the round-2 peer-path test: `inviteAgentToContextGraph` re-invite of an already-allowed agent on a public CG fires the warn exactly once across two calls. All 6 tests in `issue-865-public-cg-allowlist.test.ts` pass; adjacent `agent.test.ts > scopes CG management` (which exercises the non-default-agent invite path) also still passes. Co-authored-by: Cursor <cursoragent@cursor.com>
…x round 4 line 17475) Codex round-4 finding on `2003015a`: `warnIfAllowlistWriteOnPublicCg` fires BEFORE the subsequent `store.insert()` is awaited, but the message reads "writing allowlist on context graph X / The allowlist quad is being persisted" — past/present tense that claims the write landed. If the insert throws transiently, the operator audit log has a phantom breadcrumb claiming a public-CG allowlist write happened when it didn't. Picking option (b) from Codex's two suggestions (reword vs. defer): deferring would lose the public-CG breadcrumb on insert-failure paths, where the operator most needs to see it. Rewording keeps the breadcrumb informative under both success and failure, and any thrown insert error surfaces separately so there's no contradiction: - `writing allowlist on context graph` → `about to write allowlist on context graph` - `The allowlist quad is being persisted` → `The allowlist quad will be persisted` The literal-substring filters in `issue-865-public-cg-allowlist.test.ts` key on `accessPolicy="public"` and the `operation` prefix only — no test pinned the previous wording so the 6/6 cases still pass without modification. Co-authored-by: Cursor <cursoragent@cursor.com>
, codex round 5) Codex round 4 (line 17475) flagged that the warn fired before the awaited `store.insert()` ran — phantom breadcrumb on insert failure. Round 5 (line 17485) flagged that the round-4 reword to pre-write tense ("will be persisted") still implied a definitive outcome the caller couldn't yet guarantee. Fix moves the warn from pre-insert to post-insert in both call sites. A failing `store.insert` throws to the caller before the warn runs, so the breadcrumb is now a faithful record of persisted state and the wording can be past-tense: - `writing allowlist on context graph` → `wrote allowlist quad on context graph` - `The allowlist quad is being persisted` → `The persisted quad` Two call-site moves: 1. `inviteToContextGraph` (peer path, ~line 13988): warn moved from above `quadsToInsert.push` + `store.insert` to immediately after the awaited insert, before the `upsertContextGraphMember` bookkeeping (which is a local-only side effect, irrelevant to the audit semantics). 2. `inviteAgentToContextGraph` (agent path, ~line 14093): warn moved from above the `quadsToInsert` build loop to immediately after the awaited insert. Idempotency gate `if (!alreadyAllowed)` preserved from round 3 — re-approves that only refresh a delegation don't grow the allowlist, so they don't warrant the public-CG breadcrumb. Doc comment on `warnIfAllowlistWriteOnPublicCg` updated to enumerate all three Codex constraints (round 1: idempotency, round 4: pre-insert phantom, round 5: tense vs. realized state) that converge on the post-insert call site. The 6 cases in `issue-865-public-cg-allowlist.test.ts` filter on `accessPolicy="public"` + the operation prefix only, so they keep passing. No new tests needed: the round-3 pinning test already covers the no-op-doesn't-warn semantics, and the post-insert move strictly tightens the contract (warn ↔ persisted state). Co-authored-by: Cursor <cursoragent@cursor.com>
branarakic
force-pushed
the
forward-port/865-public-cg-allowlist-to-main
branch
from
cb5c2fd to
8ebad42
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
| this.assertCallerIsOwner(owner, callerAgentAddress, 'manage invitations'); | ||
|
|
||
| const existingParticipants = await this.getPrivateContextGraphParticipants(contextGraphId); |
There was a problem hiding this comment.
🔴 Bug: getPrivateContextGraphParticipants() includes DKG_PARTICIPANT_AGENT entries as well as the local DKG_ALLOWED_AGENT allowlist. That makes the new idempotency check treat an agent who only exists in participantAgents (registration metadata) as already invited, so inviteAgentToContextGraph() can return early without ever writing the local allowlist quad. The result is a curated CG that looks invited on-chain but still denies the agent local access. Please base alreadyAllowed on the actual local allowlist (or otherwise exclude participant-only entries from this early-return).
3 tasks
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.
Forward-port of #873 (merged to
release/rc.12at872f8cb4) ontomain. No code differences vs. the rc.12 commits — clean cherry-pick.Why this needs a separate PR
The original PR was opened against
release/rc.12and merged there.maindoesn't auto-pull from rc.12, so the public-CG / allowlist-heuristic fix needs to land here independently before it ships in any non-rc.12 release.What's included
5 commits cherry-picked in original order from #873:
92c3aa8900014de0fix(agent): respect explicit accessPolicy="public" over allowlist heuristic (#865)— primary fix:isPrivateContextGraphnow lets explicitaccessPolicywin over the allowlist heuristic, fixing the rc.12 publish-stall regression on public-but-curated CGs2ecd15bbc2f5ce3arefactor(agent): address Codex review on #873— round 2: extract sharedgetExplicitAccessPolicy()helper consumed by bothisPrivateContextGraph(read-path) andwarnIfAllowlistWriteOnPublicCg(write-path observability) so they can't drift2003015a187103c2fix(agent): inviteAgentToContextGraph idempotency + warn ordering (#873, codex line 14061)— round 3: hoist participants lookup, addalreadyAllowed && !delegationearly-return, mirror peer-path's structurebed4c250cf45eccefix(agent): pre-write warn wording on public-CG allowlist (#873, codex round 4 line 17475)— round 4: pre-write tense to avoid phantom-write breadcrumbs (superseded by round 5 below — kept for review history)53442582cb5c2fdbfix(agent): defer public-CG warn until after store.insert succeeds (#873, codex round 5)— round 5: warn deferred to after the awaited insert in both call sites; past-tense wordingConflict resolution
None. All 5 commits cherry-picked clean against current main HEAD. Files touched:
packages/agent/src/dkg-agent.ts,packages/agent/test/issue-865-public-cg-allowlist.test.ts.git diff origin/release/rc.12 -- <touched files>returns empty — byte-identical to the rc.12 state.Test plan
pnpm vitest run test/issue-865-public-cg-allowlist.test.ts— 6/6 pass on rc.12, expect same heremainTornado: core + storage + chainpasses (was the suspect upstream regression from PR fix: harden context graph registration RPC handling #883's wrap; should be unaffected here since the patch doesn't touch chain code)Ref: original PR #873, issue #865.
Made with Cursor