Skip to content

docs(spec): record the HARNESS-072 review verdict and the lesson the session produced - #1001

Merged
mlorentedev merged 3 commits into
mainfrom
docs/harness-072-review-verdict
Aug 15, 2026
Merged

docs(spec): record the HARNESS-072 review verdict and the lesson the session produced#1001
mlorentedev merged 3 commits into
mainfrom
docs/harness-072-review-verdict

Conversation

@mlorentedev

Copy link
Copy Markdown
Owner

Refs #963 — deliberately not a closing keyword. #963 closes when the spec archives, which is the follow-up this PR makes possible.

Docs only. Two records from one session, both of which would otherwise be lost.

1. The adversarial review verdict

PASS from nan/deepseek-v4-flash against 1320efa, and a real review rather than a well-formed artifact — the distinction #978 was fought over. It ran the eight features.json commands, the 47 bats tests, shellcheck, --check, diffed the vault section against the committed record byte-for-byte, and grepped all five deployed surfaces. Its transcript contains the git rev-parse HEAD reach probe that #978 made the bar.

It is committed because a 25-minute run was living untracked in a worktree, and it took three blocked attempts across the session to obtain: the launcher announced a dead run as a live one (#989, fixed in #994) while the real cause was unscoped secret resolution (#985/#988).

Four Minor findings, none blocking, to be dispositioned before the archive:

# Finding Status
1 .gitignore missed the .stderr sibling fixed in #994
2 tasks.md claims "no unrelated changes" while six vault-drift record syncs rode along to fix in the archive PR
3 features.json f2 is weaker than AC2 — checks the two committed surfaces and doctrine.inject, not the three deployed $HOME payloads to fix in the archive PR
4 check_coverage's doctrine branch has no fixture, so a regression there is uncaught to fix in the archive PR

Findings 2-4 are fair and land in the archive PR rather than here, so this stays a record and not a mixed change.

2. The lesson

bw status reported locked and I called the vault locked. It was not: ADR-028's runtime path resolves through the bw serve daemon, which held a separate unlocked session. Both readings were true about different subjects, and the one I measured was not the one the failing code uses — the remediation I was about to ask the user for would have fixed nothing while looking like progress.

Generalised past bitwarden, because the shape recurs: git status vs the remote, docker ps vs the daemon, kubectl config vs the cluster. Anything fronting a daemon, cache, proxy or pool has two states, and the human-facing status command reports the client's — the one it can see without asking. The tell is a global explanation ("it's locked") arriving before any single-element probe was tried.

PASS from nan/deepseek-v4-flash against 1320efa, and a real review rather than a
well-formed artifact: it ran the 8 features.json commands, the 47 bats tests,
shellcheck, `--check`, diffed the vault section against the committed record
byte-for-byte, and grepped all five deployed surfaces. The reach probe #978 made
the bar — `git rev-parse HEAD` from inside the reviewer — is in its transcript.

Committed on its own so a 25-minute run is not living untracked in a worktree.
`spec archive` reads this file, and it took three blocked attempts across the
session to obtain: the launcher announced a dead run as a live one (#989, fixed
in #994) while the real cause was unscoped secret resolution (#985/#988).

Four Minor findings, all REAL or THEORETICAL, none blocking, to be dispositioned
before the archive:

1. .gitignore missed the .stderr sibling — already fixed in #994.
2. tasks.md claims "no unrelated changes in the diff" while six vault-drift
   record syncs rode along. The claim is inaccurate even though the syncs were
   unavoidable and the author kept the rest out.
3. features.json f2 is weaker than AC2: it checks the two committed surfaces and
   doctrine.inject, not the three deployed $HOME payloads AC2 names. Session
   evidence covers them; the machine-readable check does not.
4. check_coverage's doctrine branch has no fixture — the three bats manifests
   have no doctrine section, so a regression there would not be caught.

Refs #963
…he server

Cost a wrong diagnosis this session: `bw status` reported `locked` and I called
the vault locked, but ADR-028's runtime path resolves through the `bw serve`
daemon, which held a separate unlocked session. Both readings were true about
different subjects; the one I measured was not the one the failing code uses.
The remediation I was about to ask for would have fixed nothing.

Generalised past bitwarden: git/remote, docker/daemon, kubectl/cluster all have
a client state and a server state, and the human-facing status command reports
the one it can see without asking.

Refs #963
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@mlorentedev, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9d818870-4949-41b4-8eb2-70479c7e09f6

📥 Commits

Reviewing files that changed from the base of the PR and between 77d7b7e and d3ddfe7.

📒 Files selected for processing (2)
  • docs/lessons.md
  • specs/HARNESS-072-pr-stewardship/review.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mlorentedev
mlorentedev merged commit 69f4bb5 into main Aug 15, 2026
8 checks passed
@mlorentedev
mlorentedev deleted the docs/harness-072-review-verdict branch August 15, 2026 08:41
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