Skip to content

fix(context): #62 lifecycle/cache/selection correctness - #86

Merged
huiyu merged 1 commit into
3.0from
fix/62-lifecycle-correctness
Aug 24, 2026
Merged

fix(context): #62 lifecycle/cache/selection correctness#86
huiyu merged 1 commit into
3.0from
fix/62-lifecycle-correctness

Conversation

@huiyu

@huiyu huiyu commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Fourth fix off the 3.0 audit — #62 correctness bugs from the external review.

  • lifecycle — MergedPendingDelivery no longer transitions backward to Reviewing on a required-PR check.
  • cache — progressive same-work enrichment (issue→PR→phase) merges in place instead of demanding reconciliation; a broken stale lock now retries and persists the write instead of dropping it.
  • managed-block — revision selection is bound to the expected workId; a foreign higher-revision comment block can't win.

4 regression tests; full suite + generated-drift green. Base 3.0. Resolves the PR #81 lifecycle, cache (×2), and managed-block threads.

Relates to #62

Addresses the #62 correctness findings from the external review of PR #81.

- **lifecycle** — a required-PR-remaining check from MergedPendingDelivery no
  longer transitions backward to Reviewing (that reopened review after merge and
  broke monotonicity); it preserves the current state.
- **cache mergeCache** — progressive same-work enrichment (issue → PR → phase)
  now merges in place instead of returning needs-reconciliation, so the cache can
  evolve through the delivery lifecycle; reconciliation is reserved for a
  contradictory shared value or a changed work_id.
- **cache acquireLock** — a broken stale lock now retries the acquire ONCE and
  persists the current write, rather than dropping it to memory-only and letting
  only the next call benefit.
- **managed-block selectCurrentManagedRevision** — selection is bound to the
  expected workId (and issue for plans); a foreign block with a higher revision
  in a comment can no longer win. The CLI passes these through.
- 4 regression tests (backward-transition guard, foreign-block filtering,
  progressive enrichment, stale-lock persistence).

Relates to #62. Resolves the PR #81 lifecycle, cache, and managed-block threads.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KwyFyRC6pepaK6DuMK7YeT
@huiyu
huiyu merged commit 50b00c7 into 3.0 Aug 24, 2026
1 check passed
huiyu added a commit that referenced this pull request Aug 24, 2026
…F4) (#90)

Independent re-verification found the #86 "worktree writer" wired to nothing:
cli.mjs's deep camelizeKeys recursed into the update-cache `entry`/`attempt`
payloads, mangling work_id/pull_request/pipeline_phase/updated_at into keys the
snake_case cache validator rejects → mergeCache returned invalid-update and
persisted nothing; active_pr/pipeline_phase were permanently null.

- C1: camelizeKeys takes an opaque-key set; `entry`/`attempt` values pass through
  verbatim (key camelized, value untouched) so the schema keys survive.
- F4: inject the repository-resolved projectId LAST so a caller-supplied
  project_id cannot override it.
- F3: mergeCache treats an explicit `null` on the existing side as absent, so
  enriching `pull_request: null` -> a real PR is progressive enhancement, not a
  contradiction.
- F2: the old UC-G8/G9 tests called mergeCache directly and bypassed the exact
  camelize break. Added tests/project-context/cli-update-cache.test.mjs — a real
  end-to-end drive of the CLI over stdin with snake_case, asserting the writer
  persists the pointer keys, round-trips a recovery attempt, and enforces F4.

Adapters regenerated (the codex mirrors carried the identical bug).


Claude-Session: https://claude.ai/code/session_01KwyFyRC6pepaK6DuMK7YeT

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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