Skip to content

Duplicate agent fix#641

Merged
lucksus merged 6 commits intodevfrom
duplicate-agent-fix
Dec 9, 2025
Merged

Duplicate agent fix#641
lucksus merged 6 commits intodevfrom
duplicate-agent-fix

Conversation

@jhweir
Copy link
Contributor

@jhweir jhweir commented Dec 4, 2025

Fix: Deduplicate agents in get_others() to prevent duplicate peer display

Problem
Users were seeing duplicate agents with the same DID in neighbourhood peer lists. This could result from various scenarios, including:

  • Accessing neighbourhoods from different app versions (localhost, deploy previews, production)
  • DNA re-initialization or restart scenarios
  • Concurrent DNA initialization from the same agent

Root Cause
The add_active_agent_link() function unconditionally created a new active agent link every time it was called during DNA initialization. This could result in:

  • Multiple agent pub keys being created for the same agent in the DHT
  • All these agent pub keys mapping to the same DID
  • get_others() returning duplicate DIDs in peer lists

Common scenarios that trigger this:

  • Multi-version DNA access: Different app versions use different DNA hashes, each creating its own agent link
  • DNA restarts/reinitialization: The init() callback runs again, creating duplicate links
  • Race conditions: Multiple initialization attempts creating concurrent links

Solution
Primary Fix: Add deduplication in get_others() (status.rs)

  • Added dedup() call to filter duplicate DIDs at query time
  • This ensures only unique DIDs are returned, regardless of how many agent pub keys exist in the DHT
  • Works for both existing and new neighbourhoods
  • Solves all duplicate scenarios at the query level

Secondary Fix: Prevent duplicate links in add_active_agent_link() (commit.rs)

  • Check if an active agent link already exists before creating a new one
  • Prevents duplicate links within the same DNA instance during re-initialization
  • Protects against restart and race condition bugs
  • Defensive programming for edge cases

Impact

  • ✅ Fixes duplicate agent display in all scenarios
  • ✅ Works with existing neighbourhoods immediately (through dedup)
  • ✅ Prevents new duplicates in future neighbourhoods (through link check)
  • ✅ Handles multi-version DNA access, restarts, and race conditions gracefully
  • ⚠️ Note: Existing duplicate links remain in the DHT but are filtered out at query time

Files Changed
status.rs

  • Import dedup utility
  • Apply deduplication to get_others() results

commit.rs

  • Add existence check in add_active_agent_link()
  • Skip link creation if agent link already exists

Summary by CodeRabbit

  • Bug Fixes

    • Prevented duplicate agent links by checking for existing links before creating new ones.
    • Ensured agent status lists return unique identifiers to avoid redundant entries.
  • Chores

    • Consolidated multiple build steps into chained, fail-fast commands for packaging scripts.
    • Updated known language/link identifiers in seed/config data.

✏️ Tip: You can customize this high-level summary in your review settings.


Note

Deduplicates peers and avoids duplicate active-agent links in p-diff-sync, updates mainnet knownLinkLanguages CID, and streamlines DNA build scripts.

  • p-diff-sync:
    • link_adapter/commit.rs: Check for existing LinkTypes::Index with tag active_agent before create_link to prevent duplicates.
    • telepresence/status.rs: Import dedup and apply to get_others() to return unique DID list.
  • Seeds:
    • cli/mainnet_seed.json, rust-executor/src/mainnet_seed.json: Update knownLinkLanguages CID.
  • Build scripts:
    • bootstrap-languages/*/hc-dna/build.sh: Chain cargo build with hc dna pack (and hc app pack where applicable) in a single command.

Written by Cursor Bugbot for commit ca7f9e2. This will update automatically on new commits. Configure here.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Walkthrough

Adds duplicate-prevention when creating an active_agent link, deduplicates collected agent DIDs in telepresence status, normalizes multiple build scripts to chained fail-fast commands, and updates two mainnet_seed.json language link entries. No public API signatures changed.

Changes

Cohort / File(s) Summary
Link creation duplicate-prevention
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs
Replaced unconditional creation of the active_agent link with a flow that computes agent_root_hash, queries existing LinkRecords tagged active_agent, checks targets for the current agent, and only calls create_link if no existing link is found; logs and skips creation when present.
DID list deduplication
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/telepresence/status.rs
Imported and applied a dedup utility in get_others to ensure the returned list of DIDs is unique when multiple agent keys map to the same DID.
Chained fail-fast build scripts
bootstrap-languages/*/hc-dna/build.sh, bootstrap-languages/file-storage/hc-dna/build.sh, bootstrap-languages/p-diff-sync/hc-dna/build.sh, bootstrap-languages/agent-language/hc-dna/build.sh, bootstrap-languages/direct-message-language/hc-dna/build.sh
Replaced separate sequential commands with single chained commands using && so subsequent packaging steps run only if the preceding build step succeeds (fail-fast behavior).
Mainnet seed language link updates
cli/mainnet_seed.json, rust-executor/src/mainnet_seed.json
Updated entries in knownLinkLanguages (replaced one DID/hash in each JSON file). No other structural changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review link query logic and matching criteria in link_adapter/commit.rs for correctness and potential race conditions (query-then-create).
  • Verify dedup implementation in telepresence/status.rs correctly preserves intended semantics and order if relevant.
  • Confirm build script chaining matches intended CI/developer workflow and that error behavior is acceptable.
  • Quick sanity-check of the updated JSON values.

Poem

🐰 I hopped through code with cautious feet,

deduped the links and pruned repeat.
Builds now halt if earlier steps fail,
seeds refreshed with a different trail.
— a rabbit cheering tidy rails ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Duplicate agent fix' directly and accurately summarizes the primary purpose of the PR, which is to fix duplicate agent entries in neighbourhood peer lists.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch duplicate-agent-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs (1)

125-155: Active‑agent link existence check makes add_active_agent_link effectively idempotent; confirm concurrency assumptions

The new flow (querying Index links with tag "active_agent" on agent_root_hash and scanning targets for the current agent) correctly avoids creating additional active‑agent links for the same agent on this cell across restarts/re‑initialization, matching the PR’s intent to stop link proliferation at write time.

The only remaining edge case is a theoretical race if add_active_agent_link() could somehow be invoked concurrently in the same cell: both callers might see no existing link before either create_link commits. If you’re confident this function only runs once per DNA init / not concurrently, the current approach is sufficient; otherwise, duplicates from that narrow window would still be possible (though now also mitigated at read time by the DID de‑dup in get_others).

If you expect any concurrent invocations, it might be worth documenting that assumption at the call site or here in a comment.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 078be9d and b5d95c7.

📒 Files selected for processing (2)
  • bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs (1 hunks)
  • bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/telepresence/status.rs (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-03T15:16:29.623Z
Learnt from: lucksus
Repo: coasys/ad4m PR: 619
File: rust-executor/src/perspectives/perspective_instance.rs:639-641
Timestamp: 2025-09-03T15:16:29.623Z
Learning: In the AD4M codebase, database operations like `add_many_links` during critical synchronization processes (such as `diff_from_link_language`) should use `.expect()` rather than error logging, as failures indicate severe system issues where fail-fast behavior is preferred over continuing with potentially corrupted state.

Applied to files:

  • bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs
🧬 Code graph analysis (1)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs (4)
test-runner/example/languages/social-context.js (1)
  • LinkQuery (4016-4054)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/retriever/mock.rs (1)
  • new (240-277)
executor/src/holochain_types.ts (1)
  • AgentPubKey (8-8)
rust-executor/src/perspectives/perspective_instance.rs (1)
  • create_link (3368-3374)
🔇 Additional comments (1)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/telepresence/status.rs (1)

9-9: DID de‑duplication in get_others cleanly addresses the duplicate display issue

Importing dedup and applying it once after collecting all DIDs ensures get_others() now returns a unique list of DIDs even when multiple agent keys resolve to the same DID, without touching the surrounding logic or error handling. This aligns with the PR’s stated behavior change and looks good.

Also applies to: 169-180

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5d95c7 and ca7f9e2.

📒 Files selected for processing (7)
  • bootstrap-languages/agent-language/hc-dna/build.sh (1 hunks)
  • bootstrap-languages/direct-message-language/hc-dna/build.sh (1 hunks)
  • bootstrap-languages/file-storage/hc-dna/build.sh (1 hunks)
  • bootstrap-languages/p-diff-sync/hc-dna/build.sh (1 hunks)
  • bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs (1 hunks)
  • cli/mainnet_seed.json (1 hunks)
  • rust-executor/src/mainnet_seed.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • cli/mainnet_seed.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-04T11:13:54.337Z
Learnt from: lucksus
Repo: coasys/ad4m PR: 605
File: bootstrap-languages/p-diff-sync/happ.js:1-5
Timestamp: 2025-06-04T11:13:54.337Z
Learning: In Holochain projects, .happ (Holochain app bundle) files are build-time artifacts generated by running "hc app pack workdir" in build scripts, not committed to the repository. The build process creates these files which are then imported by modules like happ.js.

Applied to files:

  • bootstrap-languages/file-storage/hc-dna/build.sh
  • bootstrap-languages/direct-message-language/hc-dna/build.sh
  • bootstrap-languages/p-diff-sync/hc-dna/build.sh
  • bootstrap-languages/agent-language/hc-dna/build.sh
📚 Learning: 2025-06-05T11:39:48.641Z
Learnt from: lucksus
Repo: coasys/ad4m PR: 605
File: rust-executor/src/js_core/options.rs:59-68
Timestamp: 2025-06-05T11:39:48.641Z
Learning: In the rust-executor project, CUSTOM_DENO_SNAPSHOT.bin is a build-time generated artifact created by the generate_snapshot binary (with --features generate_snapshot). The file is not checked into source control but is created during the build process via pnpm build-deno-snapshot script and then included in subsequent builds via include_bytes! macro.

Applied to files:

  • bootstrap-languages/file-storage/hc-dna/build.sh
📚 Learning: 2025-09-03T15:16:29.623Z
Learnt from: lucksus
Repo: coasys/ad4m PR: 619
File: rust-executor/src/perspectives/perspective_instance.rs:639-641
Timestamp: 2025-09-03T15:16:29.623Z
Learning: In the AD4M codebase, database operations like `add_many_links` during critical synchronization processes (such as `diff_from_link_language`) should use `.expect()` rather than error logging, as failures indicate severe system issues where fail-fast behavior is preferred over continuing with potentially corrupted state.

Applied to files:

  • bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs
🧬 Code graph analysis (1)
bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs (2)
test-runner/example/languages/social-context.js (1)
  • LinkQuery (4016-4054)
rust-executor/src/perspectives/perspective_instance.rs (1)
  • create_link (3368-3374)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cursor Bugbot
🔇 Additional comments (6)
bootstrap-languages/direct-message-language/hc-dna/build.sh (1)

2-2: Good: fail-fast build pattern applied.

Chaining commands with && ensures the build stops immediately if cargo build or hc dna pack fails, rather than continuing to hc app pack. This is an improvement over separate commands.

bootstrap-languages/agent-language/hc-dna/build.sh (1)

2-2: Good: fail-fast build pattern applied consistently.

The chained commands with && ensure sequential execution with early termination on failure, consistent with other language build scripts in this PR.

bootstrap-languages/file-storage/hc-dna/build.sh (1)

2-2: Good: fail-fast pattern applied. Confirm intentional 2-step difference.

The chained commands with && are an improvement. However, file-storage only chains cargo build and hc dna pack (2 steps), while agent-language and direct-message-language also include hc app pack (3 steps). Please confirm this difference is intentional—i.e., file-storage only needs to generate the .dna artifact and not a .happ bundle.

bootstrap-languages/p-diff-sync/hc-dna/build.sh (1)

2-2: Good: fail-fast build pattern applied consistently.

The three chained commands with && ensure sequential execution with early termination on failure, consistent with other language build scripts in this PR.

bootstrap-languages/p-diff-sync/hc-dna/zomes/perspective_diff_sync/src/link_adapter/commit.rs (1)

133-133: Verify that GetStrategy::Local is sufficient for this check.

The query uses GetStrategy::Local, which only checks locally-stored data and does not query the broader DHT. If an active agent link exists on the network but hasn't been gossiped to this node yet (e.g., during rapid re-initialization or after local data clearing), this check will miss it and create a duplicate.

Confirm whether:

  1. Active agent links created by this agent are immediately available locally (expected behavior)
  2. Re-initialization scenarios wait for gossip to complete before calling this function
  3. This is acceptable given the primary fix handles deduplication at query time

If stronger guarantees are needed, consider using GetStrategy::Network or a more comprehensive retrieval strategy.

rust-executor/src/mainnet_seed.json (1)

7-7: DID is consistently used across seed files.

The DID QmzSYwdp1nkvMSU4DiDeGsqfAjYBsMXsKwB2fFR7XiXNodiR4BF in the knownLinkLanguages array appears in both rust-executor/src/mainnet_seed.json and cli/mainnet_seed.json at line 7 with identical values. The IPFS CID format is valid. Ensure this DID references the intended language resource through external verification.

@lucksus lucksus merged commit e997730 into dev Dec 9, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants