Skip to content

fix: correct local query and curate messaging - #668

Open
strasserwm wants to merge 5 commits into
campfirein:mainfrom
strasserwm:codex/byterover-query-curate-fixes-verify
Open

strasserwm wants to merge 5 commits into
campfirein:mainfrom
strasserwm:codex/byterover-query-curate-fixes-verify

Conversation

@strasserwm

Copy link
Copy Markdown

Summary

Fixes two local ByteRover query/curate UX issues:

  1. Non-ByteRover provider 401 / Unauthorized failures were shown as cloud-sync login failures, telling users to run brv login even when local query/curate only needed provider credentials.
  2. brv curate could print Context curated successfully for blocked/no-op curation responses where no context-tree operations were applied.

Changes

  • Keep brv login guidance for true ByteRover cloud auth / NOT_AUTHENTICATED cases.
  • For non-ByteRover provider auth failures, show provider credential/base URL guidance and avoid echoing raw error text.
  • Add curate outcome classification for blocked/no-op responses.
  • Treat blocked curate responses as errors instead of successful completions.
  • Record blocked zero-op curation logs as error.
  • Print No context changes applied for genuine successful no-op curation.
  • Add/update focused tests for auth-message and curate-outcome behavior.

Verification

Performed locally:

  • git apply --check for Phase 1 patch
  • git apply --check for Phase 2 patch
  • git apply --check for combined patch
  • git diff --check
  • Phase 1 focused tests previously reported passing: 9 passing

Could not fully verify locally because this checkout lacks private/incomplete dependencies:

  • mocha is missing locally
  • npm install / full dependency repair is blocked by private github.com/campfirein/byterover-packages
  • tsc --noEmit reports repo-wide missing dependency/type errors unrelated to this patch set

Recommended maintainer verification with private deps available:

npm ci
npx mocha --forbid-only \
  "test/unit/oclif/lib/daemon-client-error.test.ts" \
  "test/commands/curate.test.ts" \
  "test/unit/infra/executor/curate-executor.test.ts" \
  "test/unit/infra/process/curate-log-handler.test.ts"
npm run typecheck
npm test

Risk notes

  • Provider auth-message change is low risk; main watch item is whether provider context is always available where the CLI formats daemon errors.
  • Curate outcome change is medium risk; main watch items are blocked-text false positives and any downstream assumptions that task completion always means curation success.

@bao-byterover bao-byterover added the safe-to-test Authorize CI to run against this fork PR. Auto-removed on each push. label May 19, 2026
@bao-byterover

Copy link
Copy Markdown
Collaborator

Hey @strasserwm could you merge main into this branch when you get a chance? There've been a few updates upstream, and syncing will keep CI green and make review easier. Thanks!

…ry-curate-fixes-verify

# Conflicts:
#	src/oclif/commands/query.ts
@github-actions github-actions Bot removed the safe-to-test Authorize CI to run against this fork PR. Auto-removed on each push. label May 20, 2026
@strasserwm

strasserwm commented May 20, 2026 via email

Copy link
Copy Markdown
Author

@leehpham leehpham added the safe-to-test Authorize CI to run against this fork PR. Auto-removed on each push. label May 21, 2026

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

safe-to-test Authorize CI to run against this fork PR. Auto-removed on each push.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants