Conversation
…n add_active_agent_link
… for duplicated agents
WalkthroughAdds 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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 makesadd_active_agent_linkeffectively idempotent; confirm concurrency assumptionsThe new flow (querying
Indexlinks with tag"active_agent"onagent_root_hashand scanning targets for the currentagent) 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 eithercreate_linkcommits. 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 inget_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
📒 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 inget_otherscleanly addresses the duplicate display issueImporting
dedupand applying it once after collecting all DIDs ensuresget_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
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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.shbootstrap-languages/direct-message-language/hc-dna/build.shbootstrap-languages/p-diff-sync/hc-dna/build.shbootstrap-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 thatGetStrategy::Localis 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:
- Active agent links created by this agent are immediately available locally (expected behavior)
- Re-initialization scenarios wait for gossip to complete before calling this function
- This is acceptable given the primary fix handles deduplication at query time
If stronger guarantees are needed, consider using
GetStrategy::Networkor a more comprehensive retrieval strategy.rust-executor/src/mainnet_seed.json (1)
7-7: DID is consistently used across seed files.The DID
QmzSYwdp1nkvMSU4DiDeGsqfAjYBsMXsKwB2fFR7XiXNodiR4BFin theknownLinkLanguagesarray appears in bothrust-executor/src/mainnet_seed.jsonandcli/mainnet_seed.jsonat line 7 with identical values. The IPFS CID format is valid. Ensure this DID references the intended language resource through external verification.
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:
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:get_others()returning duplicate DIDs in peer listsCommon scenarios that trigger this:
Solution
Primary Fix: Add deduplication in
get_others()(status.rs)Secondary Fix: Prevent duplicate links in add_active_agent_link() (commit.rs)
Impact
Files Changed
status.rscommit.rsSummary by CodeRabbit
Bug Fixes
Chores
✏️ 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.
link_adapter/commit.rs: Check for existingLinkTypes::Indexwith tagactive_agentbeforecreate_linkto prevent duplicates.telepresence/status.rs: Importdedupand apply toget_others()to return unique DID list.cli/mainnet_seed.json,rust-executor/src/mainnet_seed.json: UpdateknownLinkLanguagesCID.bootstrap-languages/*/hc-dna/build.sh: Chaincargo buildwithhc dna pack(andhc app packwhere applicable) in a single command.Written by Cursor Bugbot for commit ca7f9e2. This will update automatically on new commits. Configure here.