Skip to content

fix(skills): drop the residual-findings record file for real sinks - #1387

Merged
tmchow merged 1 commit into
mainfrom
tmchow/drop-residual-findings-file
Aug 15, 2026
Merged

fix(skills): drop the residual-findings record file for real sinks#1387
tmchow merged 1 commit into
mainfrom
tmchow/drop-residual-findings-file

Conversation

@tmchow

@tmchow tmchow commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Accepted review findings had three possible sinks: a Known Residuals section in the PR description, a ticket in the project's tracker, and — last resort in ce-work and ce-debug, primary in lfg — a committed <root>/residual-review-findings/<branch-or-sha>.md file. This removes the third.

The evidence against it is one-sided. It fired once in this repository's history, and that once was wrong: a tracker ticket existed and was filed, and the file was written anyway, though it is specified as reachable only when no tracker is. That issue has since closed and the file was still sitting in the tree, because nothing ever collects one. A path named after a branch or SHA in a directory nobody greps is closer to write-only than to durable.

It also cost more than it carried. Every consumer had to describe it, and it forced an exception into ce-debug's commit-scope constraint — "the fix-owned files and nothing else, except this one artifact" — which was itself a contract break caught in review on #1385, since the artifact is not a fix-owned file.

Where residuals go now

Situation Sink
A PR will be created or updated Known Residuals section in the PR description
No PR, tracker reachable One ticket per finding
lfg — findings its tracker chain could not file (failed / no_sink) One run-report comment on the PR
Neither a PR nor a tracker Stated in the run report, explicitly as recorded nowhere else

The lfg change is the substantive one. Its DONE gate previously blocked on the record file being written, committed, and pushed. It now blocks on the run-report comment — the same surface ce-babysit-pr already uses for unfixable CI, so this consolidates onto an existing pattern rather than inventing one. A point-in-time comment also does not go stale as items resolve, which is the property lfg cited when it rejected a PR-body section.

The last row is the honest part. With no PR and no tracker there is no durable sink, and saying so is better than writing a file that only looks like one. That case is also the narrowest: it needs a repo with neither a remote nor a tracker, where the run output is in front of the user anyway.

Scope

Removed from ce-work, ce-debug (skill, pipeline-mode, post-fix-handoff), lfg, the ce-doc-review protected-artifact list, the config template and its byte-identical config.example.yaml copy, and the user-facing docs/skills/lfg.md. The stale artifact is deleted.

tests/docs-root-literals.test.ts deliberately keeps residual-review-findings in its subdirectory list: that guard catches hardcoded docs/<subdir> literals, and keeping the name means a reintroduced literal would still fail rather than pass silently.

Historical planning docs under docs/plans/ still mention the mechanism and are left alone — they are records of what was decided at the time.

Validation

bun test 3101 pass / 0 fail; release:validate and plugin:validate --strict clean. Two contract tests in tests/review-skill-contract.test.ts pinned the removed path and the record file's push mechanics; both now pin the replacement behaviour instead.

Related: #1385

Security Disclosure

No security-relevant changes. This removes a write: lfg no longer creates, commits, and pushes a file as part of its DONE gate, so the autonomous pipeline touches the tree less than before.

Agent Disclosure

  • Model: Claude Code · claude-opus-5[1m]

The committed `<root>/residual-review-findings/<sha>.md` file was the
last-resort sink for accepted review findings in ce-work and ce-debug, and the
primary sink in lfg. It fired exactly once in this repository's history, and
that once was wrong: a tracker ticket existed and was filed, and the file was
written anyway. The issue it duplicated has since closed; the file was still in
the tree because nothing ever collects one. A path named after a branch or SHA
in a directory nobody greps is closer to write-only than to durable.

It also cost more than it carried. Every consumer had to describe it, and it
forced an exception into ce-debug's commit-scope constraint — a contract break
caught in review on #1385 — because the artifact is not a fix-owned file.

Residuals now use sinks that a human actually encounters: the PR description's
Known Residuals section, or a ticket in the project's tracker. lfg carries the
findings its tracker chain could not file in one run-report comment on the PR,
the same surface ce-babysit-pr already uses for unfixable CI, and its DONE gate
waits on that comment rather than on a file write and a push. When neither sink
is reachable — no PR and no tracker — the run states the residuals in its report
and says plainly that nothing else recorded them, which is honest where a
committed file only looked durable.

Removes the stale artifact, the artifact-root declarations that named the
directory, and its entry in the ce-doc-review protected-artifact list. The
`docs-root-literals` guard keeps the subdirectory name so a hardcoded literal
would still be caught if one were reintroduced.

Claude-Session: https://claude.ai/code/session_015eproCfjCz2WZdynWFwGiT
@cursor

cursor Bot commented Aug 15, 2026

Copy link
Copy Markdown

PR Summary

Cursor Bugbot is generating a summary for commit 7db6f4b. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7db6f4b. Configure here.

Comment thread skills/lfg/SKILL.md
4. **Durable record — never the PR body.** Do NOT write a `## Residual Review Findings` section into the PR description; it duplicates GitHub's own tracking and goes stale as items resolve. Review residuals have no GitHub thread of their own, so they are made durable by the tracker tickets filed in step 2 plus **one run-report comment on the PR** carrying the composed section (ticket links included) and the source run context — the same surface `ce-babysit-pr` already uses for unfixable CI. Post it with `gh pr comment`; a point-in-time comment does not go stale as items resolve, the way a body section or a committed file does.

Do not output DONE until the residuals are durable (tracker tickets filed and/or the record file committed). Never block DONE on tracker filing failures once the record file exists. A push that fails when a remote exists is a stop-and-report; never retry a push, or block DONE, when no remote exists.
Do not output DONE until the residuals are durable: tracker tickets filed, and — when a PR exists — the run-report comment posted. Never block DONE on tracker filing failures once the comment is posted. When no PR exists at all (no remote, per the shipping precondition), the run output is the record: state the residuals in the DONE report rather than committing a file nobody will read.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR comment posted before PR exists

High Severity

Step 6 now makes residuals durable by posting a run-report comment with gh pr comment, but the PR is only created later in step 8 via ce-commit-push-pr. On a fresh branch the comment target does not exist yet, so the new DONE gate either stalls or skips the comment, leaving failed / no_sink findings without the durable sink this change intended. Step 8 back-fills ticket links once the PR URL is known, but never posts a deferred residual comment.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7db6f4b. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7db6f4bf48

ℹ️ 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".

Comment thread skills/lfg/SKILL.md
- For each `settled_conflict`-stamped finding from step 4: a bullet with severity, file:line, title, and the conflicting KTD the stamp names — included even though the finding is report-only.
- For each proceeded-and-flagged `settled_decision_conflicts` entry from step 2: a bullet with the KTD, the evidence, and how it was routed.
4. **Durable record — never the PR body.** Do NOT write a `## Residual Review Findings` section into the PR description; it duplicates GitHub's own tracking and goes stale as items resolve. Review residuals have no GitHub thread of their own, so they are made durable by the tracker tickets filed in step 2 plus a committed record file — not a PR-body section and not a PR comment that duplicates the tickets. Create/replace `<root>/residual-review-findings/<branch-or-head-sha>.md` with the composed section (ticket links included) and the source run context. Stage only that file, commit `docs(review): record residual review findings`, and push **when a remote is configured** (per the shipping precondition): if an upstream exists, `git push`; else if a remote exists, resolve a writable one (prefer `origin`, otherwise the first configured remote) and `git push --set-upstream <remote> HEAD`; if there is no remote at all, the local commit is the durable sink.
4. **Durable record — never the PR body.** Do NOT write a `## Residual Review Findings` section into the PR description; it duplicates GitHub's own tracking and goes stale as items resolve. Review residuals have no GitHub thread of their own, so they are made durable by the tracker tickets filed in step 2 plus **one run-report comment on the PR** carrying the composed section (ticket links included) and the source run context — the same surface `ce-babysit-pr` already uses for unfixable CI. Post it with `gh pr comment`; a point-in-time comment does not go stale as items resolve, the way a body section or a committed file does.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Delay the residual comment until after PR creation

On the normal LFG path where the current branch has no existing PR, this step runs before step 8 opens one. The checked gh pr comment --help defines the command as adding a comment to an existing pull request, so it has no target here; no later step retries the composed comment after receiving the PR URL, leaving failed and no_sink findings without the promised durable record. Carry the section forward and post it immediately after step 8 creates or identifies the PR.

AGENTS.md reference: AGENTS.md:L147-L147

Useful? React with 👍 / 👎.

Comment thread docs/skills/lfg.md
4. `/ce-simplify-code` runs on the branch diff before review, unless the change is docs-only or roughly under 10 lines.
5. `/ce-code-review` (`mode:agent`) reports findings. `lfg` applies eligible mechanical fixes and commits them. Review itself does not edit the tree.
6. Leftover actionable findings, plus any flagged settlement conflicts, become durable as tracker tickets and a committed file under `<root>/residual-review-findings/`. They are not written into the PR body.
6. Leftover actionable findings, plus any flagged settlement conflicts, become durable as tracker tickets and one run-report comment on the PR. They are not written into the PR body.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove the stale claim that residuals are committed

After this changed summary establishes tracker tickets plus a PR comment as the residual sinks, the same user-facing page still says at line 106 that unapplied findings are “filed and committed.” The record-file commit was removed by this change, so that description now falsely promises a committed artifact; update it to describe the tracker/comment behavior consistently.

AGENTS.md reference: AGENTS.md:L88-L90

Useful? React with 👍 / 👎.

@tmchow
tmchow merged commit 4d41c7d into main Aug 15, 2026
5 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 15, 2026
ethras added a commit to ethras/compound-engineering-orca that referenced this pull request Aug 16, 2026
* fix(ce-commit-push-pr): root PR stacks on the parent PR the user named (EveryInc#1365)

* fix(ce-babysit-pr): decode gh output as UTF-8 on Windows (EveryInc#1368)

* fix(ce-prototype): cover decisions settled by seeing, not just driving (EveryInc#1369)

* perf(tests): cut suite wall time by splitting the largest test file (EveryInc#1370)

* fix(tests): stop the cross-model routes test reading the working tree (EveryInc#1371)

* fix(ce-doc-review): ask only where a real choice exists, batch the rest (EveryInc#1373)

* chore(orca): re-pin upstream provenance baseline to 421a337

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(ce-doc-review): align Orca ownership wording with Apply routing

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(ce-prototype): add a seeing-mode craft floor and durable storage (EveryInc#1374)

* fix(ce-pov): stop the panel guessing the cross-model host argument (EveryInc#1375)

* chore(cross-model): pin the Grok peer to 4.6 (EveryInc#1376)

* docs(skills): rewrite user skill pages for accuracy and clearer use (EveryInc#1377)

* fix(commit): append known plan unit ids to commit subjects (EveryInc#1379)

* fix(ce-work): stop sandboxed workers committing in linked worktrees (EveryInc#1382)

* fix(ce-doc-review): edit HTML plans in native format (EveryInc#1381)

* fix(ce-code-review): cover adversarial after quota or auth no-review (EveryInc#1380)

* fix(skills): correct a rejected dispatch instead of spending the fallback (EveryInc#1383)

* fix(ce-compound): find Claude sessions started outside the repo root (EveryInc#1378)

* ci(windows-native): retry peer-job-runner smoke on ctypes flake (EveryInc#1384)

* fix(ce-debug): stop asking at the handoff, stop shipping unoffered work (EveryInc#1385)

* docs(solutions): record why skill gates state conditions, not git commands (EveryInc#1386)

* fix(skills): drop the residual-findings record file for real sinks (EveryInc#1387)

* fix(ce-doc-review): run the cross-model pass when CROSS_MODEL_PEERS is unset (EveryInc#1389)

* fix(ce-proof): sync with current Proof v3 contract (EveryInc#1390)

* fix(skill-authoring): make goal-first the default when authoring and reviewing skills (EveryInc#1391)

* fix(cross-model): let reviews run on Fable and pin model/effort from CE config (EveryInc#1392)

* docs(cross-model): point superseded peer benchmarks at the luna/xhigh decision (EveryInc#1393)

* fix(cross-model): discover the Codex.app-bundled codex CLI and name the peer-CLI requirement (EveryInc#1395)

* feat(cross-model): add cross_model_review_mode checkout egress gate (EveryInc#1396)

* fix(ce-compound-refresh): compare knowledge-track learnings against guidance they name (EveryInc#1399)

* docs(solutions): capture the named-guidance contradiction-check learning (EveryInc#1400)

* fix(ce-compound): prefer the repo's own frontmatter vocabulary over the Rails-era enums (EveryInc#1394)

* fix(ce-work): stop asking about branches before starting work (EveryInc#1397)

* fix(review): answer covered cases on skill prose with the condition, not a patch (EveryInc#1401)

* fix(scratch): fall back to $TMPDIR when /tmp cannot host the scratch root (EveryInc#1398)

* feat(ce-skill-work): repo-local skill for authoring, editing, reviewing, and responding to review on skills (EveryInc#1402)

* fix(ce-pov): reject non-final peer positions instead of folding them in (EveryInc#1403)

* feat(manifest): add Agent Plugins v1.0.0 manifest support (EveryInc#1345)

* chore: release main (EveryInc#1354)

* fix(ce-work): run cross-model verification on warm checkouts (EveryInc#1404)

* fix(orca): reconcile upstream skill contracts

* fix(orca): preserve additive Codex session roots

---------

Co-authored-by: Trevin Chow <trevin@trevinchow.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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