S2: Owen.CSharp.Rewriter — deterministic acquire rewrite (postimage + report)#288
Merged
Conversation
…mage + report)
The load-bearing S2 slice (steps 4-7): a SEPARATE Roslyn project — the extractor stays
strictly read-only, and this never writes into the source tree.
owen-rewrite --plan <validated-plan.json> --candidates <candidates.json>
--root <source-root> --out <artifact-dir>
It reads the hash-bound pair (the Python gate has already validated + bound them),
RE-checks the pristine preimage SHA against the exact bytes it parses — closing the
TOCTOU the Python check cannot cover — and runs the span-node identity guard for every
convert_acquire decision:
* the span resolves to exactly one node whose Span == the span and which is an
AddAssignmentExpression;
* no comment/directive trivia inside the replaced expression (refuse rather than move it);
* the LHS is an event member access (or a bare event on implicit `this`);
* the event NAME matches the candidate;
* the receiver text and the handler text match the hash-bound candidate (normalized);
* the acquire is statement-form (an expression-bodied `=> a += h` is a valid but
UNSUPPORTED form in this slice and is refused, not guessed at);
* the statement's containing-type FQN (built from syntax) matches the candidate;
* convert_acquire spans never overlap.
Syntax-only by design: the INotifyPropertyChanged contract + the convert_acquire
permission were proven in S0 and ride in the hash-bound candidate, so no SemanticModel
(and no shared FixCore) is needed yet.
The edit replaces only the assignment expression with `<target>(<receiver>, <handler>)`,
so the trailing `;`, the indentation and every byte of trivia outside it are untouched;
`<target>` comes solely from validated-plan.target_api.subscribe. Decoding matches the
extractor's (BOM-detecting UTF-8) so the spans align; a non-UTF-8 source is refused; the
original BOM presence and the file's own line endings are preserved. All edits are
computed and guarded BEFORE any is applied, then applied via SourceText.WithChanges and
independently re-derived to prove the postimage is the source plus EXACTLY the declared
edits. Any failure refuses the whole operation (exit 2, nothing written) — never partial.
Emits <out>/postimage/<path> + <out>/rewriter-report.json (deterministic: version,
operation, input_bundle_sha256, validated_plan_sha256, target_api, source pre/post
sha256, applied + manual_review findings; no timestamps/paths/ids).
Verified locally + a new CI step over the full chain (extractor --fix-candidates ->
own-fix candidates -> validate_plan -> owen-rewrite): both acquires convert to the
wrapper, a re-run is byte-identical, the source tree is untouched, and a stale preimage
SHA is a hard refusal that writes nothing.
Next slice: the Python `own-fix subscriptions apply` orchestration — change.patch +
apply-manifest (patch_sha256) + the self-gates (diff guard, analyzer delta, patch
verification), then the 007 target-gate contract.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
![chatgpt-codex-connector[bot]](https://cdn.github.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: 7f3c142f52
ℹ️ 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".
| if (!EditsAreExact(sourceText, edits, postText)) | ||
| return Refuse("post-image differs from the source outside the declared edits"); | ||
|
|
||
| string outPost = Path.Combine(outDir, "postimage", relPath); |
There was a problem hiding this comment.
When owen-rewrite is invoked directly, no code in this executable runs the repository's validate_apply_inputs root-confinement gate, and the new CI workflow also calls the rewriter immediately after validate_plan. A crafted absolute or .. source path with a matching SHA is therefore accepted; because a rooted relPath makes Path.Combine discard the postimage prefix, this write can target the original source or another arbitrary file, violating the command's central guarantee that it never writes into the source tree. Revalidate/constrain the path inside this process before both reading and constructing the output path.
Useful? React with 👍 / 👎.
Comment on lines
+95
to
+96
| using (var reader = new StreamReader(new MemoryStream(sourceBytes), Encoding.UTF8, | ||
| detectEncodingFromByteOrderMarks: true)) |
There was a problem hiding this comment.
For a BOM-less legacy-encoded or otherwise invalid UTF-8 source, Encoding.UTF8 silently replaces invalid bytes with U+FFFD while CurrentEncoding.CodePage remains UTF-8, so the check below accepts the file. Re-encoding the postimage then changes bytes outside the declared edit spans, and EditsAreExact cannot detect this because it compares only the already-corrupted decoded text. Use a strict UTF-8 decoder or verify a byte-for-byte decode/re-encode round trip before rewriting.
Useful? React with 👍 / 👎.
Comment on lines
+170
to
+172
| var handlerText = asg.Right.ToString(); | ||
| if (NormWs(handlerText) != NormWs(cand.GetProperty("handler").GetString()!)) | ||
| return Refuse($"{fid}: handler does not match the candidate handler"); |
There was a problem hiding this comment.
For supported subscriptions written as pub.PropertyChanged += new PropertyChangedEventHandler(OnChanged), the extractor's NormalizeHandler strips the delegate construction and stores OnChanged in the candidate, but this guard compares that value with the raw asg.Right text. Consequently a validated candidate generated by the extractor is always refused for this documented and common handler form; apply the same normalization used by the extractor before matching and emitting the handler.
Useful? React with 👍 / 👎.
Comment on lines
+178
to
+179
| var typeFqn = SyntacticTypeFqn(asg); | ||
| if (typeFqn != cand.GetProperty("containing_type").GetString()) |
There was a problem hiding this comment.
For a subscription inside a generic type such as N.ViewModel<T>, the extractor's FixFqnFormat explicitly includes type parameters in containing_type, while SyntacticTypeFqn collects only each declaration's identifier and produces N.ViewModel. Generic classes are not rejected by candidate collection, so every otherwise-valid conversion in one fails this containing-type guard; construct the syntax name with its type parameter list or compare against an equivalently normalized candidate name.
Useful? React with 👍 / 👎.
The rewriter is the load-bearing step, so it may not assume the Python gate ran. Every guarantee it makes it now proves for itself, over the exact bytes it parses. 1. SELF-CONTAINED input / hash / root validation. The CLI took raw plan + candidates on faith (and CI called it with no validate_apply_inputs at all): a malformed shape could surface as a GetProperty exception, `manual_review` needed no candidate, and a rooted or `..` path in `postimage/<rel>` could have thrown the write clean out of the out-dir — the "never writes the source tree" promise rested on the JSON's good manners. It now RE-DERIVES the canonical plan projection from the bundle and requires the plan to BE it: bundle_sha256 binding (a canonical-JSON writer matching Python's json.dumps producer byte for byte), exact envelope key sets, fixed constraints, a total ordered decision<->candidate bijection, per-candidate action permissions, canonical root-relative source path, symlink-resolved root confinement, an out-dir that is neither the root nor inside it nor already populated, and a postimage destination confined to the out-dir. Every shape failure is exit 2, never a traceback. validated_plan_sha256 now hashes the SAME planBytes that were parsed (it re-read the file at report time). 2. STRICT UTF-8. `new StreamReader(..., Encoding.UTF8, ...)` silently replaces invalid bytes with U+FFFD and still reports CodePage 65001, so the encoding check passed and the post-image could re-encode bytes OUTSIDE the declared edits — invisibly to EditsAreExact, which compares the already-corrupted decoded text. Now a throwing decoder (UTF8Encoding(false, throwOnInvalidBytes: true)); DecoderFallbackException and a UTF-16 BOM are controlled refusals. 3. TARGET API GRAMMAR, before the replacement exists. `target_api.subscribe` was an arbitrary plan string interpolated straight into C#. It is now parsed with Roslyn and must be a canonical identifier / member-access chain and nothing else — no invocation, arguments, generics, `?.`, operators, object creation or trailing junk — and the replacement is built from the VALIDATED syntax node via SyntaxFactory, not string interpolation. 4. EXTRACTOR-COMPATIBLE normalization. Handler/receiver were compared as whitespace- stripped raw text, and SyntacticTypeFqn dropped type parameters — so a candidate for `N.ViewModel<T>` was compared against `N.ViewModel` and could never match. Both sides are now re-derived with the PRODUCERS' own derivations (extractor `a.Right.ToString()` for handler, the collector's last-'.' split for source, `FixNormWs`+`NormalizeHandler` for the computed identities, and a type FQN carrying its type parameter list the way SymbolDisplay prints it), so the comparison is exact over SHA-pinned bytes rather than a lossy re-normalization. The weak wrapper now receives the NORMALIZED handler: `new PropertyChangedEventHandler(OnChanged)` reaches it as `OnChanged`. 5. TRANSACTIONAL publication. postimage-then-report meant a failed report write left the postimage behind, breaking "nothing written". The whole bundle is now staged in a sibling dir and published with ONE atomic rename; any failure discards the staging dir entirely, and a pre-existing out-dir is a hard refusal so runs cannot mix. Regressions (tests/rewriter_regressions.sh, new CI step) drive the real chain end to end and cover all five groups: 12 input/hash/path refusals — including path confinement against an attacker who owns BOTH files (bundle re-hashed so only the rewriter's own check can refuse) — 8 target-grammar refusals, out-dir confinement + a failed publication leaving no out-dir and no staging dir, the normalization forms (new samples/ FixRewriteSample.cs: explicit delegate construction, `this.`-qualified handler, generic containing type, and the nested type S0 itself refuses), and encoding (BOM preserved, all CRLF endings preserved with no lone LF, invalid UTF-8 refused). Plus the happy path, byte-identical re-run, source tree untouched, and the stale-SHA refusal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a deterministic .NET subscription rewriter, canonical patch-bundle builder, ChangesSubscription rewrite and patch bundle
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as own-fix subscriptions apply
participant Bundle as apply_bundle
participant Rewriter as owen-rewrite
participant Source as Source tree
participant Output as Patch bundle
CLI->>Bundle: parse and validate apply inputs
Bundle->>Rewriter: execute with plan and candidates
Rewriter->>Source: read and rewrite confined source
Rewriter->>Bundle: return postimage and report
Bundle->>Output: publish canonical patch, manifest, and postimage
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ 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 |
![github-advanced-security[bot]](https://cdn.github.com/in/57789?s=60&v=4){kind=small}
| { | ||
| public ExplicitDelegate(IPub pub) | ||
| { | ||
| pub.PropertyChanged += new PropertyChangedEventHandler(OnChanged); |
| { | ||
| public ThisQualified(IPub pub) | ||
| { | ||
| pub.PropertyChanged += this.OnChanged; |
| { | ||
| public GenericHolder(IPub pub) | ||
| { | ||
| pub.PropertyChanged += OnChanged; |
| { | ||
| public Inner(IPub pub) | ||
| { | ||
| pub.PropertyChanged += OnChanged; |
| { | ||
| public Inner(IPub pub) | ||
| { | ||
| pub.PropertyChanged += OnChanged; |
…ore the RHS `local name="$1" root="$T/u_$name"` reads $name while it is still unset, so `set -u` aborted the encoding section. Assign one per line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…table
The two unclosed fragments of the first blocker. Both are the same mistake in different
clothes: trusting a proof that does not prove what it was being asked to prove.
1. CONFINEMENT WAS LEXICAL AT INTERMEDIATE SYMLINKS. ResolveLinkTarget(path, true)
returns null when `path` itself is not a link — it is not os.path.realpath. So with
root/linked -> /outside
the final `root/linked/source.cs` is not a link, the old resolve returned the lexical
path, and the escape was called confined. `--out` was worse: PrepareOutDir used
Path.GetFullPath + a string IsInside only, so a parent linking INTO the source root
could have put the staging dir and the published bundle physically in the tree while
the string still looked external.
RealPath now resolves EVERY existing component: on each link it restarts from the
target's root with the target's segments ahead of the remaining ones (so a link whose
own target sits behind further links still resolves), and `..` is applied AFTER the
prefix resolves — which is what makes it physical rather than textual. The source must
physically resolve inside the physical root. The out-dir's parent is resolved first,
proven off the tree, and the staging dir + out-dir are then built under that VERIFIED
physical parent — and the parent is re-proven immediately before the publishing rename.
2. THE FROZEN ENVELOPE WAS NOT RESTATED. ValidateBundleShape checked some strings and a
non-empty allowed_actions, and an action was "permitted" merely by appearing in the
candidate's own list. But the hash binding proves the plan and the candidates agree
with EACH OTHER; it says nothing about whether the candidates obey the permission
policy. A self-consistent forgery could carry `event_contract: name_only` with
`allowed_actions: ["convert_acquire"]`, re-hash, and be honoured. Cryptography is not
a substitute for meaning. The bundle is now checked against the frozen contract:
version/operation, exactly one allowed type and one source file, type.file ==
source.path (each was previously compared only against its own copy, so type -> A.cs /
source -> B.cs was possible), selected_findings null-or-exactly-these-ids, known
event_contract, allowed_actions subset of the S1 enum with no duplicates, and
convert_acquire ONLY for a proven inotify_property_changed contract — whatever the
bundle claims to allow. Every candidate must belong to the one selected (type, file).
The controlled-refusal contract is closed too: Path.GetFullPath / LinkTarget failures
(a NUL in a path) are normalized to a refusal, span bounds are checked as
`start >= 0 && length >= 0 && start <= len && length <= len - start` BEFORE
`new TextSpan` can throw on the overflow, and a final catch-all turns any
unanticipated exception into exit 2 rather than a traceback. Fail closed.
Regressions: two intermediate-symlink cases (source reached through `root/linked ->
outside` with the bundle re-hashed; an `--out` whose parent links into the repo — refused,
with the tree untouched), five frozen-envelope forgeries (forged name_only tiering,
type/source file drift, malformed selected_findings, unknown contract, action outside the
enum), plus the NUL path and the overflowing span. Each forgery is fully self-consistent
and freshly hash-bound, and its plan is hand-built as the canonical projection — because
validate_plan, correctly, will not produce one from a forged bundle. So only the
rewriter's own restatement of the contract can refuse them, which is exactly the property
under test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…timage)
Turns the accepted rewriter's postimage into a deterministic, reviewable, hash-addressed
artifact for the later step-9 self-gates and the 007 gate:
python -m ownlang own-fix subscriptions apply \
--plan <validated-plan.json> --candidates <candidates.json> \
--root <source-root> --out <artifact-dir> [--rewriter <owen-rewrite-command>]
<out>/change.patch <out>/apply-manifest.json <out>/postimage/<canonical-rel-path>
Thin orchestration only — no model, no o7, no analyzer, no gate.toml. It re-runs the
accepted validate_apply_inputs, invokes the accepted Owen.CSharp.Rewriter as an argv
VECTOR (never a shell string, so nothing in a path can be read as syntax), verifies the
transport output, then builds and publishes the bundle.
rewriter-report.json is transport, and is verified rather than believed — a neighbouring
executable produced it, which is not the same as it being true. Checked: exact key set,
version/operation, input_bundle_sha256, validated_plan_sha256, target api, source path,
pre-SHA, a RECOMPUTED post-SHA over the actual postimage bytes, applied/manual as sets
with the expected partition, a postimage for the allowed path ONLY, and no unexpected file
anywhere in the rewriter workdir. The validated plan stays the sole authority for action,
the hash-bound candidates the sole authority for identity; the report contributes nothing
to the manifest.
change.patch is generated in-process (difflib over byte-lines, git headers) rather than
shelled out to `diff`, so it cannot inherit a timestamp column, a temp path, a username or
a cwd. Lines split on b"\n" ONLY — git's own notion of a line — because bytes.splitlines()
also breaks on a lone \r (and \v, \f, \x1c), which would silently reflow a file whose
content carries one. A missing EOF newline gets git's own marker on both sides. The result
is a/<rel> -> b/<rel>, one file, no rename/mode/binary records, and `git apply` reproduces
the postimage byte for byte.
apply-manifest.json is the authority artifact: exact shape, sort_keys + compact separators
+ trailing newline, built ONLY from the validated plan (actions, in candidate order), the
plan's own bytes, and the real postimage/patch bytes. No timestamps, no absolute or temp
paths, no hostname, run id, command line or token counts — a re-run over the same pristine
inputs is byte-identical.
An all-manual_review plan is VALID, not a refusal: postimage == preimage, change.patch is
zero length, patch_sha256 is the sha of no bytes, applied_findings is empty and every id
stays in candidate order under manual_review_findings.
Publication is atomic: everything is built in a sibling work directory and the finished
staging bundle is renamed into place, so <out> never exists until every check has passed.
An existing <out>, an <out> physically inside the source root (symlinks resolved), or any
rewriter/patch/manifest/write/rename failure leaves no <out> at all, no work directory,
and a byte-identical source tree.
Tests: tests/test_patch_bundle.py (52 checks) covers the patch/manifest pure functions and
every transport-tampering case — those live at the pure-function level ON PURPOSE, since
forging a bad report must not require a test-only hook in the production rewriter.
tests/patch_bundle_regressions.sh + a CI step drive the real chain: the three-part bundle,
canonical headers with no temp/absolute path or timestamp, exact manifest shape and SHAs,
`diff -r` determinism, a git apply round-trip on a pristine copy, mixed actions, the empty
patch, and the refusal/no-output cases.
Steps 9-12 are NOT started: no analyzer delta, no own-check re-run, no OWN001/OWN050
assertions, no gate.toml, no 007 evidence, no STS clone, no apply to a real branch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1722-1737: The S2 stale-preimage workflow test currently mutates
every SHA, allowing an earlier validation failure instead of exercising the
source comparison. Update the step around the stale.json generation and dotnet
run to create matching plan and candidate data where only the source SHA is
replaced with a stale value while dependent hashes and bindings remain valid,
then assert exit code 2 and verify output contains “STALE SOURCE / PREIMAGE
MISMATCH”; preserve the checks that no output directory or source changes
remain.
In `@frontend/roslyn/Owen.CSharp.Rewriter/Program.cs`:
- Around line 679-695: Update PrepareOutDir and the corresponding staging-write
flow around Publish to claim staging exclusively with an unpredictable,
process-owned directory or equivalent no-follow/held-handle mechanism. Ensure
all Directory.CreateDirectory and File.WriteAllBytes operations target the
claimed staging location and cannot be redirected if another process replaces
the predictable path; preserve the existing refusal behavior for conflicting
output paths.
- Around line 651-653: Update IsInside to perform platform-aware containment
using Path.GetRelativePath and Windows case-insensitive comparison, while
retaining ordinal comparison on case-sensitive platforms. Ensure the check
accepts the directory itself and descendants but rejects paths outside the
directory, including Windows paths differing only by case.
In `@ownlang/__main__.py`:
- Around line 582-583: Normalize parsing in the --rewriter handling near
rewriter and apply_bundle so it uses one documented argv grammar consistently
across platforms, without preserving quote characters in executable or argument
tokens. Catch shlex splitting errors such as unbalanced quotes and convert them
to the existing ApplyError path with exit status 2, while preserving the
resulting argv vector for apply_bundle().
In `@ownlang/fix_bundle.py`:
- Around line 88-93: Update the diff-header generation around the rel path
handling to prevent unsafe or Git-quoted filenames from being emitted unescaped.
Either apply Git-compatible quoting to rel consistently in the diff --git, --- ,
and +++ headers, or reject unsupported names before this block; preserve the
existing path validation and ensure no control characters or other unquoted path
content reaches the patch.
In `@tests/rewriter_regressions.sh`:
- Around line 107-113: Update the malformed-target cases in the
fixture-generation loop so each case changes both the plan target and its
corresponding candidate target. Recompute input_bundle_sha256 after those
candidate changes, and ensure validation receives the per-case candidate file so
ValidateCanonicalPlan reaches the intended target validation path.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f5aa706-6297-4f3d-b208-86dbbfe95380
📒 Files selected for processing (9)
.github/workflows/ci.ymlfrontend/roslyn/Owen.CSharp.Rewriter/Owen.CSharp.Rewriter.csprojfrontend/roslyn/Owen.CSharp.Rewriter/Program.csfrontend/roslyn/samples/FixRewriteSample.csownlang/__main__.pyownlang/fix_bundle.pytests/patch_bundle_regressions.shtests/rewriter_regressions.shtests/test_patch_bundle.py
… honest fixtures
The four corrections plus the two misleading regressions. Nothing else in steps 4-8 moves.
1. PATH CONTAINMENT WAS CASE-SENSITIVE EVERYWHERE. Case sensitivity is a property of the
PLATFORM, not of the string: `C:\Repo` and `c:\repo` are one directory on Windows and
two on a case-sensitive filesystem. Comparing ordinally everywhere let `c:\repo\out`
slip past a `C:\Repo` root in both implementations. Python now compares through
os.path.normcase (the platform rule itself; identity on POSIX); C# selects
OrdinalIgnoreCase on Windows and Ordinal elsewhere, behind one SameOrInside helper that
also folds in the old separate `== root` equality test.
2. PATCH FILENAMES WERE EMITTED UNESCAPED. The canonical-relative-path contract does not
exclude a tab, newline, DEL or unpaired surrogate — all legal in a real filename — and
any of them produces headers git cannot parse back. The bundle would have shipped a
change.patch that does not apply while step 8 promises every published patch does. Git's
answer is C-quoting; this slice's answer is to refuse the file, which is honest about
what it supports. Refused in canonical_patch (it is public and must hold alone) AND
early in apply_bundle, before the rewriter is spawned for a bundle that could never
publish.
3. --rewriter PARSING WAS PLATFORM-FORKED AND COULD TRACE BACK. `posix=(os.name != "nt")`
made one documented command line mean two things — posix=False keeps the quotes around a
quoted exe path, so they land INSIDE argv[0] — and an unbalanced quote raised ValueError
straight past the ApplyError handling. Now ONE grammar everywhere (POSIX word-splitting,
quotes removed, documented: quote a path with spaces or backslashes), in a named
function, with parse errors mapped to `own-fix: refuse:` + exit 2.
4. C# STAGING WAS PREDICTABLE AND CHECKED-THEN-WRITTEN. The deterministic name was tested
for existence and only later written into: between the check and the first write, anyone
able to guess it could drop a link there and redirect the "isolated" postimage into the
source tree — a late-discovered hole in the locked "never writes the source tree"
invariant. Staging is now claimed immediately under the already-verified physical parent
with an unpredictable name (16 random bytes), created owner-only on Unix as part of the
mkdir itself, and then PROVEN to be ours: not a link, resolving to itself, empty. Every
write happens inside the claimed directory; the rename and cleanup are unchanged.
TEST EVIDENCE. Two regressions were green through the wrong branch, which is the same as
not testing the branch at all:
* the stale-SHA workflow step rewrote every sha256-looking value, so the hash BINDING
refused first. It now changes the preimage SHA in both files, re-binds the bundle hash,
and asserts the message is STALE SOURCE / PREIMAGE MISMATCH.
* the target-grammar fixtures mutated only the plan, so the plan-vs-bundle projection
check refused first. They now mutate both and re-bind, so only ValidateTargetApi can
object — verified: all eight now refuse with the grammar message, where before they
reported "plan.target_api.subscribe does not match the candidates bundle".
New regressions: 7 unpatchable filename classes x (changed + unchanged file) + a space is
still supported; platform-aware containment incl. the name-prefix trap (/repo vs
/repository) and the fs root; the --rewriter grammar (bare, extra args, quoted exe path
with spaces, quoted Windows path, unterminated quote). End to end: a REAL on-disk source
whose filename carries a tab, reached through a hash-bound bundle -> exit 2, no bundle,
source unchanged; a quoted exe path with spaces produces the same bundle; an unterminated
quote and a missing executable are refusals, not tracebacks. tests/test_patch_bundle.py is
now 79 checks.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
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.
S2 —
Owen.CSharp.Rewriter: deterministic acquire rewrite (steps 4–7)The load-bearing S2 slice. A separate Roslyn project (the extractor stays strictly read-only), named per the arbitrated brand (
Owen.CSharp.Rewriter, notOwnSharp.*, not a language-independentOwen.Rewriter). It never writes into the source tree.Reads the hash-bound pair (the Python gate validated + bound them), re-checks the pristine preimage SHA against the exact bytes it parses — closing the TOCTOU the Python check can't — then runs the span-node identity guard per convert_acquire:
Span == spanthat is anAddAssignmentExpression;this); event name matches the candidate;=> a += his refused, not guessed;Syntax-only by design: the INPC contract + convert_acquire permission were proven in S0 and ride in the hash-bound candidate — no SemanticModel (and no shared FixCore) needed yet.
The edit replaces only the assignment with
<target>(<receiver>, <handler>)(<target>solely fromvalidated-plan.target_api.subscribe), so the trailing;, indentation and all surrounding trivia are untouched. Decoding matches the extractor's (BOM-detecting UTF-8) so spans align; non-UTF-8 is refused; BOM presence + line endings preserved. All edits are computed and guarded before any is applied, then applied viaSourceText.WithChangesand independently re-derived to prove the postimage is the source plus exactly the declared edits. Any failure refuses the whole operation (exit 2, nothing written).Emits
<out>/postimage/<path>+<out>/rewriter-report.json(deterministic — no timestamps/paths/ids).Verified (locally + a new CI step over the full chain extractor → own-fix candidates → validate_plan → owen-rewrite): both acquires convert to the wrapper, a re-run is byte-identical, the source tree is untouched, and a stale preimage SHA is a hard refusal that writes nothing.
Next slice: the Python
own-fix subscriptions applyorchestration —change.patch+apply-manifest(patch_sha256) + self-gates (diff guard, analyzer delta, patch verification), then the 007 target-gate contract.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
own-fix subscriptions applyto generate publishable fix bundles.Bug Fixes