Repository navigation
Retain installer failure results and map crashes to Core source - #2888
Merged
Merged
Conversation
trissim
marked this pull request as ready for review
October 23, 2026 20:24
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
debt-src at e626852, every finding: |
This was referenced Oct 23, 2026
trissim
added a commit
that referenced
this pull request
Oct 23, 2026
…sult through Merge origin/main (#2888 retained failure outcomes) and finish the second send-back on #2841. - ProbePhase(probe: type[RuntimeProbe]), CarryPhase(operation: type[CarryOperation]), new CarryStepPhase(step: type[Carry]) and OwnersPhase(command: type[CliCommand]) hold the class itself. Each is declared beside the family it names; FieldCodec carries it as the declared name and decodes it back to the class. label() reads the declared name through subject() only when the report renders it. - InstallRun.launch writes the child's result verbatim again, as on main. The old installed runtime decoding the target's RehearsedUpgrade could not resolve a probe or carry step it predates ("Unknown RuntimeProbe name"), so InstallerProcessPhase and the launcher include are deleted; the launcher's own phases still reach the child through InstallRun.phases. - Ruff findings the branch added are cleared; black on interpreter_operation.py and carry.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Supersedes #2556. Two implementation commits port its remaining reporting changes; merge bunched with current main e93892c without conflicts. Keep #2556 open until this merges.
55ca03ffd,installer/launch.py:76-87,93: select the last Core traceback frame and map it through Repo.module_file using the TargetTree's source repository. Preserve exception type/message and the original line number.944499c40,installer/launch.py:59-61,99-105,installer/reporting.py:32-63,installer/upgrade.py:342,365,828: retain a returned LiveRun before bridge resumption/worker retirement/harness registration, and keep it when recording a failure. The existing result variants supply live lines; rehearsals invent no live result. Existing bridge output and writer selection stay with their owners. A failed report publication still annotates and rethrows the original exception.#2556 remainder: no additional functional P404 change remains to port. Comparison used its original
46391711d...c6cd8d4f9diff (12 production files and 8 test/support files), checked against this head:The old permanent InstallFailure/CarriedInstallError/SourceWriter alternatives are not needed to preserve those outcomes under the current ownership. Do not restore the old writer hierarchy, CommanderConfig class or outcome.py module. The eight old test/support diffs are historical verification code, not additional production features; the review fork owns current execution checks.
Source inventory used refactor-audit Package.load: 449 Core modules, 486 test modules and 11 tool modules at the fresh base, zero parse omissions. Changed modules parse through AST; git diff --check is clean. No tests, practice installs or local ratchets ran in Drafts.
Assumed: errors crossing InstallReport.running contain a Core traceback frame; Repo.module_file owns Core's source layout; only returned LiveRun values are retained and partial/rolled-back work keeps its existing failure description; existing writer and bridge ownership remain authoritative. Source evidence is not an execution result.
Send-back fixed: c60cd15 failed the review fork with 7 failed / 10 passed because I read nonexistent TargetTree.checkout. The correction constructs Repo from the declared TargetTree.path (upgrade.py:625); no alias, fallback or exception handler was added. The AST inventory never proved attribute existence, and the earlier source-parsing statement was not execution evidence. The review fork reruns the two files; Drafts ran no tests.
Attribute reads checked by opening the declarations:
Assumed after correction: errors crossing running contain a Core traceback frame; the declared Repo source layout maps that frame after tree deletion; only returned live results are retained. The accessed project attributes above are source-confirmed declarations, not assumptions. No tests were run by Drafts.