Skip to content

fix(recall): honor --dry-run for feedback (#900) - #965

Open
zszz3 wants to merge 1 commit into
Tencent:mainfrom
zszz3:codex/recall-feedback-dry-run
Open

zszz3 wants to merge 1 commit into
Tencent:mainfrom
zszz3:codex/recall-feedback-dry-run

Conversation

@zszz3

@zszz3 zszz3 commented Oct 2, 2026

Copy link
Copy Markdown

Summary

  • Make recall feedback --positive/--negative --dry-run preview the requested feedback instead of recording a vote.
  • Forward dry-run through config resolution and error diagnosis, so legacy config migration stays read-only; return before vote loading, migration or locking.
  • Add real CLI regressions and sync the bilingual usage guides, data-layout design and agent troubleshooting reference.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Test Plan

Local macOS, Node 26.7.0:

  • npm run build
  • npx tsc --noEmit
  • npm run lint
  • Full unit suite with npm 11.6.0: npm exec --yes --package=npm@11.6.0 -- node node_modules/vitest/vitest.mjs run --maxWorkers=4 — 370 files passed; 7,172 tests passed, 1 skipped.
  • npx vitest run commands-reference -u — passed; generated reference unchanged because this reuses the existing global option.
  • npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/recall-feedback-dry-run.test.ts --retry=0 — 13 passed.
  • git diff --cached --check

The E2E tests launch the built dist/index.js in isolated user/project scopes. Positive and negative previews work with the flag before or after the command, preserving config, votes and directory inventories. Legacy votes stay unchanged, missing votes are not created, and an unreadable project config still fails rather than falling back to user scope. Without the flag, votes change from 2 to 3 and back to 2 in the selected scope. Ordinary diagnostic logging is excluded from the filesystem comparison.

Before the runtime fix, the new test file had 10 failures and 3 passes on upstream b0ce1f2; all 13 pass with the fix. The first full-suite run under npm 12 hit the existing package-content test's assumption that npm pack --json returns an array. Re-running under npm 11 passed; that unrelated test was not changed.

Related Issues

Refs #900 (D: recall feedback only; the other dry-run items remain open).

Notes for Reviewers

The preview reports the requested action, not a predicted vote count. Existing vote storage, locking and normal feedback behavior are unchanged.

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.

1 participant