Skip to content

cache: continue merging the rest of entities on failure - #1616

Open
MichaelMure wants to merge 1 commit into
trunkfrom
bugfixx
Open

MichaelMure wants to merge 1 commit into
trunkfrom
bugfixx

Conversation

@MichaelMure

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Identity merging still returns early after a per-identity failure, preventing later identities from being processed.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates cache pull handling to continue merging later entities after an earlier merge failure while preserving the first error.

Changes:

  • Drain merge-result channels after failures.
  • Continue processing remaining entity and cache merges.
  • Add regression coverage for failures followed by later merges.
File Summary
entity/​dag/​entity_actions.go Drains entity merge results after failure.
entities/​identity/​identity_actions.go Identity merging can still stop after a per-identity error; changes are required.
cache/​repo_cache_test.go Tests merging after an identity failure.
cache/​repo_cache_common.go Continues merging remaining cache subentities.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +34 to +37
if failure != nil {
// keep reading, so that the other identities are merged and
// MergeAll's goroutine doesn't block forever
continue

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants