Skip to content

Conversation

@seefeldb
Copy link
Contributor

@seefeldb seefeldb commented Sep 8, 2025

Summary by cubic

Downgraded logging for expected “already exists” conflicts in storage to info by checking conflict.expected == null, reducing noisy error logs. Also cleaned up TheConflictError message formatting with no behavior change.

@seefeldb seefeldb requested review from Copilot and ubik2 September 8, 2025 18:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates conflict error handling to log "already in history" conflicts as informational messages rather than errors. The change modifies the condition that determines when conflicts should be treated as expected/informational.

  • Changes the condition from checking existsInHistory to checking if expected is null
  • Removes unnecessary line breaks in error message formatting

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/runner/src/storage/cache.ts Updates conflict detection logic to use expected == null condition
packages/memory/error.ts Cleans up formatting by removing unnecessary line breaks in error message construction

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

result.error.conflict.existsInHistory
result.error.conflict.expected == null
) {
logger.info(() => ["Transaction failed (aready exists)", result.error]);
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

There's a typo in the log message: 'aready' should be 'already'.

Suggested change
logger.info(() => ["Transaction failed (aready exists)", result.error]);
logger.info(() => ["Transaction failed (already exists)", result.error]);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

@ubik2
Copy link
Contributor

ubik2 commented Sep 8, 2025

I had some confusion in discord, but perhaps this is viable.
I suspect it's still not what's desired.

Copy link
Contributor

@ubik2 ubik2 left a comment

Choose a reason for hiding this comment

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

I think we're abandoning this, but don't want to close your PR.

@seefeldb seefeldb closed this Oct 17, 2025
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.

3 participants