feat(resume): add trusted resume validation and transition lineage (#786) - #845
Merged
Merged
Conversation
Contributor
|
✅ OpenCodeReview: Review partially complete: 0 finding(s); 1 of 9 selected item(s) failed. |
Collaborator
|
@Gongyl01 rebase main |
`ocr review --resume` admitted a session whenever the ref text matched, which is neither sufficient nor necessary evidence about the input: `abc1234` and `abc1234def` name the same commit, while a branch name that did not change can name a new one. Resuming then mixed comments computed from one input with comments computed from another, and nothing in the report distinguished them. Compare the resolved input identity instead. agent.ResolveIdentity replays the run's own selection — the same diff load and the same two filter passes — and returns the identity a real run would record, so the parent manifest and the child candidate are directly comparable. Any mismatch on mode, repository, source artifact or rule config rejects the whole resume rather than degrading to partial reuse. A provider or model change must be asked for with --provider or --model; one that arrived through config or the environment is rejected. Two behaviour changes fall out of this. Ref text no longer decides admission, so ValidateOptions only checks the review mode. A parent that completed zero items is now admitted: its manifest is verifiable, so its selected set is simply re-dispatched, which is the case resume exists for. Reuse is then gated on the parent manifest rather than on the checkpoint lines: only a fingerprint the manifest claims as completed or reused is reused, which keeps the manifest the single source of coverage truth. That gate is also what makes an unreadable checkpoint survivable, so review loads through LoadReviewResumeState, which drops lines it cannot parse — the file such a line described is simply reviewed again, instead of one truncated write costing every other file its checkpoint. Scan keeps the strict LoadResumeState, because with no manifest to arbitrate, a dropped line cannot be told apart from a checkpoint that was never written. Rejection happens strictly before agent.New, because session.New writes session_start the moment it is called — validating any later would leave an orphan session behind every rejection. Keeping it there needs the run to review exactly what was admitted, so the pre-flight hands back the commit endpoints it resolved that identity from, and a resumed run loads its diff from those instead of from the refs the user typed; file_read reads at the same sealed head. Both loads then see the same immutable objects, so a ref moving after admission can no longer change what the run reviews, and no mid-run re-check is needed to discover that it did. An accepted resume records one resume_lineage event naming the parent run and the provider/model endpoints, surfaced by `ocr session show`; it carries non-secret labels only. Interrupted runs become unresumable, since session_end is the sole carrier of run_manifest. That is deliberate: an unverifiable input is exactly what this change refuses to build on, and the error says so rather than reporting the parent as unproductive. Refs alibaba#786
Gongyl01
force-pushed
the
feat/trusted-resume
branch
from
August 13, 2026 08:52
9c82d0b to
0413e7b
Compare
Contributor
Author
I’ve rebased it, and CI is now green. It’s one commit behind again, but no further rebase should be necessary. |
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
2 participants
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.
Summary
Closes #786.
Part of #368.
Makes
ocr review --resumevalidate the complete resolved run identity before reusing checkpoints.--provideror--modelflags for provider/model transitions and records accepted transitions in oneocr.resume-lineage/v1session event.RunManifestas the single source of coverage truth and does not change its schema or terminal-state rules.Workspace, scan, manifest-less, aborted, legacy, and unknown-schema resume parents remain unsupported and are rejected with guidance to start a new review.
How to test
make test make coverage go vet ./... git diff --check upstream/main...HEADChecklist
go vet,gofmt,go mod tidy, and license checks passRelated Issues
Closes #786
Part of #368