Skip to content

test(setup): align v0.8.67 QA script with repo constitution source#4024

Merged
Hmbown merged 1 commit into
mainfrom
codex/v0867-final-qa-script
Jul 3, 2026
Merged

test(setup): align v0.8.67 QA script with repo constitution source#4024
Hmbown merged 1 commit into
mainfrom
codex/v0867-final-qa-script

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • canonicalize CODEWHALE_BIN before the v0.8.67 setup QA script changes into temp workspaces
  • update the repo-law context assertion to match the current doctor --context-json source kind: repo_constitution

Testing

  • CODEWHALE_BIN=target/release/codewhale-tui ./scripts/v0867-setup-qa.sh
  • git diff --check

Canonicalize CODEWHALE_BIN before temp-workspace probes so relative target paths keep working after cd, and assert the current repo_constitution source kind emitted by doctor --context-json.
Copilot AI review requested due to automatic review settings July 3, 2026 22:26

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 was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Claude finished @Hmbown's task in 1m 5s —— View job


Review: v0.8.67 QA script alignment

  • Gather context (diff, full script, doctor context-json source)
  • Verify BIN canonicalization logic (REPO_ROOT usage, relative path handling)
  • Verify repo_constitution source_kind matches current Rust source
  • Check script for other correctness issues
  • Post final review

Summary: Both changes are correct and address a real bug in the QA script.

  1. BIN canonicalization (scripts/v0867-setup-qa.sh:38-40) — necessary fix. The default BIN="target/release/codewhale-tui" is repo-root-relative, but the "repo law" and "WHALE.md migration" scenarios invoke "$BIN" inside cd "$WS" && ... subshells (lines 122-123, 136-137), where $WS is a separate mktemp -d workspace. Without canonicalizing to an absolute path first, those scenarios would fail to locate the binary whenever a relative CODEWHALE_BIN (or the relative default) is in play. Absolute paths passed via CODEWHALE_BIN=/abs/path are correctly left untouched by the [[ "$BIN" != /* ]] guard.

  2. source_kind assertion (scripts/v0867-setup-qa.sh:124) — verified against crates/tui/src/context_report.rs. SourceKind::RepoConstitution (used for repo-local .codewhale/constitution.json, context_report.rs:319) serializes to "repo_constitution", matching the existing Rust test context_report_surfaces_repo_constitution_source_and_warnings (context_report.rs:834-880, which asserts json.contains("\"repo_constitution\"")). The old assertion (source_kind == "constitution") would never have matched this entry — "constitution" is reserved for the static system-prompt SourceKind::Constitution entry (context_report.rs:307), a different source. So this was a real false-negative bug in the QA probe.

No correctness, provider/model-routing, reuse, or security concerns in this diff — it's a narrowly-scoped test-infra fix with no production code touched. CI was still in progress at review time (Claude PR Review, CI workflow); DCO and contribution-gate checks have already passed.

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