Repository navigation
Land 0030-0033 repo-truth and MCP debt fixes - #18
Merged
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 30 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
flyingrobots
changed the title
flyingrobots
marked this pull request as ready for review
April 8, 2026 21:05
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecde6ac00e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/method/backlog/inbox/PROCESS_semantic-drift-detector.md (1)
15-15: 🧹 Nitpick | 🔵 TrivialConsider normalizing decimal precision in threshold values.
Line 15 specifies "LLM confidence >= 0.9" while other thresholds in this document use two decimal places (0.80, 0.65). For consistency and readability, consider using "0.90" instead of "0.9".
♻️ Proposed consistency refinement
-- **Automatic Match**: Cosine similarity >= 0.80 or LLM confidence >= 0.9. +- **Automatic Match**: Cosine similarity >= 0.80 or LLM confidence >= 0.90.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/method/backlog/inbox/PROCESS_semantic-drift-detector.md` at line 15, Update the LLM confidence threshold text to use two-decimal precision for consistency with other thresholds; replace "LLM confidence >= 0.9" in the "Automatic Match" line (the line that currently reads "Cosine similarity >= 0.80 or LLM confidence >= 0.9") with "LLM confidence >= 0.90" so all thresholds use the same decimal format (e.g., 0.80, 0.90, 0.65).src/mcp.ts (1)
183-187:⚠️ Potential issue | 🟠 Major
method_closenever validates the advertised input contract.Lines 185-186 are type assertions only. A caller can send
driftCheck: "false"oroutcome: "done"and this handler will still close the cycle, potentially writing invalid retro metadata despite the tool schema claiming otherwise.Suggested fix
if (request.params.name === 'method_close') { + const driftCheck = args.driftCheck; + if (typeof driftCheck !== 'boolean') { + throw new Error('driftCheck must be a boolean.'); + } + + const outcome = args.outcome; + if (outcome !== 'hill-met' && outcome !== 'partial' && outcome !== 'not-met') { + throw new Error('outcome must be one of: hill-met, partial, not-met.'); + } + const cycle = await workspace.closeCycle( args.cycle as string | undefined, - args.driftCheck as boolean, - args.outcome as Outcome, + driftCheck, + outcome, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcp.ts` around lines 183 - 187, The handler method_close is calling workspace.closeCycle with unvalidated type-asserted values (args.cycle, args.driftCheck, args.outcome) so invalid strings like "false" or "done" can be written; before invoking workspace.closeCycle validate and coerce these inputs: ensure args.cycle is string|undefined, verify args.driftCheck is a boolean (coerce "true"/"false" or reject), and validate args.outcome against the allowed Outcome enum/values (reject unknown values), and return/throw a clear error when validation fails; update the code path that calls workspace.closeCycle to pass only the validated/coerced values.src/index.ts (1)
363-376:⚠️ Potential issue | 🟠 MajorSame-lane moves currently skip the metadata repair.
Line 363 returns before the new frontmatter updates are written. If an item is already in
targetDirbut its in-filelaneis stale or missing,moveBacklogItem()cannot repair it, which breaks the new “managed moves repair metadata” contract and makes the operation non-idempotent.Suggested fix
- if (fullPath === targetPath) { - return relative(this.root, targetPath); - } - - if (existsSync(targetPath)) { + if (fullPath !== targetPath && existsSync(targetPath)) { throw new MethodError(`Destination already exists: ${relative(this.root, targetPath)}`); } - renameSync(fullPath, targetPath); + if (fullPath !== targetPath) { + renameSync(fullPath, targetPath); + } const frontmatterUpdates: Record<string, string> = { lane: targetLane }; if (metadata.legend !== undefined) { frontmatterUpdates.legend = metadata.legend; } fmUpdateFrontmatter(targetPath, frontmatterUpdates); return relative(this.root, targetPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 363 - 376, The current early return when fullPath === targetPath in moveBacklogItem prevents writing frontmatter updates, so update metadata repair logic to run even for same-path (same-lane) moves: compute frontmatterUpdates (using targetLane and metadata.legend) and call fmUpdateFrontmatter(targetPath, frontmatterUpdates) before returning (or remove the early return and only skip renameSync when paths match), ensuring you still check existsSync(targetPath) appropriately and avoid calling renameSync when fullPath === targetPath; reference symbols: fullPath, targetPath, targetLane, metadata.legend, frontmatterUpdates, fmUpdateFrontmatter, renameSync, existsSync, moveBacklogItem.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/method/backlog/bad-code/PROCESS_dist-output-pruning.md`:
- Around line 14-17: Update the backlog item to include concrete, actionable
steps: specify Option 1 (preferred) — add a pre-build clean step by updating the
build script in package.json to run a clean (e.g., remove dist/) before
compilation and add a pre-build hook; Option 2 — add a dist-validation script
that maps src/**/*.ts to dist/**/*.js and fails CI when mismatches are found,
and wire that script into CI job(s) so it runs on PRs and releases; assign
ownership (team or role) and set priority (e.g., Medium); add explicit
acceptance criteria (no dist/*.js without corresponding src/*.ts, build
documented in CONTRIBUTING.md, CI enforces validation); and list the specific
files/locations to change (build script, pre-build hook, validation script, CI
workflow, CONTRIBUTING.md) so the implementer knows where to make edits.
- Around line 9-12: Update the document to remove time-bound assertions and
unverifiable specifics: replace phrases like "still contains" and "no longer"
with durable wording (e.g., "may contain" or "can retain"), and either remove or
reframe the `workspace.*`/`dist/`/`src/workspace.ts` example as a clear
hypothetical (e.g., "for example, build output can accidentally retain obsolete
files such as workspace.*") or delete that specific example and describe the
general pattern of stale build artifacts and how to prevent them; ensure the
file PROCESS_dist-output-pruning.md uses non-temporal language and only asserts
verifiable or explicitly hypothetical scenarios.
In `@docs/method/backlog/up-next/PROCESS_live-legend-definition-coverage.md`:
- Around line 15-16: Replace the informal phrase "real enough" in the taxonomy
statement with a more formal term such as "established enough" or "sufficiently
established" to tighten wording; locate the occurrence of the exact phrase "real
enough" in PROCESS_live-legend-definition-coverage.md and update the sentence so
it reads with the chosen replacement and preserves the original meaning and
tone.
In `@docs/method/feedback/archive/README.md`:
- Around line 12-17: Add a minimal canonical archive-entry template under the
"Each archived document should preserve:" section that enforces consistent
headers/fields; include at least: Title, Date, Author, Original Feedback
(verbatim), Disposition (short note), Related Items/Links
(backlog/docs/outputs), Tags/Status, and Optional Follow-up Actions so future
miners have predictable structure—place the template directly beneath that list
and call it "Archive Entry Template" or similar to make it discoverable when
searching for the phrase "archived document" or "archive-entry".
In `@docs/method/graveyard/.gitkeep`:
- Line 1: Replace the empty placeholder docs/method/graveyard/.gitkeep with a
governed README by deleting .gitkeep and adding docs/method/graveyard/README.md
that documents the directory purpose and scope, a clear retention and deletion
policy, entry/exit criteria for graveyard items, and an index/manifest of
retired documentation (including tombstone metadata like retirement date and
rationale); if the directory must remain empty, ensure README.md serves as the
tracking anchor and contains the governance text and migration/resurrection
procedures referenced in the review.
In `@docs/method/graveyard/MCP_status-summary-mode.md`:
- Around line 23-33: The "Proposal" heading is misleading for a graveyard
document; update the heading text in the section containing the description of
the optional `summary` parameter (the block referencing `method_status`,
`summary`, and the lane counts) to something clearly historical such as
"Original Proposal" or "What Was Proposed" so readers understand this was
retired work rather than an active proposal.
In
`@docs/method/retro/0030-backlog-metadata-single-source-of-truth/backlog-metadata-single-source-of-truth.md`:
- Around line 9-13: The prose duplicates frontmatter metadata (design_doc,
outcome, drift_check); remove the redundant sentences in the body that repeat
those keys or replace them with templated references that pull from frontmatter
so the document follows the single source of truth—locate the paragraph that
repeats "Design: `docs/design/...`", "Outcome: hill-met" and "Drift check: yes"
and either delete those lines or render them programmatically from the
frontmatter fields design_doc, outcome and drift_check.
In
`@docs/method/retro/0030-backlog-metadata-single-source-of-truth/witness/verification.md`:
- Around line 12-30: Add a fenced-code language tag "text" to both fenced
blocks: the block starting with "> method@0.3.0 test" and the empty fenced block
under "## Drift Results" so they become ```text ... ```; this fixes the
markdownlint MD040 violations while preserving the witness formatting contract.
In
`@docs/method/retro/0031-generated-doc-scaffold-contract/witness/verification.md`:
- Around line 12-30: Add explicit language specifiers to the fenced code blocks:
change the test output block (the first fenced block showing the vitest run and
results) to use ```text and change the empty Drift Results fenced block to
```text as well, and populate the drift block with a short explicit message such
as "No drift detected" (or a relevant summary) so it is not an empty code fence;
update the two fenced blocks accordingly in verification.md.
In `@docs/method/retro/0032-mcp-tool-result-contract/mcp-tool-result-contract.md`:
- Around line 33-37: The "New Debt" entry wrongly lists a pre-existing item
(PROCESS_witness-drift-output-capture, origin cycle 0031) as new; update the
docs/method/retro/0032-mcp-tool-result-contract/mcp-tool-result-contract.md by
either moving that bullet out of the "New Debt" section into a new "Observed
Debt" or "Known Limitations" section, or revise the line text to explicitly
state this is a re-observation of debt filed in cycle 0031 (referencing
PROCESS_witness-drift-output-capture.md) so the entry accurately reflects
history rather than claiming a new issue.
In `@docs/method/retro/0032-mcp-tool-result-contract/witness/verification.md`:
- Around line 12-30: The markdown contains fenced code blocks without language
specifiers (e.g., the test output block and the empty "Drift Results" block in
verification.md), triggering MD040; update the witness scaffold generator so it
emits fenced blocks as ```text instead of ``` for verification witnesses, or
manually add the "text" language specifier to the affected blocks in
verification.md; look for the generator code that emits witness scaffolds /
verification witnesses and change the fence emission to use "text" (or adjust
the template for verification.md) so all future witness files include ```text
language tags.
In `@docs/method/retro/0033-bearing-truthfulness/witness/verification.md`:
- Around line 12-30: Update the two fenced code blocks in the document: add a
language specifier (e.g., "text" or "console") to the test output block that
starts with "> method@0.3.0 test" so the linter MD040 is satisfied, and either
add a language specifier and keep the empty "Drift Results" block with a
placeholder (e.g., "None recorded") or remove that empty fenced block entirely;
locate these blocks by searching for the test output snippet and the empty
"Drift Results" code fence in the file (they appear near the "Test Files" /
"Drift Results" sections) and apply the change.
In `@src/cli.ts`:
- Around line 75-80: The code calls workspace.closeCycle(...) twice when
completedDriftCheck is falsy; modify the control flow in the CLI handler so
closeCycle is invoked exactly once: either return immediately after the first
workspace.closeCycle(parsed.cycle, completedDriftCheck, parsed.outcome ??
'partial') call inside the if (!completedDriftCheck) branch, or restructure to
compute the final outcome (using parsed.outcome and completedDriftCheck) and
call workspace.closeCycle(...) a single time at the end. Ensure you reference
the existing symbols: completedDriftCheck, parsed.outcome, parsed.cycle, and
workspace.closeCycle when making the change.
In `@src/index.ts`:
- Around line 503-514: The code currently uses
normalizeLegend(frontmatter.legend) ?? filenameMetadata.legend which lets legacy
filename prefixes override an explicit frontmatter intent to clear the legend
(e.g. frontmatter.legend === 'NONE'); update readBacklogItem (and the similar
block at the other occurrence) to first call normalizeLegend(frontmatter.legend)
into a variable (e.g. const normalized = normalizeLegend(frontmatter.legend)),
then set legend to filenameMetadata.legend only when normalized is
null/undefined and frontmatter did not explicitly request clearing;
alternatively treat a sentinel normalized value (like 'NONE' or an explicit
isCleared test) as an instruction to set legend to null/empty so the
filenameMetadata.legend is not used. Ensure you change both readBacklogItem and
the other analogous assignment so status(), pullItem(), and moveBacklogItem no
longer carry the stale filename legend.
In `@src/mcp.ts`:
- Around line 144-154: The success response in method_inbox currently echoes the
incoming args instead of the persisted values; call captureIdea() and use the
values it returns (e.g., the persisted path/title/legend) when building
successResult so MCP clients see canonical metadata (uppercase legend,
synthesized title, actual persisted path) rather than the raw args; replace the
use of args.title/args.legend with the fields returned by captureIdea() and
ensure the relativePath used in the message comes from the persisted path if
captureIdea returns an absolute path.
In `@src/renderers.ts`:
- Around line 228-230: yamlString currently only escapes backslashes and double
quotes, so literal newlines in inputs like options.title get folded/lost by
YAML; update the yamlString function to also escape line breaks (handle CRLF,
CR, LF) and optionally other line separators by replacing \r\n, \r, and \n with
their escaped sequences (e.g. \\r and \\n) before wrapping in quotes, preserving
the existing .replace calls for backslashes and quotes (i.e., perform newline
replacements either before or after the existing replaces so the final string
contains escaped newline characters).
In `@tests/ship-sync.test.ts`:
- Line 1: The import list includes mkdirSync which is never used; remove
mkdirSync from the named imports on the top import statement (the one importing
mkdtempSync, readFileSync, rmSync, writeFileSync, mkdirSync) so only used
functions remain, or if you actually need to create a directory replace the
unused import by adding a call to mkdirSync where appropriate (e.g., in test
setup) and ensure it's exercised—preferably just remove mkdirSync from the
import to fix the unused-import lint error.
---
Outside diff comments:
In `@docs/method/backlog/inbox/PROCESS_semantic-drift-detector.md`:
- Line 15: Update the LLM confidence threshold text to use two-decimal precision
for consistency with other thresholds; replace "LLM confidence >= 0.9" in the
"Automatic Match" line (the line that currently reads "Cosine similarity >= 0.80
or LLM confidence >= 0.9") with "LLM confidence >= 0.90" so all thresholds use
the same decimal format (e.g., 0.80, 0.90, 0.65).
In `@src/index.ts`:
- Around line 363-376: The current early return when fullPath === targetPath in
moveBacklogItem prevents writing frontmatter updates, so update metadata repair
logic to run even for same-path (same-lane) moves: compute frontmatterUpdates
(using targetLane and metadata.legend) and call fmUpdateFrontmatter(targetPath,
frontmatterUpdates) before returning (or remove the early return and only skip
renameSync when paths match), ensuring you still check existsSync(targetPath)
appropriately and avoid calling renameSync when fullPath === targetPath;
reference symbols: fullPath, targetPath, targetLane, metadata.legend,
frontmatterUpdates, fmUpdateFrontmatter, renameSync, existsSync,
moveBacklogItem.
In `@src/mcp.ts`:
- Around line 183-187: The handler method_close is calling workspace.closeCycle
with unvalidated type-asserted values (args.cycle, args.driftCheck,
args.outcome) so invalid strings like "false" or "done" can be written; before
invoking workspace.closeCycle validate and coerce these inputs: ensure
args.cycle is string|undefined, verify args.driftCheck is a boolean (coerce
"true"/"false" or reject), and validate args.outcome against the allowed Outcome
enum/values (reject unknown values), and return/throw a clear error when
validation fails; update the code path that calls workspace.closeCycle to pass
only the validated/coerced values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43b12266-466f-4e92-b6b6-2c8f4f3b6b4f
📒 Files selected for processing (57)
README.mddocs/GUIDE.mddocs/design/0030-backlog-metadata-single-source-of-truth/backlog-metadata-single-source-of-truth.mddocs/design/0031-generated-doc-scaffold-contract/generated-doc-scaffold-contract.mddocs/design/0032-mcp-tool-result-contract/mcp-tool-result-contract.mddocs/design/0033-bearing-truthfulness/bearing-truthfulness.mddocs/method/backlog/bad-code/PROCESS_dist-output-pruning.mddocs/method/backlog/bad-code/PROCESS_github-comment-sync-identity.mddocs/method/backlog/bad-code/PROCESS_test-taxonomy-and-fixture-drift.mddocs/method/backlog/bad-code/PROCESS_witness-drift-output-capture.mddocs/method/backlog/bad-code/PROCESS_workspace-operation-split.mddocs/method/backlog/cool-ideas/PROCESS_architecture-source-inventory-section.mddocs/method/backlog/cool-ideas/PROCESS_backlog-summary-signpost-section.mddocs/method/backlog/cool-ideas/PROCESS_drift-score-visibility.mddocs/method/backlog/cool-ideas/PROCESS_drift-threshold-config.mddocs/method/backlog/cool-ideas/PROCESS_github-milestone-lane-sync.mddocs/method/backlog/cool-ideas/PROCESS_legend-audit-and-assignment.mddocs/method/backlog/cool-ideas/PROCESS_remote-issue-deletion-sync.mddocs/method/backlog/cool-ideas/PROCESS_retro-conversational-closeout.mddocs/method/backlog/cool-ideas/PROCESS_review-config-hardening.mddocs/method/backlog/cool-ideas/PROCESS_review-state-query.mddocs/method/backlog/cool-ideas/PROCESS_signpost-template-overrides.mddocs/method/backlog/cool-ideas/PROCESS_spike-command.mddocs/method/backlog/cool-ideas/PROCESS_validate-command.mddocs/method/backlog/cool-ideas/PROCESS_visual-witness-screenshot-capture.mddocs/method/backlog/cool-ideas/PROCESS_witness-file-capture.mddocs/method/backlog/cool-ideas/SYNTH_artifact-history-and-semantic-provenance.mddocs/method/backlog/cool-ideas/SYNTH_cycle-witness-command.mddocs/method/backlog/cool-ideas/SYNTH_sync-vision-command.mddocs/method/backlog/inbox/PROCESS_i18n-string-extraction.mddocs/method/backlog/inbox/PROCESS_interactive-scaffolder.mddocs/method/backlog/inbox/PROCESS_multi-forge-adapter.mddocs/method/backlog/inbox/PROCESS_semantic-drift-detector.mddocs/method/backlog/up-next/PROCESS_live-legend-definition-coverage.mddocs/method/feedback/README.mddocs/method/feedback/archive/README.mddocs/method/graveyard/.gitkeepdocs/method/graveyard/MCP_status-summary-mode.mddocs/method/graveyard/PROCESS_legend-should-be-in-frontmatter-yaml.mddocs/method/process.mddocs/method/retro/0030-backlog-metadata-single-source-of-truth/backlog-metadata-single-source-of-truth.mddocs/method/retro/0030-backlog-metadata-single-source-of-truth/witness/verification.mddocs/method/retro/0031-generated-doc-scaffold-contract/generated-doc-scaffold-contract.mddocs/method/retro/0031-generated-doc-scaffold-contract/witness/verification.mddocs/method/retro/0032-mcp-tool-result-contract/mcp-tool-result-contract.mddocs/method/retro/0032-mcp-tool-result-contract/witness/verification.mddocs/method/retro/0033-bearing-truthfulness/bearing-truthfulness.mddocs/method/retro/0033-bearing-truthfulness/witness/verification.mdsrc/cli.tssrc/index.tssrc/mcp.tssrc/renderers.tstests/api.test.tstests/cli.test.tstests/github-adapter.test.tstests/mcp.test.tstests/ship-sync.test.ts
Owner
Author
|
Addressed the current unresolved review feedback on
If CodeRabbit wants another pass on the pushed branch, I am ready to address that round too. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mcp.ts (1)
212-214:⚠️ Potential issue | 🔴 CriticalCritical:
push: falseis silently ignored due to||operator semantics.Line 213 uses
||which treatsfalseas falsy. If a client explicitly passes{ push: false }, the expression(false) || (!false && !undefined)evaluates totrue, ignoring the user's intent.Use nullish coalescing to respect explicit
false:🐛 Proposed fix
- const push = (args.push as boolean | undefined) || (!args.push && !args.pull); + const push = args.push ?? (!args.pull);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcp.ts` around lines 212 - 214, The push flag logic in the method_sync_github branch ignores an explicit false because it uses the || operator; change the expressions that set push and pull to use nullish coalescing (??) so that explicit false values from args.push and args.pull are preserved (e.g. update the lines that assign push and pull when request.params.name === 'method_sync_github' to use args.push ?? (...) and args.pull ?? false respectively and adjust the fallback for push to compute only when args.push is null/undefined).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/method/graveyard/MCP_status-summary-mode.md`:
- Around line 18-21: The disposition text incorrectly states that method_status
defaults to compact summary; instead, change the doc wording to state that
summary-mode is opt-in (an explicit summary flag) and that omitting it yields
full mode per the implementation; update the document paragraph describing the
absorbed behavior to be historical only. Also ensure references/comments align
with the current implementation in the method that handles the summary flag (the
method_status handling in src/mcp.ts) and the tests asserting default behavior
(the expectations in tests/mcp.test.ts) so the docs/tests remain consistent
about summary being opt-in.
In `@src/index.ts`:
- Around line 646-650: normalizeLegend currently allows any non-empty string
except "NONE", which lets invalid frontmatter legends (e.g., "foo-bar", "human
review") become authoritative and propagate through status(), pullItem(), and
moveBacklogItem(); change normalizeLegend to validate the incoming value against
the same pattern used by captureIdea (accept only /^[A-Z][A-Z0-9]*$/), returning
undefined for undefined, empty, "NONE", or any string that fails that regex, so
only canonical legends are normalized and propagated.
In `@src/mcp.ts`:
- Around line 281-293: summarizeStatus currently triggers hidden I/O by calling
workspace.closedCycles() inside the pure helper; change its signature to accept
closedCycles: Cycle[] (e.g., summarizeStatus(workspace: Workspace, status:
WorkspaceStatus, closedCycles: Cycle[])) and replace workspace.closedCycles()
usage with the new closedCycles parameter, then update all callers to pass the
precomputed workspace.closedCycles() result so the filesystem scan happens once
at the call site rather than on every summarizeStatus invocation.
In `@src/renderers.ts`:
- Around line 118-124: The frontmatter writes legendValue unescaped which allows
user-controlled text (options.legend) to break or inject YAML; in the renderer
that builds frontmatter (the code using legendValue and yamlString for other
fields) replace the raw legend insertion with a YAML-escaped version by passing
legendValue (or options.legend ?? 'none') through yamlString so the legend line
is generated like the other fields; update the usage of legendValue to use
yamlString(...) and ensure default 'none' is preserved.
In `@tests/docs.test.ts`:
- Around line 728-729: The assertions on mcp are too loose; instead of checking
raw tokens, parse or anchor to the actual MCP structure and assert the keys
exist in the right place. Update the test in tests/docs.test.ts to either
JSON.parse(mcp) and use expect(parsedMcp).toHaveProperty('summary') and
expect(parsedMcp).toHaveProperty('structuredContent'), or use anchored regexes
like expect(mcp).toMatch(/"summary"\s*:/) and
expect(mcp).toMatch(/"structuredContent"\s*:/) so the assertions verify the
actual contract rather than incidental tokens.
In `@tests/mcp.test.ts`:
- Around line 25-39: createCallToolHarness installs a spy on
Server.prototype.setRequestHandler (via vi.spyOn in createCallToolHarness) but
relies on a trailing vi.restoreAllMocks() in tests which can leak if assertions
fail; fix by ensuring mocks are always cleaned: either add vi.restoreAllMocks()
to the file-level afterEach, or change createCallToolHarness to return a cleanup
function and call it in a finally block in each test (or wrap the spy setup in
try/finally inside createCallToolHarness) so Server.prototype.setRequestHandler
is always restored even if assertions throw.
---
Outside diff comments:
In `@src/mcp.ts`:
- Around line 212-214: The push flag logic in the method_sync_github branch
ignores an explicit false because it uses the || operator; change the
expressions that set push and pull to use nullish coalescing (??) so that
explicit false values from args.push and args.pull are preserved (e.g. update
the lines that assign push and pull when request.params.name ===
'method_sync_github' to use args.push ?? (...) and args.pull ?? false
respectively and adjust the fallback for push to compute only when args.push is
null/undefined).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 37bfe71a-1954-4819-82e9-d157def2045c
📒 Files selected for processing (27)
docs/CLI.mddocs/MCP.mddocs/method/backlog/bad-code/PROCESS_dist-output-pruning.mddocs/method/backlog/inbox/PROCESS_semantic-drift-detector.mddocs/method/backlog/up-next/PROCESS_live-legend-definition-coverage.mddocs/method/feedback/archive/README.mddocs/method/graveyard/MCP_status-summary-mode.mddocs/method/graveyard/README.mddocs/method/retro/0030-backlog-metadata-single-source-of-truth/backlog-metadata-single-source-of-truth.mddocs/method/retro/0030-backlog-metadata-single-source-of-truth/witness/verification.mddocs/method/retro/0031-generated-doc-scaffold-contract/generated-doc-scaffold-contract.mddocs/method/retro/0031-generated-doc-scaffold-contract/witness/verification.mddocs/method/retro/0032-mcp-tool-result-contract/mcp-tool-result-contract.mddocs/method/retro/0032-mcp-tool-result-contract/witness/verification.mddocs/method/retro/0033-bearing-truthfulness/bearing-truthfulness.mddocs/method/retro/0033-bearing-truthfulness/witness/verification.mdsrc/cli-args.tssrc/cli.tssrc/index.tssrc/mcp.tssrc/renderers.tstests/api.test.tstests/cli.test.tstests/docs.test.tstests/mcp.test.tstests/ship-sync.test.tstests/witness.test.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/method/backlog/bad-code/PROCESS_mcp-runtime-input-validation-audit.md`:
- Around line 19-23: Replace the vague acceptance criterion "Structured error
responses stay clear and consistent when validation fails" with a concrete
requirement that every audited MCP mutation tool returns the defined MCP error
envelope (name the exact envelope schema) on validation failure, reference the
source-of-truth for that envelope (the canonical schema/doc) as the audit scope,
and require tests that assert the exact envelope shape and fields for at least
one invalid-input case per tool; update the checklist entries in
PROCESS_mcp-runtime-input-validation-audit.md to name the envelope/schema and
the canonical source so compliance is machine-verifiable.
In `@docs/method/backlog/cool-ideas/PROCESS_cycle-packet-markdownlint-gate.md`:
- Around line 9-16: The proposal lacks concrete scope and acceptance
criteria—replace the vague "bounded markdown validation step" with a precise
contract: list exact file globs to lint (e.g., docs/design/*/witness*.md,
docs/design/*/retro.md, docs/BEARING.md, docs/MCP.md and any other generated
outputs), enumerate the markdownlint rules to enforce (explicit rule IDs like
MD040, MD031 and any custom frontmatter schema checks), state the integration
point (e.g., run as a CI job named "markdownlint-gate" and optionally a
pre-commit hook or as a `method validate markdown` subcommand), and define exit
criteria (fail the job on any ERROR-level rule violations, allow WARN-level
rules but report them). Ensure the updated idea text contains those four items
exactly: file globs, rule list, integration point, and pass/fail exit criteria.
- Around line 1-5: The frontmatter for the Cycle Packet Markdownlint Gate is
missing required metadata—add explicit owner:, priority:, and
acceptance_criteria: fields to the YAML frontmatter (alongside existing title:
and legend:) so the backlog item matches the PR standard; update the file's
frontmatter in
docs/method/backlog/cool-ideas/PROCESS_cycle-packet-markdownlint-gate.md to
include an owner (person or team), a priority value, and concise acceptance
criteria entries that reflect the expectations described in the body.
In `@docs/method/backlog/cool-ideas/PROCESS_retro-scaffold-refresh-command.md`:
- Around line 9-15: Update the backlog doc
PROCESS_retro-scaffold-refresh-command.md to replace the current high-level
description with concrete acceptance criteria: specify the command input (e.g.,
target cycle packet IDs or file globs), enumerate exactly which scaffold
sections must be preserved (e.g., human-edited header blocks, // HANDWRITTEN:
markers or YAML front-matter keys), define expected diff behavior (only
scaffolded blocks replaced, stable whitespace and unchanged preserved sections,
and a summary of modified/unchanged files), and state failure modes and exit
codes (e.g., conflict when preserved markers missing, dry-run output, and
non-zero exit on parse errors). Include examples of a sample input, before/after
snippets demonstrating preserved vs. regenerated sections, and a clear PASS
criterion that can be used in tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2ff9506e-5e38-4411-b40e-e762d3714688
📒 Files selected for processing (3)
docs/method/backlog/bad-code/PROCESS_mcp-runtime-input-validation-audit.mddocs/method/backlog/cool-ideas/PROCESS_cycle-packet-markdownlint-gate.mddocs/method/backlog/cool-ideas/PROCESS_retro-scaffold-refresh-command.md
Owner
Author
|
Addressed the latest PR #18 feedback on current head
Local verification on the pushed branch:
Both GitHub |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderers.ts (1)
81-95:⚠️ Potential issue | 🟡 MinorDon't emit an empty
Test Resultsfence.Only
driftResultgets a fallback. If the captured test command produces no output, Lines 94-96 still render an empty fenced block—the same artifact shape this PR is trying to stamp out. GivetestResultthe same placeholder treatment.Proposed fix
export function renderWitnessDoc(options: { cycle: Cycle; testResult: string; driftResult: string; }): string { const title = readHeading(options.cycle.designDoc) || titleCase(options.cycle.slug); + const testResult = options.testResult.trim() || 'No test output captured.'; const driftResult = options.driftResult.trim() || 'No drift output captured.'; return [ @@ '## Test Results', '', '```text', - options.testResult.trim(), + testResult, '```',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderers.ts` around lines 81 - 95, The Test Results block can emit an empty fenced block because options.testResult is used raw; create a fallback like you did for driftResult by defining const testResult = options.testResult.trim() || 'No test output captured.' (or similar message) and replace options.testResult.trim() in the returned array with testResult so the fenced block always contains a meaningful placeholder when no test output exists.src/index.ts (1)
344-375:⚠️ Potential issue | 🔴 CriticalReject backlog paths that escape the workspace.
Line 345 trusts
pathafterresolve(this.root, path). That accepts absolute inputs and../traversal, so this method can rename an arbitrary filesystem file into the repo and then rewrite its frontmatter. Add a confinement check beforereadBacklogItem()/renameSync()and mirror the same boundary in the direct-path branch ofresolveBacklogItem().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 344 - 375, The code currently resolves an arbitrary input path with resolve(this.root, path) in moveBacklogItem and then operates on it, allowing absolute/../ traversal; add a confinement check that the resolved fullPath is inside the repository workspace (this.root) before calling readBacklogItem or performing renameSync: compute the canonical/resolved fullPath and ensure path.relative(this.root, fullPath) does not start with '..' (or use a utility like pathIsInside), otherwise throw a MethodError; apply the same containment check logic to the direct-path branch in resolveBacklogItem so both code paths enforce the workspace boundary.
♻️ Duplicate comments (1)
docs/method/backlog/cool-ideas/PROCESS_retro-scaffold-refresh-command.md (1)
47-51:⚠️ Potential issue | 🟠 MajorExamples show input only; expected output is missing.
The past review comment emphasized "expected diff behavior" and the contract section defines exit codes, but your examples show zero output. How is an implementer supposed to know what success looks like?
Show before/after snippets, sample diff output, and error messages for each exit code path.
📸 Add output examples
## Examples -- `method refresh retro 0033-bearing-truthfulness` -- `method refresh retro 003[0-3]-* --dry-run` -- `method refresh retro docs/method/retro/0032-mcp-tool-result-contract` +### Success case +```shell +$ method refresh retro 0033-bearing-truthfulness +Refreshed: docs/method/retro/0033-bearing-truthfulness/bearing-truthfulness.md + - Updated frontmatter: title, legend + - Preserved sections: Summary, Drift, New Debt +Exit 0 +``` + +### Dry-run mode +```shell +$ method refresh retro 003[0-3]-* --dry-run +Would refresh: 0030-backlog-metadata-single-source-of-truth +Would refresh: 0031-generated-doc-scaffold-contract +Would refresh: 0032-mcp-tool-result-contract +Would refresh: 0033-bearing-truthfulness +Exit 0 (dry-run, no files modified) +``` + +### Error case: no matches +```shell +$ method refresh retro 9999-nonexistent +Error: No retro packets match selector '9999-nonexistent' +Exit 1 +``` + +### Error case: write failure +```shell +$ method refresh retro 0032-mcp-tool-result-contract +Error: Failed to write docs/method/retro/0032-mcp-tool-result-contract/mcp-tool-result-contract.md (permission denied) +Exit 3 +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/method/backlog/cool-ideas/PROCESS_retro-scaffold-refresh-command.md` around lines 47 - 51, Add expected output examples to the three current examples for the "method refresh retro" command: show realistic before/after file paths and a sample diff/summary for a successful run (e.g., output lines like "Refreshed: docs/method/retro/0033-.../file.md" and a short list of changes), include a dry-run example that lists "Would refresh: ..." and explicitly states "Exit 0 (dry-run, no files modified)", and provide error-case outputs for the two contract exit paths (no matches -> "Error: No retro packets match selector '...'; Exit 1" and write failure -> "Error: Failed to write <path> (permission denied); Exit 3"). Reference the existing command examples ("method refresh retro 0033-bearing-truthfulness", "method refresh retro 003[0-3]-* --dry-run", "method refresh retro docs/method/retro/0032-mcp-tool-result-contract") so maintainers can place the before/after and error outputs next to each corresponding input example.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 41-43: Move the "## Unreleased" section (the header string "##
Unreleased" and its subsection "### Fixed") to the very top of CHANGELOG.md so
it appears before any tagged release sections; ensure the "Unreleased" block is
the first version section in the file and retain its content, preserving
existing headings and formatting when reordering.
In `@docs/method/backlog/cool-ideas/PROCESS_cycle-packet-markdownlint-gate.md`:
- Around line 26-36: The retro frontmatter rule currently targets the glob
`docs/method/retro/[0-9][0-9][0-9][0-9]-*/**/*.md` which inadvertently includes
witness files; narrow the rule by excluding witness artifacts (e.g. add an
exclusion like `!**/witness/**/*.md`) or change the retro target to only
top-level retro docs (for example target
`docs/method/retro/[0-9][0-9][0-9][0-9]-*/index.md`), so that the frontmatter
contract requiring keys `title`, `cycle`, `design_doc`, `outcome`, and
`drift_check` is not applied to witness files.
In `@docs/method/backlog/cool-ideas/PROCESS_retro-scaffold-refresh-command.md`:
- Around line 35-38: Replace the vague phrases "scaffold-owned frontmatter
fields" and "generated placeholder blocks whose contract changed" with a
concrete, testable specification: either list the exact YAML frontmatter keys
(e.g., enumerate each scaffold-owned key) and the exact block markers/markers
for generated placeholders, or define a machine-readable detection mechanism
(e.g., require a specific YAML key like scaffold_managed: true or an HTML
comment marker like <!-- scaffold-managed -->) and describe the detection
algorithm; update occurrences of the terms "scaffold-owned frontmatter fields"
and "generated placeholder blocks" in the document to reference the chosen
keys/marker and include a short example of the marker usage so implementers can
reliably detect and regenerate them.
- Around line 42-45: The document's exit code section for
PROCESS_retro-scaffold-refresh-command.md lacks batch semantics for multiple
selected packets; update the "Failure modes and exit codes" paragraph in
PROCESS_retro-scaffold-refresh-command.md to state whether the command: stops on
first packet failure or continues processing remaining packets, how it
aggregates results (e.g., counts of succeeded/failed/skipped), and define
concrete exit codes for partial vs total failure (for example distinct code for
any partial failures vs all failures) and how detailed error reporting (summary
+ per-packet errors) is exposed to users; ensure the text references the
existing exit codes (0,1,2,3) and clarifies how they map to the new batch
outcomes.
- Line 17: The document uses inconsistent terminology ("cycle packet" vs "retro
packet"); choose one term and apply it consistently across
PROCESS_retro-scaffold-refresh-command.md (replace "cycle packet" on line 17 to
match "retro packet" used on lines 29/43/55) or, if they are distinct concepts,
add a short clarifying sentence near the top defining both terms (e.g., "cycle
packet = X; retro packet = Y") and then update each occurrence to follow that
definition so all references are unambiguous.
- Around line 7-10: Update the acceptance_criteria block to explicitly list
which frontmatter keys are scaffold-owned and which are human-editable (e.g.,
enumerate keys like title, owner, priority, created_at, scaffold_version, etc.)
and define "generated placeholder" (e.g., auto-filled sections such as TODOs,
autogenerated witness stubs, scaffold markers) so implementers have a definite
reference; then remove the duplicated behavioral details by replacing the
duplicate lines with a single pointer to the "Proposed Contract" section (or
reference the exact subsection) to avoid drift and keep authoritative logic in
one place.
- Around line 53-57: Expand the PASS Criteria section into concrete, testable
items that map to each contract element: enumerate and specify expected behavior
and test oracle for each of the five exit codes (e.g., SUCCESS, PARTIAL_FAILURE,
INVALID_INPUT, NO_OP, ERROR), for each of the three input types (single packet
id, bounded range, entire backlog) include sample inputs and expected outcome,
add separate criteria for dry-run vs write modes (no state change vs applied
changes), add criteria to assert preserved vs regenerated content detection (how
to assert "preserved" vs "regenerated" with example before/after content), and
add batch failure handling criteria (atomicity expectations and per-packet error
reporting); also define a precise output format or schema for the command
explanation (what fields/messages must be present when “explains exactly what
changed or why it refused”) so tests can assert exact text/fields.
- Around line 28-29: Fix capitalization by replacing every instance of the
phrase "cycle ids" with "cycle IDs" (including both occurrences in the document)
and clarify the path resolution rule for the phrase "or a path list resolved to
retro packet roots" by specifying an exact algorithm: accept each argument as
either an explicit cycle ID, a glob, or a filesystem path; if a path is
provided, resolve it by (1) checking if it is a directory and whether it
contains a valid retro packet marker (e.g., packet root file), (2) if it is a
file, walk upward from the file to the nearest ancestor directory containing the
packet marker, (3) reject or error if no packet root is found, and (4) normalize
and deduplicate resolved packet roots before processing; update the text around
the command description (the clause containing "or a path list resolved to retro
packet roots") to include this algorithm and the expected error behavior when a
path points to a witness file or an invalid location.
In `@src/mcp.ts`:
- Around line 383-390: The validateOptionalString function currently allows
empty or whitespace-only strings which later causes Workspace.resolveCycle
(e.g., name.endsWith('')) to match all cycles; change validateOptionalString to
trim the input when it's a string and reject (throw an Error) if the trimmed
value is an empty string or only whitespace, otherwise return the trimmed string
so callers like Workspace.resolveCycle receive a non-empty, normalized value.
- Around line 213-214: The code currently sets push and pull via TypeScript
casts which don't validate runtime types, so malformed inputs like { push:
"false" } will be treated as truthy; before deciding to call pushBacklog() or
pullBacklog(), validate args.push and args.pull as real booleans (e.g., using a
runtime check or helper like isBoolean(value) or a parseBoolean utility) and
only apply the defaults after validation: compute validatedPush =
isBoolean(args.push) ? args.push : undefined and validatedPull =
isBoolean(args.pull) ? args.pull : undefined, then set push = validatedPush ??
!(validatedPull ?? false) and pull = validatedPull ?? false; update any
locations that reference push/pull (the pushBacklog and pullBacklog decision
points) to use these validated values.
In `@src/renderers.ts`:
- Around line 121-124: The frontmatter serialization in renderers.ts uses
options.source and the computed designDoc which can contain Windows backslashes;
normalize both values to POSIX separators before calling yamlString so generated
paths are consistent across OSes—replace backslashes with forward slashes (e.g.,
use options.source.replace(/\\/gu, '/') and designDoc.replace(/\\/gu, '/') prior
to serializing) in the lines that build `source_backlog` and the design doc
frontmatter.
---
Outside diff comments:
In `@src/index.ts`:
- Around line 344-375: The code currently resolves an arbitrary input path with
resolve(this.root, path) in moveBacklogItem and then operates on it, allowing
absolute/../ traversal; add a confinement check that the resolved fullPath is
inside the repository workspace (this.root) before calling readBacklogItem or
performing renameSync: compute the canonical/resolved fullPath and ensure
path.relative(this.root, fullPath) does not start with '..' (or use a utility
like pathIsInside), otherwise throw a MethodError; apply the same containment
check logic to the direct-path branch in resolveBacklogItem so both code paths
enforce the workspace boundary.
In `@src/renderers.ts`:
- Around line 81-95: The Test Results block can emit an empty fenced block
because options.testResult is used raw; create a fallback like you did for
driftResult by defining const testResult = options.testResult.trim() || 'No test
output captured.' (or similar message) and replace options.testResult.trim() in
the returned array with testResult so the fenced block always contains a
meaningful placeholder when no test output exists.
---
Duplicate comments:
In `@docs/method/backlog/cool-ideas/PROCESS_retro-scaffold-refresh-command.md`:
- Around line 47-51: Add expected output examples to the three current examples
for the "method refresh retro" command: show realistic before/after file paths
and a sample diff/summary for a successful run (e.g., output lines like
"Refreshed: docs/method/retro/0033-.../file.md" and a short list of changes),
include a dry-run example that lists "Would refresh: ..." and explicitly states
"Exit 0 (dry-run, no files modified)", and provide error-case outputs for the
two contract exit paths (no matches -> "Error: No retro packets match selector
'...'; Exit 1" and write failure -> "Error: Failed to write <path> (permission
denied); Exit 3"). Reference the existing command examples ("method refresh
retro 0033-bearing-truthfulness", "method refresh retro 003[0-3]-* --dry-run",
"method refresh retro docs/method/retro/0032-mcp-tool-result-contract") so
maintainers can place the before/after and error outputs next to each
corresponding input example.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 645f5126-4d5e-474d-bafc-9f594cfe19c4
📒 Files selected for processing (13)
CHANGELOG.mddocs/method/backlog/bad-code/PROCESS_mcp-runtime-input-validation-audit.mddocs/method/backlog/cool-ideas/PROCESS_cycle-packet-markdownlint-gate.mddocs/method/backlog/cool-ideas/PROCESS_retro-scaffold-refresh-command.mddocs/method/graveyard/MCP_status-summary-mode.mdsrc/index.tssrc/mcp.tssrc/renderers.tstests/api.test.tstests/cli.test.tstests/docs.test.tstests/mcp.test.tstests/witness.test.ts
This was referenced Jun 1, 2026
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
This branch lands four completed bad-code cycles plus one follow-on backlog capture:
0030-backlog-metadata-single-source-of-truth0031-generated-doc-scaffold-contract0032-mcp-tool-result-contractstructuredContentenvelopes instead of prose-only machine interfacesmethod_statusdefault to summary mode withsummary: falsefor expanded stateMCP_status-summary-modebacklog item to the graveyard0033-bearing-truthfulnessWhat feels wrong?section from live backlog and active-cycle statebad-codePROCESS_witness-drift-output-captureWhy it changed
The repo had multiple repo-truth and agent-DX failures at once:
Those are all coordination failures. They make the repo harder for both humans and agents to trust.
Impact
For developers and agents, METHOD is now more legible and cheaper to automate against:
structuredContentas the machine interfaceRoot cause
These issues all came from split authority and contract drift:
Validation
Ran on
cycles/0033-bearing-truthfulnessafter push:npm testnpm run buildgit diff --check./node_modules/.bin/tsx src/cli.ts statusAll passed.
Follow-up after merge
Run
method sync shipon mergedmainso repo-level generated surfaces such asdocs/BEARING.md,docs/MCP.md, andCHANGELOG.mdreflect the merged state rather than branch-local closeout state.