S2 Step 11 — Verified Target Wrapper gate (own-fix subscriptions verify-target) - #291
Merged
Conversation
The fixed C# probe for Step 11's Verified Target Wrapper gate. Two internal modes,
no code generation / build / restore inside verify-target:
- bind (G1, G2): parses the pristine preimage (syntactic) and the accepted Step 8
postimage (semantic), locates each converted candidate's AddAssignment at its
hash-bound acquire_span, reproduces the frozen Step 8 replacement text, computes
each derived postimage span via ordered non-overlapping edits + cumulative length
deltas, requires exactly one invocation node filling that span matching the target
+ source + normalized handler, resolves its IMethodSymbol, enforces the per-finding
bijection (all callsites one symbol; not source-defined), derives the wrapper slot
ordinal from the resolved assembly (first-simple-name-wins), and emits a canonical
binding-result.json with a sorted callsites array. Framework references come from
the selected runtime (TPA) the probe is pinned to, plus the ordered reference slots.
- probe (G3, G4, F4): loads the derived wrapper from its EXACT materialized slot path
via a dedicated AssemblyLoadContext (deps by first-wins across slots, framework from
the selected runtime), runs the runtime-compatibility preflight (resolve type + exact
method, build delegate, RuntimeHelpers.PrepareMethod; loader failures -> exit 10 =
WRAPPER_RUNTIME_UNSUPPORTED), then the frozen GC harness for ONE attempt: strong
control, collectability control, and the target attempt (deliver once while alive,
drop the subscriber in a NoInlining/NoOptimization helper, 5 collection rounds with
the fixed allocation pressure, GC.KeepAlive(source), then require the subscriber
WeakReference is dead). Emits a canonical probe-result.json with the actually-loaded
resolved_wrapper identity (ordinal, slot_sha256, MVID, metadata token, signature).
Smoke-validated locally: a genuine weak wrapper -> subscriber_collected true; a strong
decoy -> false; bind and probe emit byte-identical signatures. No frozen file touched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
The Python half of Step 11. ownlang/fix_target.py reuses the frozen Step 8/9/10
helpers by import (nothing frozen is touched) and drives:
- load_authority + the OWN001-only candidate guard;
- bind_delta (F1/F2): canonical Step 10 delta, exact schema, all seventeen checks
pass, hashes/target/expected bound to THESE plan/candidates;
- bind_bundle (F1): frozen Step 8 layout + the four bundle hashes bound to the
delta + the target rel equals the frozen Step 10 analysis scope;
- reference_closure (F1): reconstruct via the frozen Step 10 ordering and require
semantic equality with delta.reference_closure (REFERENCE_BINDING);
- conditional inputs (F2): converted needs --probe-dll + --wrapper-ordinal;
manual-only forbids them and runs no bind/probe;
- snapshot_probe_deployment + resolve_probe_runtime (G2): the probe runtime must
match delta.resolved_runtime_identity;
- build_bind_params + run_bind (G1): the Roslyn callsite bijection; derived ordinal
cross-checked against --wrapper-ordinal (caller ordinal is only an assertion);
- three isolated run_probe_attempt children + classify (G3/G4/F5): actual-loaded
identity cross-check, controls, and the exact classification precedence
(WRAPPER_RUNTIME_UNSUPPORTED / HARNESS_INVALID / TARGET_BEHAVIOR / TARGET_RETAINS
/ HARNESS_NONDETERMINISM / pass);
- converted + manual-only target-result serializers (executed-check tracking);
- _publish_target (G5): protected-root exclusion, EXECUTION_WORK_ROOT removed
before publication, single atomic rename, honest cleanup-failure PUBLICATION.
Adds the `own-fix subscriptions verify-target` CLI verb. Validated end-to-end over a
real chain: a genuine weak wrapper -> status pass; a strong decoy -> TARGET_RETAINS.
ruff + mypy clean. No frozen file touched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
Tier A (SDK-free) drives fix_target's manual-only path end to end plus the classify precedence matrix, the converted/manual-only serializers, the atomic publisher, and the handler peel. Tier B drives the real PUBLIC CLI only (extractor -> candidates -> validate-plan -> apply -> gate -> verify-delta -> verify-target) over separately-compiled wrapper fixtures shipped as reference slots: weak -> pass, strong -> TARGET_RETAINS, no-op/twice/throwing -> TARGET_BEHAVIOR, wrong-signature -> WRAPPER_BINDING, source-defined -> CALLSITE_BINDING, two converted callsites -> pass, net9 / missing-dependency -> WRAPPER_RUNTIME_UNSUPPORTED, manual-only -> pass (six probe checks not_applicable), two runs -> byte-identical. The wrong-signature case is RED against the current bind: a three-parameter wrapper surfaces as WRAPPER_RUNTIME_UNSUPPORTED (the probe method-select throws) instead of the contract's WRAPPER_BINDING. The next commit adds the TARGET_BINDING shape check that turns it green. Wired REQUIRED into the wpf-extractor CI job (OWN_TIERB_REQUIRED=1); adds the verify-target row to spec/CLI.md and installs the 9.0 SDK there purely to build the deliberately-incompatible net9 wrapper fixture. Frozen Steps 0-10 untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…BINDING) The accepted contract's TARGET_BINDING literal requires plan.target_api.subscribe, resolved by name inside the selected reference-slot wrapper, to have the exact accepted shape: exactly one public type; exactly one public static, non-generic void(INotifyPropertyChanged, PropertyChangedEventHandler) method; no other overload; no ref/optional/params/custom-modifier parameter. Any mismatch is WRAPPER_BINDING. bind now resolves the converted callsite symbol with a CandidateSymbols fallback (so a failed overload still yields the candidate wrapper method) and validates its containing type via TargetBinding.Validate before deriving the ordinal. A wrong-signature or overloaded wrapper is therefore refused WRAPPER_BINDING at bind time rather than surfacing as an unresolved callsite (CALLSITE_BINDING) or a runtime load failure (WRAPPER_RUNTIME_UNSUPPORTED). The genuine weak / strong / no-op / throwing paths are unchanged. Turns the Tier B wrong-signature case green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
📝 WalkthroughWalkthroughAdds ChangesVerified target wrapper
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant VerifyTarget
participant RoslynProbe
participant Wrapper
participant Evidence
CLI->>VerifyTarget: invoke verify-target
VerifyTarget->>RoslynProbe: bind frozen target
RoslynProbe->>Wrapper: resolve wrapper method
VerifyTarget->>RoslynProbe: run runtime probes
RoslynProbe-->>VerifyTarget: return binding and behavior results
VerifyTarget->>Evidence: publish target-result.json
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36cea9aeaa
ℹ️ 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 on lines
+174
to
+176
| var si = postModel.GetSymbolInfo(inv); | ||
| var sym = si.Symbol as IMethodSymbol | ||
| ?? si.CandidateSymbols.OfType<IMethodSymbol>().FirstOrDefault(); |
There was a problem hiding this comment.
Reject unresolved or ambiguous wrapper invocations
When the generated call is ambiguous or otherwise fails overload resolution, si.Symbol is null, but selecting the first CandidateSymbols entry can still bind and probe one valid wrapper and publish a passing result even though the postimage does not actually call that method and may not compile. This can occur when two referenced namespaces expose the configured simple type/method name; require a uniquely resolved si.Symbol and classify candidate-only results as a binding refusal.
Useful? React with 👍 / 👎.
Comment on lines
+597
to
+607
| protected override Assembly? Load(AssemblyName name) | ||
| { | ||
| if (!Directory.Exists(_slotsDir)) return null; | ||
| foreach (var slot in Directory.GetDirectories(_slotsDir).OrderBy(d => d, StringComparer.Ordinal)) | ||
| { | ||
| var dll = Directory.GetFiles(slot, "*.dll").SingleOrDefault(); | ||
| if (dll != null && string.Equals(Path.GetFileNameWithoutExtension(dll), name.Name, | ||
| StringComparison.OrdinalIgnoreCase)) | ||
| return LoadFromAssemblyPath(Path.GetFullPath(dll)); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Block fallback to dependencies from the probe deployment
When a wrapper dependency is absent from the materialized reference slots, returning null lets the custom AssemblyLoadContext continue resolution through the default context. Consequently, a wrapper can execute using a private dependency from the probe application's deployment (for example a Roslyn package DLL) even though that DLL is absent from the Step 10 reference closure, producing a pass that is not bound to the recorded target dependencies. Permit only selected shared-framework fallback and refuse unresolved non-framework dependencies.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
…ous decoy Adds the H1 Tier B regressions to the fake-target gate and generalizes the harness to multi-ref-dir cases: a wrapper exporting a same-name overload is refused WRAPPER_BINDING, and two DIFFERENT reference assemblies both exporting global WeakEvents make the source call ambiguous. The ambiguous case is RED against the current bind: it falls back to CandidateSymbols.FirstOrDefault(), promotes a candidate to the bound symbol, and wrongly PASSES. The next commit forbids promoting any candidate to the bound symbol. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…bound symbol (H1) A converted callsite may pass ONLY on a cleanly bound Symbol that is an IMethodSymbol; Symbol == null is ALWAYS a refusal. CandidateSymbols are inspected only to choose the category: exactly one candidate wrapper slot of the wrong frozen shape -> WRAPPER_BINDING; a shape correct by name that still fails to unify with the fixed probe runtime (a net9 / .NET-Framework-only wrapper) -> WRAPPER_RUNTIME_UNSUPPORTED (new bind exit 14, never TARGET_RETAINS); anything else (ambiguous / multiple / otherwise unresolved) -> CALLSITE_BINDING. bind now also enforces: the invocation sits in the accepted source tree/file and inside the selected class; every converted finding id appears exactly once; the emitted callsites are a total bijection onto the converted findings with distinct postimage spans. fix_target maps bind exit 14 -> WRAPPER_RUNTIME_UNSUPPORTED. The genuine weak / strong / no-op / wrong-signature / source-defined / net9 outcomes are unchanged; the ambiguous two-assembly decoy is now CALLSITE_BINDING, not a false pass. Tier B 39/39. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
Adds the H2 Tier B regressions: a wrapper dependency present in an accepted slot passes; a dependency absent from the slots but copied NEXT TO the probe deployment, or ALREADY present in the probe's default context (Microsoft.CodeAnalysis), is refused WRAPPER_RUNTIME_UNSUPPORTED; two slots exporting the same simple-name dependency prove the frozen first-winning slot is the one loaded; and no absolute path leaks into published evidence. The next-to-probe and default-context cases are RED against the current probe: its WrapperLoadContext returns null for a missing dependency, so the .NET default context happily satisfies it from the probe deployment and the wrapper wrongly runs (pass / TARGET_RETAINS). The next commit closes that. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
WrapperLoadContext no longer returns null for an arbitrary missing dependency (which
let the .NET default context satisfy the wrapper from the probe deployment). It now
resolves against an explicit framework set = the simple names physically present in
the SELECTED probe runtime directory:
- framework assemblies resolve ONLY from the selected runtime (delegate to default);
- the wrapper root loads only from its exact derived slot;
- every non-framework dependency resolves ONLY from the ordered materialized slots
(first filename wins) — never the probe deployment, the default context, the cwd,
the user profile, or PATH;
- a non-framework dependency absent from the slots throws a controlled loader
failure -> WRAPPER_RUNTIME_UNSUPPORTED;
- a dependency already loaded in the default context from the probe deployment can
no longer satisfy the wrapper.
fix_target passes the selected runtime dir to each probe attempt via --runtime-dir.
Tier B 52/52; genuine weak / dep-in-slot pass, dep-next-to-probe / dep-in-default /
missing-dependency are WRAPPER_RUNTIME_UNSUPPORTED, the frozen first-winning slot is
proven behaviorally, and no absolute path leaks into evidence.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
Adds Tier A unit regressions for the G5 protocol: EXECUTION_WORK_ROOT is refused ISOLATION when the temp parent resolves inside the source / bundle / output-parent; plan / candidates / delta / pristine source / patch / manifest / postimage drift is refused; probe-deployment / slot / dotnet-host / selected-runtime drift is refused; a cleanup failure is PUBLICATION; and no filesystem operation runs after the publication rename (no execution-root residue). RED against the current fix_target, which has no _execution_root / _reval_inputs / _reval_toolchain / _remove_root and creates the work root with an unchecked mkdtemp plus an ignore_errors cleanup. The next commit implements the literal G5 protocol. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…dation (H3) run_verify_target now follows the ordered G5 protocol on both paths: snapshot all inputs; create EXECUTION_WORK_ROOT under a temp parent PHYSICALLY resolved to be outside and not equal to every protected root (source, bundle, probe deployment, ref-dirs, output parent), with no protected root inside it (else ISOLATION); materialize and execute; revalidate the probe deployment, slots, dotnet host, and selected runtime before bind AND before every probe attempt; revalidate every input and the whole toolchain again before constructing evidence; build the canonical bytes in memory; remove the execution root strictly (a failure is PUBLICATION and the out-dir stays absent); then publish through one atomic rename with no filesystem operation afterwards. Every ignore_errors=True is removed from the Step 11 path: the success path removes the work root strictly, the failure path cleans up best-effort without masking the refusal. Tier A 49/49, Tier B 52/52. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…bind_delta Adds Tier A regressions: a bounded subprocess runner (over-time / stdout-overflow / stderr-overflow are killed and capped); strict binding-result validation (bool ordinal / extra key / negative span / identity mismatch -> INFRASTRUCTURE; a well-formed but non-bijective / duplicate-span binding -> CALLSITE_BINDING); strict probe-result field formats (bool ordinal / malformed sha / mvid / extra key); the runtime-unsupported result schema that gates accepting child exit 10; and bind_delta rejecting an unknown top-level key / a missing or wrong-typed consumed field -> DELTA_BINDING. Extends the fixture delta to the frozen 15-key shape. RED against the current fix_target, which has no _run_child / _validate_binding_result / _validate_unsupported_result / _bind_delta_shapes and buffers child output before checking it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…ind_delta (H4) One shared bounded runner (_run_child) drives both bind and every probe attempt: it enforces the fixed 30s timeout, caps stdout/stderr at 65536 bytes WHILE the child runs, and kills the direct child (no descendant-containment claim) on timeout or overflow. The Python parent now strictly validates the canonical binding-result.json (exact keys/types, non-negative derived ordinal, per-callsite identity == resolved wrapper, a sorted total bijection onto the converted ids with distinct postimage spans; malformed -> INFRASTRUCTURE, non-bijective -> CALLSITE_BINDING), the probe-result resolved-wrapper field formats (int ordinal, lowercase sha256:, 0x token, GUID mvid), and the exact runtime-unsupported schema before accepting child exit 10 (else INFRASTRUCTURE). bind_delta rejects unknown top-level keys and validates the exact frozen shapes it consumes, so a malformed Step 10 delta is DELTA_BINDING, never a KeyError. Tier A 73/73, Tier B 52/52; the frozen Step 10 producer/schema is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
Owner
Author
Corrective round H1–H4 (REQUEST CHANGES on
|
| commit | ||
|---|---|---|
| H1 | a44564f → fb4fcd1 |
bind never promotes CandidateSymbols to the bound symbol. Symbol == null is always a refusal; candidates only pick the category — wrong frozen shape → WRAPPER_BINDING, a shape correct by name that will not unify with the fixed runtime (net9 / .NET-Framework-only) → WRAPPER_RUNTIME_UNSUPPORTED (new bind exit 14, never TARGET_RETAINS), anything else (ambiguous / multiple) → CALLSITE_BINDING. bind also enforces containing-type == selected class, tree/file, unique finding ids, and a sorted total bijection with distinct postimage spans. |
| H2 | 619d3c4 → 90ddc9f |
closed WrapperLoadContext: framework set = the simple names in the SELECTED runtime dir; the wrapper root loads only from its exact slot; non-framework deps resolve ONLY from the ordered slots (first filename wins); a dep absent from the slots throws a controlled loader failure → WRAPPER_RUNTIME_UNSUPPORTED; the probe deployment / default context can never satisfy a wrapper dependency. --runtime-dir threaded from fix_target. |
| H3 | 4961460 → 8c692b8 |
literal G5: EXECUTION_WORK_ROOT created under a temp parent physically outside every protected root (else ISOLATION); revalidate probe deployment / slots / dotnet host / selected runtime before bind AND before every attempt; revalidate every input + toolchain again before evidence; build canonical bytes in memory; strict work-root removal (failure → PUBLICATION, out-dir absent); one atomic rename with no filesystem op afterwards. Every ignore_errors=True removed. |
| H4 | 97db6e3 → 9876b36 |
one shared bounded child runner (30s timeout, stdout/stderr capped at 65536 while running, direct child killed on timeout/overflow, no descendant claim) for bind and every probe attempt; strict canonical binding-result validation (exact keys/types, non-negative ordinal, per-callsite identity == resolved wrapper, sorted bijection with distinct spans — malformed → INFRASTRUCTURE, non-bijective → CALLSITE_BINDING); strict probe-result field formats; the exact runtime-unsupported schema gates accepting child exit 10; bind_delta rejects unknown top-level keys and validates the exact consumed shapes → DELTA_BINDING, never a KeyError. |
New Tier B regressions (full public CLI)
overloaded target → WRAPPER_BINDING; two assemblies both exporting WeakEvents (ambiguous) → CALLSITE_BINDING; dependency in an accepted slot → pass; dependency next to the probe deployment → WRAPPER_RUNTIME_UNSUPPORTED; dependency already in the probe's default context (Microsoft.CodeAnalysis) → WRAPPER_RUNTIME_UNSUPPORTED; two slot versions prove the frozen first-winning slot is loaded; no absolute path in published evidence.
New Tier A regressions
execution-root ISOLATION (TMPDIR inside source / bundle / output parent); plan/candidates/delta/source/patch/manifest/postimage drift; probe-deployment / slot / dotnet-host / selected-runtime drift; cleanup failure → PUBLICATION; no filesystem op after the publication rename + zero residue; bounded runner timeout / stdout-overflow / stderr-overflow; strict binding/probe/unsupported validators; bind_delta unknown-key / missing-field / wrong-typed.
Validation
- Tier A (SDK-free): 73/73. Tier B (
OWN_TIERB_REQUIRED=1, full public CLI): 52/52. tests/run_tests.pygreen, including frozen patch bundle (Step 8) 79/79, gate (Step 9) 69/69, verify-delta (Step 10) 104/104, harness 25/25.ruff check .clean;mypyclean.- CI (push/PR): run
29599763016(in progress at time of posting).
Changed files (unchanged set; new surface only)
A frontend/roslyn/OwnSharp.WeakTargetProbe/OwnSharp.WeakTargetProbe.csproj
A frontend/roslyn/OwnSharp.WeakTargetProbe/Program.cs
A ownlang/fix_target.py
M ownlang/__main__.py (narrow verify-target wiring only)
A tests/test_verify_target.py
A tests/test_verify_target_tierb.py
M .github/workflows/ci.yml
M spec/CLI.md
Steps 0–10 remain source-identical — git diff main...HEAD over fix_candidates.py, fix_plan.py, fix_apply.py, fix_bundle.py, fix_gate.py, fix_delta.py and the Step 8/9/10 tests is empty. Step 12 not started. Not merged — awaiting final arbiter review.
Owner
Author
|
CI run |
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 210-215: Update the CI workflow’s .NET setup and subsequent net8
probe/extractor steps so they explicitly use the 8.0.x SDK rather than resolving
the newest installed SDK. Keep 9.0.x available only for the net9 wrapper fixture
build, preserving the intended Tier B incompatibility check.
In `@frontend/roslyn/OwnSharp.WeakTargetProbe/Program.cs`:
- Line 508: Update the probe type lookup around root.GetTypes().Single so it
matches Bind’s fully qualified type handling: compare typeName against each
candidate’s fully qualified name as well as its simple name, preserving the
existing public, non-generic filtering and ensuring Namespace.Type.Method
wrappers resolve the intended type.
In `@ownlang/__main__.py`:
- Around line 724-726: Update the short-form usage message in cmd_own_fix for
the len(rest) < 2 path to include verify-target alongside the existing
subscription verbs, keeping the documented and dispatched command list
consistent.
In `@ownlang/fix_target.py`:
- Around line 314-315: Update the explicit-delegate matching in the relevant
handler-normalization logic around the `re.fullmatch` calls so the captured
constructor arguments can span newlines. Use a multiline-compatible matching
approach while preserving support for both `new Type(...)` and target-typed
`new(...)` forms, ensuring multiline delegate handlers are unpeeled consistently
with the Roslyn normalizer.
- Around line 473-502: Strengthen validation in the result-checking flow around
callsite binding so it rejects semantic violations before accepting evidence:
require all callsites to share the resolved wrapper symbol when
all_callsites_same_symbol is false, reject target_is_source_defined being true,
and reject partially overlapping postimage spans rather than only identical
spans. Use the existing binding fields and span validation symbols near the
calls to _BINDING_CB_KEYS and spans.
In `@tests/test_verify_target_tierb.py`:
- Around line 437-443: The _no_absolute_paths helper currently relies on an
incomplete directory denylist. Replace its leaked-path detection with structural
checks that recognize Unix absolute paths, including /opt, /mnt, and /Users, and
Windows absolute paths using either backslashes or forward slashes, while
preserving the existing assertion and diagnostic message.
🪄 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: 6cdb4cc4-987a-48f1-b13f-ce332f2219f1
📒 Files selected for processing (8)
.github/workflows/ci.ymlfrontend/roslyn/OwnSharp.WeakTargetProbe/OwnSharp.WeakTargetProbe.csprojfrontend/roslyn/OwnSharp.WeakTargetProbe/Program.csownlang/__main__.pyownlang/fix_target.pyspec/CLI.mdtests/test_verify_target.pytests/test_verify_target_tierb.py
Comment on lines
+210
to
+215
| # 8.0.x runs the pinned net8.0 extractor/probe; 9.0.x is only needed to BUILD the | ||
| # deliberately-incompatible net9 wrapper fixture in the step 11 Tier B suite (which | ||
| # then proves it is refused WRAPPER_RUNTIME_UNSUPPORTED under the fixed net8 probe). | ||
| dotnet-version: | | ||
| 8.0.x | ||
| 9.0.x |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
fd -a '^global\.json$' . -x sh -c 'echo "=== $1"; cat "$1"' sh {}
rg -n -C3 'rollForward|sdk.version|TargetFramework' \
-g 'global.json' -g '*.csproj' frontend/roslynRepository: PhysShell/Own.NET
Length of output: 4882
🏁 Script executed:
#!/bin/bash
set -eu
# Inspect the relevant workflow section with line numbers.
sed -n '180,250p' .github/workflows/ci.yml | cat -n
# Look for any SDK pinning or dotnet configuration files.
printf '\n=== SDK pins ===\n'
fd -a '^(global\.json|Directory\.Build\.props|Directory\.Build\.targets)$' . -x sh -c 'echo "=== $1"; cat "$1"' sh {}
printf '\n=== dotnet install/build usage ===\n'
rg -n -C2 'setup-dotnet|dotnet-version|dotnet build|dotnet test|dotnet publish|dotnet run' .github/workflows/ci.yml frontend/roslyn -g '*.yml' -g '*.yaml' -g '*.csproj' -g 'global.json' -g 'Directory.Build.props' -g 'Directory.Build.targets'Repository: PhysShell/Own.NET
Length of output: 21681
🏁 Script executed:
#!/bin/bash
set -eu
# Show the full workflow area around the SDK installation and subsequent build steps.
sed -n '180,320p' .github/workflows/ci.yml | cat -n
# Search for repo-level SDK pinning/configuration files and any build commands.
printf '\n=== SDK/config files ===\n'
fd -a '^(global\.json|Directory\.Build\.props|Directory\.Build\.targets)$' .
printf '\n=== workflow/build references ===\n'
rg -n -C2 'setup-dotnet|dotnet-version|dotnet build|dotnet test|dotnet publish|dotnet run|UseDotNet' .github/workflows/ci.yml frontend/roslyn -g '*.yml' -g '*.yaml' -g '*.csproj' -g 'global.json' -g 'Directory.Build.props' -g 'Directory.Build.targets'Repository: PhysShell/Own.NET
Length of output: 26917
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '180,320p' .github/workflows/ci.yml | cat -n
printf '\n=== SDK/config files ===\n'
fd -a '^(global\.json|Directory\.Build\.props|Directory\.Build\.targets)$' .
printf '\n=== workflow/build references ===\n'
rg -n -C2 'setup-dotnet|dotnet-version|dotnet build|dotnet test|dotnet publish|dotnet run|UseDotNet' .github/workflows/ci.yml frontend/roslyn -g '*.yml' -g '*.yaml' -g '*.csproj' -g 'global.json' -g 'Directory.Build.props' -g 'Directory.Build.targets'Repository: PhysShell/Own.NET
Length of output: 26917
🌐 Web query:
GitHub actions/setup-dotnet multiple dotnet-version list SDK selection latest installed global.json dotnet SDK resolver
💡 Result:
When you use the actions/setup-dotnet GitHub Action to install multiple.NET SDK versions, the action installs all specified versions side-by-side on the runner [1][2]. However, the.NET SDK selection—which version is actually used when you run the dotnet command—is governed by the.NET SDK resolver's standard logic, not by the order in which the action installed them [3][4]. Here is how the selection works and how you can manage it: 1. How the.NET SDK Resolver Selects a Version By default, the dotnet CLI automatically selects the latest installed.NET SDK version [2][4]. This behavior is a fundamental part of the.NET design and occurs regardless of which versions were installed by the setup-dotnet action [3]. 2. The Role of global.json The presence of a global.json file in your project or repository is the primary mechanism to override this "latest version" default [4][5]. The.NET CLI iteratively searches for a global.json file by traversing upward from the current working directory [4]. If a file is found, it instructs the resolver to use the specific version defined in that file, subject to your defined roll-forward policy [6][4]. 3. Managing Multiple Versions with setup-dotnet Because the setup-dotnet action installs multiple versions without automatically creating a global.json file, the system default (the latest installed version) will be used by the dotnet CLI unless you explicitly take control [3]. To ensure a specific version from your installed set is used, you have two primary options: - Use a global.json file: Explicitly include a global.json file in your repository [4]. When the dotnet CLI runs, it will detect this file and bind to the required version, provided that version was installed by the action [4]. - Matrix Strategy (Recommended for CI): Instead of installing multiple versions in one step, use a matrix strategy in your GitHub Actions workflow [7][2]. This allows you to run your build/test jobs for each version independently, ensuring that the environment is explicitly configured for the specific version you intend to test [8]. Example of using a matrix strategy: jobs: build: runs-on: ubuntu-latest strategy: matrix: dotnet-version: [ '8.0.x', '9.0.x' ] steps: - uses: actions/checkout@v6 - name: Setup dotnet uses: actions/setup-dotnet@v5 with: dotnet-version: ${{ matrix.dotnet-version }} - run: dotnet build Summary of Behavior The action installs all requested versions [2]. If no global.json is present, the highest version number installed on the runner will be used by the dotnet CLI [2][4]. If you need to use a non-latest version that you installed, you must provide a global.json file to direct the resolver to the specific version [3][4].
Citations:
- 1: https://github.com/actions/setup-dotnet/blob/main/README.md
- 2: https://github.com/actions/setup-dotnet?tab=readme-ov-file
- 3: https://github.com/actions/setup-dotnet/blob/main/docs/adrs/v3-setup-dotnet.md
- 4: https://learn.microsoft.com/en-us/dotnet/core/versions/selection
- 5: .NET Core 2+ Version Binding dotnet/runtime#43525
- 6: https://learn.microsoft.com/en-us/dotnet/core/tools/global-json
- 7: https://docs.github.com/en/actions/tutorials/build-and-test-code/net
- 8: https://docs.github.com/actions/guides/building-and-testing-net
Pin the net8 build steps to 8.0.x
Installing 9.0.x alongside 8.0.x lets dotnet resolve to the newest SDK on this job, so the later dotnet run steps can use the 9.0 toolchain instead of the intended net8 one. Keep 9.0.x scoped to the net9 fixture build, or pin the net8 steps explicitly.
🤖 Prompt for 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.
In @.github/workflows/ci.yml around lines 210 - 215, Update the CI workflow’s
.NET setup and subsequent net8 probe/extractor steps so they explicitly use the
8.0.x SDK rather than resolving the newest installed SDK. Keep 9.0.x available
only for the net9 wrapper fixture build, preserving the intended Tier B
incompatibility check.
| { | ||
| // G4 preflight: load-by-path, resolve type + exact method, build delegate, prepare. | ||
| var root = alc.LoadFromAssemblyPath(rootPath); | ||
| var type = root.GetTypes().Single(t => t.IsPublic && !t.IsGenericType && t.Name == typeName); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Match probe lookup with bind’s fully qualified type handling.
Bind accepts Namespace.Type.Method, but probe mode only compares Type.Name. A fully qualified wrapper can bind successfully and then fail during preflight because Single() never finds the type.
🤖 Prompt for 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.
In `@frontend/roslyn/OwnSharp.WeakTargetProbe/Program.cs` at line 508, Update the
probe type lookup around root.GetTypes().Single so it matches Bind’s fully
qualified type handling: compare typeName against each candidate’s fully
qualified name as well as its simple name, preserving the existing public,
non-generic filtering and ensuring Namespace.Type.Method wrappers resolve the
intended type.
Comment on lines
724
to
+726
| def cmd_own_fix(rest: list[str]) -> int: | ||
| """`own-fix subscriptions {candidates|render|validate-plan|apply|gate|verify-delta} ...`.""" | ||
| """`own-fix subscriptions {candidates|render|validate-plan|apply|gate|verify-delta| | ||
| verify-target} ...`.""" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add verify-target to the short-form usage message.
The new verb is documented and dispatched here, but the len(rest) < 2 path on Lines 728-729 still lists only through verify-delta, giving users stale CLI guidance.
🤖 Prompt for 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.
In `@ownlang/__main__.py` around lines 724 - 726, Update the short-form usage
message in cmd_own_fix for the len(rest) < 2 path to include verify-target
alongside the existing subscription verbs, keeping the documented and dispatched
command list consistent.
Comment on lines
+314
to
+315
| m = re.fullmatch(r"new\s+[^\s(]+\s*\(\s*(.*)\s*\)", s) or re.fullmatch( | ||
| r"new\s*\(\s*(.*)\s*\)", s) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle multiline explicit-delegate handlers.
Regex . does not cross newlines, so a valid multiline new PropertyChangedEventHandler(...) remains unpeeled and disagrees with the Roslyn normalizer, causing CALLSITE_BINDING.
Proposed fix
- m = re.fullmatch(r"new\s+[^\s(]+\s*\(\s*(.*)\s*\)", s) or re.fullmatch(
- r"new\s*\(\s*(.*)\s*\)", s)
+ m = re.fullmatch(
+ r"new\s+[^\s(]+\s*\(\s*(.*)\s*\)", s, flags=re.DOTALL
+ ) or re.fullmatch(r"new\s*\(\s*(.*)\s*\)", s, flags=re.DOTALL)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| m = re.fullmatch(r"new\s+[^\s(]+\s*\(\s*(.*)\s*\)", s) or re.fullmatch( | |
| r"new\s*\(\s*(.*)\s*\)", s) | |
| m = re.fullmatch( | |
| r"new\s+[^\s(]+\s*\(\s*(.*)\s*\)", s, flags=re.DOTALL | |
| ) or re.fullmatch(r"new\s*\(\s*(.*)\s*\)", s, flags=re.DOTALL) |
🤖 Prompt for 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.
In `@ownlang/fix_target.py` around lines 314 - 315, Update the explicit-delegate
matching in the relevant handler-normalization logic around the `re.fullmatch`
calls so the captured constructor arguments can span newlines. Use a
multiline-compatible matching approach while preserving support for both `new
Type(...)` and target-typed `new(...)` forms, ensuring multiline delegate
handlers are unpeeled consistently with the Roslyn normalizer.
Comment on lines
+473
to
+502
| cb = obj["callsite_binding"] | ||
| if not isinstance(cb, dict) or set(cb) != set(_BINDING_CB_KEYS) \ | ||
| or not all(isinstance(cb[k], bool) for k in _BINDING_CB_KEYS): | ||
| raise TargetError(inf, "callsite_binding is not the exact schema") | ||
| cs = obj["callsites"] | ||
| if not isinstance(cs, list): | ||
| raise TargetError(inf, "callsites must be a list") | ||
| fids, spans = [], [] | ||
| for c in cs: | ||
| if not isinstance(c, dict) or set(c) != set(_BINDING_CS_KEYS): | ||
| raise TargetError(inf, "a callsite is not the exact schema") | ||
| if not isinstance(c["finding_id"], str): | ||
| raise TargetError(inf, "callsite finding_id must be a string") | ||
| for sk in ("preimage_span", "postimage_span"): | ||
| sp = c[sk] | ||
| if not isinstance(sp, list) or len(sp) != 2 \ | ||
| or not all(_is_int(v) and v >= 0 for v in sp): | ||
| raise TargetError(inf, f"callsite {sk} must be two non-negative ints") | ||
| if any(c[k] != rw[k] for k in _BINDING_RW_KEYS): | ||
| raise TargetError(inf, "a callsite identity does not equal resolved_wrapper") | ||
| fids.append(c["finding_id"]) | ||
| spans.append((c["postimage_span"][0], c["postimage_span"][1])) | ||
| if fids != sorted(fids): | ||
| raise TargetError(inf, "callsites are not sorted by finding_id") | ||
| want = set(convert_ids) | ||
| if obj["converted_callsites"] != len(want) or len(cs) != len(want) \ | ||
| or set(fids) != want or len(set(fids)) != len(fids): | ||
| raise TargetError(CALLSITE_BINDING, "callsites are not a bijection onto the converted ids") | ||
| if len(set(spans)) != len(spans): | ||
| raise TargetError(CALLSITE_BINDING, "two callsites share a postimage span") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce semantic binding invariants before accepting the result.
A canonical result with all_callsites_same_symbol=false, target_is_source_defined=true, or partially overlapping postimage spans currently passes and can produce pass evidence.
Proposed fix
if not isinstance(cb, dict) or set(cb) != set(_BINDING_CB_KEYS) \
or not all(isinstance(cb[k], bool) for k in _BINDING_CB_KEYS):
raise TargetError(inf, "callsite_binding is not the exact schema")
+ if not cb["all_callsites_same_symbol"] or cb["target_is_source_defined"]:
+ raise TargetError(CALLSITE_BINDING, "callsite binding invariants failed")
@@
- if not isinstance(sp, list) or len(sp) != 2 \
- or not all(_is_int(v) and v >= 0 for v in sp):
+ if not isinstance(sp, list) or len(sp) != 2 \
+ or not all(_is_int(v) and v >= 0 for v in sp) or sp[1] == 0:
raise TargetError(inf, f"callsite {sk} must be two non-negative ints")
@@
- spans.append((c["postimage_span"][0], c["postimage_span"][1]))
+ start, length = c["postimage_span"]
+ spans.append((start, start + length))
@@
- if len(set(spans)) != len(spans):
- raise TargetError(CALLSITE_BINDING, "two callsites share a postimage span")
+ ordered = sorted(spans)
+ if any(start < previous_end
+ for (_, previous_end), (start, _) in zip(ordered, ordered[1:])):
+ raise TargetError(CALLSITE_BINDING, "postimage spans overlap")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cb = obj["callsite_binding"] | |
| if not isinstance(cb, dict) or set(cb) != set(_BINDING_CB_KEYS) \ | |
| or not all(isinstance(cb[k], bool) for k in _BINDING_CB_KEYS): | |
| raise TargetError(inf, "callsite_binding is not the exact schema") | |
| cs = obj["callsites"] | |
| if not isinstance(cs, list): | |
| raise TargetError(inf, "callsites must be a list") | |
| fids, spans = [], [] | |
| for c in cs: | |
| if not isinstance(c, dict) or set(c) != set(_BINDING_CS_KEYS): | |
| raise TargetError(inf, "a callsite is not the exact schema") | |
| if not isinstance(c["finding_id"], str): | |
| raise TargetError(inf, "callsite finding_id must be a string") | |
| for sk in ("preimage_span", "postimage_span"): | |
| sp = c[sk] | |
| if not isinstance(sp, list) or len(sp) != 2 \ | |
| or not all(_is_int(v) and v >= 0 for v in sp): | |
| raise TargetError(inf, f"callsite {sk} must be two non-negative ints") | |
| if any(c[k] != rw[k] for k in _BINDING_RW_KEYS): | |
| raise TargetError(inf, "a callsite identity does not equal resolved_wrapper") | |
| fids.append(c["finding_id"]) | |
| spans.append((c["postimage_span"][0], c["postimage_span"][1])) | |
| if fids != sorted(fids): | |
| raise TargetError(inf, "callsites are not sorted by finding_id") | |
| want = set(convert_ids) | |
| if obj["converted_callsites"] != len(want) or len(cs) != len(want) \ | |
| or set(fids) != want or len(set(fids)) != len(fids): | |
| raise TargetError(CALLSITE_BINDING, "callsites are not a bijection onto the converted ids") | |
| if len(set(spans)) != len(spans): | |
| raise TargetError(CALLSITE_BINDING, "two callsites share a postimage span") | |
| cb = obj["callsite_binding"] | |
| if not isinstance(cb, dict) or set(cb) != set(_BINDING_CB_KEYS) \ | |
| or not all(isinstance(cb[k], bool) for k in _BINDING_CB_KEYS): | |
| raise TargetError(inf, "callsite_binding is not the exact schema") | |
| if not cb["all_callsites_same_symbol"] or cb["target_is_source_defined"]: | |
| raise TargetError(CALLSITE_BINDING, "callsite binding invariants failed") | |
| cs = obj["callsites"] | |
| if not isinstance(cs, list): | |
| raise TargetError(inf, "callsites must be a list") | |
| fids, spans = [], [] | |
| for c in cs: | |
| if not isinstance(c, dict) or set(c) != set(_BINDING_CS_KEYS): | |
| raise TargetError(inf, "a callsite is not the exact schema") | |
| if not isinstance(c["finding_id"], str): | |
| raise TargetError(inf, "callsite finding_id must be a string") | |
| for sk in ("preimage_span", "postimage_span"): | |
| sp = c[sk] | |
| if not isinstance(sp, list) or len(sp) != 2 \ | |
| or not all(_is_int(v) and v >= 0 for v in sp) or sp[1] == 0: | |
| raise TargetError(inf, f"callsite {sk} must be two non-negative ints") | |
| if any(c[k] != rw[k] for k in _BINDING_RW_KEYS): | |
| raise TargetError(inf, "a callsite identity does not equal resolved_wrapper") | |
| fids.append(c["finding_id"]) | |
| start, length = c["postimage_span"] | |
| spans.append((start, start + length)) | |
| if fids != sorted(fids): | |
| raise TargetError(inf, "callsites are not sorted by finding_id") | |
| want = set(convert_ids) | |
| if obj["converted_callsites"] != len(want) or len(cs) != len(want) \ | |
| or set(fids) != want or len(set(fids)) != len(fids): | |
| raise TargetError(CALLSITE_BINDING, "callsites are not a bijection onto the converted ids") | |
| ordered = sorted(spans) | |
| if any(start < previous_end | |
| for (_, previous_end), (start, _) in zip(ordered, ordered[1:])): | |
| raise TargetError(CALLSITE_BINDING, "postimage spans overlap") |
🤖 Prompt for 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.
In `@ownlang/fix_target.py` around lines 473 - 502, Strengthen validation in the
result-checking flow around callsite binding so it rejects semantic violations
before accepting evidence: require all callsites to share the resolved wrapper
symbol when all_callsites_same_symbol is false, reject target_is_source_defined
being true, and reject partially overlapping postimage spans rather than only
identical spans. Use the existing binding fields and span validation symbols
near the calls to _BINDING_CB_KEYS and spans.
Comment on lines
+437
to
+443
| def _no_absolute_paths(raw: bytes, check, name: str) -> None: | ||
| """No slot / deployment / execution absolute path may leak into published evidence (H2).""" | ||
| import re | ||
| text = raw.decode("utf-8") | ||
| leaked = bool(re.search(r"[A-Za-z]:\\", text)) or any( | ||
| s in text for s in ("/tmp/", "/home/", "/var/", "/root/", "\\\\")) | ||
| check(not leaked, f"{name}: evidence publishes no absolute path") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Detect absolute paths structurally instead of using a directory denylist.
This misses paths such as /opt/..., /mnt/..., /Users/..., and Windows paths using /, allowing evidence-path leaks to pass the acceptance test.
Proposed fix
def _no_absolute_paths(raw: bytes, check, name: str) -> None:
"""No slot / deployment / execution absolute path may leak into published evidence (H2)."""
- import re
- text = raw.decode("utf-8")
- leaked = bool(re.search(r"[A-Za-z]:\\", text)) or any(
- s in text for s in ("/tmp/", "/home/", "/var/", "/root/", "\\\\"))
+ import ntpath
+ import posixpath
+
+ def strings(value):
+ if isinstance(value, str):
+ yield value
+ elif isinstance(value, dict):
+ for item in value.values():
+ yield from strings(item)
+ elif isinstance(value, list):
+ for item in value:
+ yield from strings(item)
+
+ leaked = any(
+ posixpath.isabs(value) or ntpath.isabs(value)
+ for value in strings(json.loads(raw))
+ )
check(not leaked, f"{name}: evidence publishes no absolute path")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _no_absolute_paths(raw: bytes, check, name: str) -> None: | |
| """No slot / deployment / execution absolute path may leak into published evidence (H2).""" | |
| import re | |
| text = raw.decode("utf-8") | |
| leaked = bool(re.search(r"[A-Za-z]:\\", text)) or any( | |
| s in text for s in ("/tmp/", "/home/", "/var/", "/root/", "\\\\")) | |
| check(not leaked, f"{name}: evidence publishes no absolute path") | |
| def _no_absolute_paths(raw: bytes, check, name: str) -> None: | |
| """No slot / deployment / execution absolute path may leak into published evidence (H2).""" | |
| import ntpath | |
| import posixpath | |
| def strings(value): | |
| if isinstance(value, str): | |
| yield value | |
| elif isinstance(value, dict): | |
| for item in value.values(): | |
| yield from strings(item) | |
| elif isinstance(value, list): | |
| for item in value: | |
| yield from strings(item) | |
| leaked = any( | |
| posixpath.isabs(value) or ntpath.isabs(value) | |
| for value in strings(json.loads(raw)) | |
| ) | |
| check(not leaked, f"{name}: evidence publishes no absolute path") |
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 441-441: Do not hardcode temporary file or directory names
Context: "/tmp/"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
🤖 Prompt for 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.
In `@tests/test_verify_target_tierb.py` around lines 437 - 443, The
_no_absolute_paths helper currently relies on an incomplete directory denylist.
Replace its leaked-path detection with structural checks that recognize Unix
absolute paths, including /opt, /mnt, and /Users, and Windows absolute paths
using either backslashes or forward slashes, while preserving the existing
assertion and diagnostic message.
K1: source / change.patch / apply-manifest.json / accepted postimage drift AFTER the initial binding must be ISOLATION (a startup mismatch stays DELTA_BINDING via bind_bundle). K2: every JSON integer field is validated with the exact-int helper (a bool is not an int), version==1 and attempt==the expected non-negative int; the consumed Step 10 nested objects (analysis_scope, input_hashes, target_api, expected, resolved_runtime_identity, each reference_closure entry) are closed key sets -> DELTA_BINDING. RED against the current fix_target, which classifies source/patch/manifest/postimage drift as DELTA_BINDING, accepts version=true / attempt=false via `!= 1` / `!= attempt` (bool == int in Python), and only presence-checks the nested delta shapes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…target) K1: _reval_inputs classifies post-binding drift of the pristine source / change.patch / apply-manifest.json / accepted postimage as ISOLATION; plan/candidates stay AUTHORITY_BINDING, the delta stays DELTA_BINDING, and the initial hash mismatch in bind_bundle stays DELTA_BINDING. K2: the exact-int helper (isinstance int and not bool) now guards binding-result, probe-result, and runtime-unsupported version + attempt (version must be integer 1, attempt the exact expected non-negative integer — a boolean is rejected). bind_delta validates the EXACT frozen key sets of every consumed Step 10 nested object (analysis_scope, input_hashes, target_api, expected, resolved_runtime_identity, and each reference_closure entry) -> DELTA_BINDING, never a KeyError. The frozen Step 10 producer/schema is unchanged. Tier A 91/91. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…e predicate Adds bind-unit regressions: an exact-shape net8 wrapper whose callsite cannot bind for a NON-runtime reason — a receiver that cannot convert to INotifyPropertyChanged, or an ambiguous / non-convertible handler method group — must be CALLSITE_BINDING, not WRAPPER_RUNTIME_UNSUPPORTED. RED against the current bind, which returns WRAPPER_RUNTIME_UNSUPPORTED for ANY shape-correct single-candidate slot whose callsite fails to bind, inferring incompatibility instead of positively establishing it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…(K3) WRAPPER_RUNTIME_UNSUPPORTED is now returned ONLY when a deterministic positive predicate holds: the wrapper assembly references a core framework assembly (System.Runtime / System.Private.CoreLib / netstandard / mscorlib) at a version STRICTLY NEWER than the one the selected probe runtime provides to the bind compilation (via System.Object's containing assembly). A shape-correct single-candidate wrapper whose callsite merely fails to bind for another reason (argument-conversion failure, method-group ambiguity, source-compilation failure) is CALLSITE_BINDING; a wrong frozen shape stays WRAPPER_BINDING; ambiguous / multiple candidate assemblies stay CALLSITE_BINDING. A clean IMethodSymbol is still required for success and CandidateSymbols remain diagnostic-only. A net9 wrapper still refuses WRAPPER_RUNTIME_UNSUPPORTED, now positively established. Tier B 54/54. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
Owner
Author
|
Corrective round K1–K3 applied (HEAD |
Adds Tier B regressions: a .NET Framework 4.x wrapper (references mscorlib) under the net8 probe is refused WRAPPER_RUNTIME_UNSUPPORTED (cross-family), while a compatible netstandard2.0 wrapper is NOT rejected by the runtime-family predicate and continues to binding/probe (passes). Also decodes child output as UTF-8 (a localized MSBuild/NuGet line must not crash the reader thread) and sets LangVersion=latest so net48/netstandard2.0 compile the nullable wrapper sources; net4x pulls in the cross-platform reference assemblies. RED against the current bind: a net48 wrapper binds cleanly (its types retarget onto the net8 refs) and reaches the probe, so the version-only predicate never sees the mscorlib cross-family reference. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
RuntimeIncompatible now positively establishes an incompatible runtime FAMILY as well as a newer same-family contract: it reads the selected probe runtime's core family from the compilation (System.Object's containing assembly — System.Private.CoreLib for CoreCLR) and refuses WRAPPER_RUNTIME_UNSUPPORTED when the wrapper references mscorlib under a CoreCLR probe (or System.Private.CoreLib under a non-CoreCLR probe), a System.Runtime / System.Private.CoreLib contract at a version strictly newer than the runtime supplies, or carries a TargetFrameworkAttribute that positively declares an incompatible family. A netstandard reference is never a reason to reject. The check runs on BOTH the Symbol==null path and the cleanly-bound path (a net48 wrapper retargets onto the net8 refs and binds), so it is refused before probing. Argument-conversion failures, ambiguous handler groups, source-defined targets and multiple assemblies remain CALLSITE_BINDING; a wrong frozen shape remains WRAPPER_BINDING. Tier B 65/65. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…leanup L2 Tier A regressions: delta schema=true, reference_dir_count=false, non-string selected_class / closure_kind, boolean / negative reference ordinal, non-string expected id, malformed input / reference SHA, and a duplicate / unsorted reference ordinal are all DELTA_BINDING. L3 Tier A regressions: an ordinary controlled failure after the execution root is claimed re-raises its original category on successful cleanup, becomes PUBLICATION on a cleanup failure (and likewise for an isolation-after-creation failure), and leaves OUTPUT_DIR absent in every case. RED against the current fix_target, which presence-checks (not SHA / ordinal-order / exact-int) the consumed Step 10 shapes and cleans up the work root best-effort (except OSError: pass) so a cleanup failure is silently swallowed rather than PUBLICATION. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
…cleanup (L3) L2: _bind_delta_shapes validates the exact consumed Step 10 schema — an exact-int helper for delta.schema(==1), analysis_scope.reference_dir_count(>=0) and every reference_closure ordinal (>=0); string types for the analysis_scope fields; a list-of-strings for the expected id lists; the `sha256:<64 hex>` form for every consumed digest (input_hashes, reference_closure sha256, runtime_manifest_sha256); and reference_closure entries in exact ordinal order with no duplicate. Any violation is DELTA_BINDING. L3: the single _remove_root_strict is the ONLY execution-root removal, used by the success path, the isolation-after-creation path, and the failure path. On any pre-publication failure the work root is removed without swallowing OSError; a successful cleanup re-raises the original controlled refusal, a cleanup failure is PUBLICATION (chained from the cleanup error), and OUTPUT_DIR stays absent. Every ignore_errors and `except OSError: pass` is gone. Tier A 108/108. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/test_verify_target.py`:
- Around line 758-766: Update the valid two-entry closure setup in the delta
binding test to set reference_dir_count to 2. Add coverage that rejects a
reference_dir_count/ordinal-count mismatch, and separate the unsorted-ordinal
case from the duplicate case by using distinct ordinals in descending order;
retain the valid ordered closure assertion.
🪄 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: 222d9928-eac4-4524-b50a-038dd25137dc
📒 Files selected for processing (4)
frontend/roslyn/OwnSharp.WeakTargetProbe/Program.csownlang/fix_target.pytests/test_verify_target.pytests/test_verify_target_tierb.py
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/roslyn/OwnSharp.WeakTargetProbe/Program.cs
- ownlang/fix_target.py
Comment on lines
+758
to
+766
| run_bad(lambda d: d.update(reference_closure=[dict(rc0), {**rc0, "ordinal": 0}]), | ||
| "L2: duplicate / unsorted reference ordinal -> DELTA_BINDING") | ||
| # positive control: a valid, ordered two-entry closure binds. | ||
| d = json.loads(delta_bytes) | ||
| d["reference_closure"] = [rc0, {"ordinal": 1, "source_dir_ordinal": 1, | ||
| "relative_path": "B.dll", "sha256": "sha256:" + "b" * 64}] | ||
| try: | ||
| ft.bind_delta(_canonical_bytes(d), auth, plan_b, cand_b) | ||
| check(True, "L2: a valid ordered two-entry closure binds") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the closure vectors consistent with reference_dir_count.
The “valid” two-source closure leaves the count at 0, blessing inconsistent Step-10 evidence. Set it to 2, ensure a count/ordinal mismatch is rejected, and test distinct unsorted ordinals separately—the current [0, 0] case only covers duplicates.
Proposed correction
- run_bad(lambda d: d.update(reference_closure=[dict(rc0), {**rc0, "ordinal": 0}]),
- "L2: duplicate / unsorted reference ordinal -> DELTA_BINDING")
+ run_bad(lambda d: d.update(reference_closure=[dict(rc0), {**rc0, "ordinal": 0}]),
+ "L2: duplicate reference ordinal -> DELTA_BINDING")
# positive control: a valid, ordered two-entry closure binds.
d = json.loads(delta_bytes)
+ d["analysis_scope"]["reference_dir_count"] = 2
d["reference_closure"] = [rc0, {"ordinal": 1, "source_dir_ordinal": 1,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| run_bad(lambda d: d.update(reference_closure=[dict(rc0), {**rc0, "ordinal": 0}]), | |
| "L2: duplicate / unsorted reference ordinal -> DELTA_BINDING") | |
| # positive control: a valid, ordered two-entry closure binds. | |
| d = json.loads(delta_bytes) | |
| d["reference_closure"] = [rc0, {"ordinal": 1, "source_dir_ordinal": 1, | |
| "relative_path": "B.dll", "sha256": "sha256:" + "b" * 64}] | |
| try: | |
| ft.bind_delta(_canonical_bytes(d), auth, plan_b, cand_b) | |
| check(True, "L2: a valid ordered two-entry closure binds") | |
| run_bad(lambda d: d.update(reference_closure=[dict(rc0), {**rc0, "ordinal": 0}]), | |
| "L2: duplicate reference ordinal -> DELTA_BINDING") | |
| # positive control: a valid, ordered two-entry closure binds. | |
| d = json.loads(delta_bytes) | |
| d["analysis_scope"]["reference_dir_count"] = 2 | |
| d["reference_closure"] = [rc0, {"ordinal": 1, "source_dir_ordinal": 1, | |
| "relative_path": "B.dll", "sha256": "sha256:" + "b" * 64}] | |
| try: | |
| ft.bind_delta(_canonical_bytes(d), auth, plan_b, cand_b) | |
| check(True, "L2: a valid ordered two-entry closure binds") |
🤖 Prompt for 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.
In `@tests/test_verify_target.py` around lines 758 - 766, Update the valid
two-entry closure setup in the delta binding test to set reference_dir_count to
2. Add coverage that rejects a reference_dir_count/ordinal-count mismatch, and
separate the unsorted-ordinal case from the duplicate case by using distinct
ordinals in descending order; retain the valid ordered closure assertion.
Owner
Author
|
Corrective round L1–L3 applied (HEAD |
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.
S2 Step 11 — the Verified Target Wrapper gate (
own-fix subscriptions verify-target)Step 10 proves the analyzer stops reporting OWN001 for a converted subscription, but it recognizes the replacement wrapper by name only. Step 11 proves the wrapper the accepted Step 8 postimage actually calls is a genuine non-retaining subscription — defeating a name-only-recognized decoy. A fixed Roslyn
bind(SemanticModel over the pristine preimage + accepted postimage; per-finding callsite bijection; theplan.target_api.subscribeshape resolved inside the selected reference-slot wrapper) is followed by a fixed runtimeprobe(three fresh isolated children that load the derived wrapper from its exact materialized slot via a closedAssemblyLoadContext, run a runtime-compatibility preflight, then a frozen GC harness). A wrapper that retains the subscriber is a fake target → refusedTARGET_RETAINS.Implemented from the accepted rev F contract
S2_STEP11_LOCKED_CONTRACTSHA-25691137d98c11982c0da870137d6314ef3a3efdee4ee771d06deeaa69e59f495e8, with locked amendments G1–G5, then the arbiter corrective rounds H1–H4, K1–K3, and L1–L3. Frozen Steps 0–10 are untouched; Step 12 not started.Final HEAD:
c7ab5e4Commit sequence (red → green)
What the corrective rounds established
IMethodSymbol;Symbol == nullis always a refusal andCandidateSymbolsare diagnostic-only.WRAPPER_RUNTIME_UNSUPPORTEDis returned ONLY on a positive predicate: the wrapper referencesmscorlibunder a CoreCLR probe (orSystem.Private.CoreLibunder a non-CoreCLR probe), aSystem.Runtime/System.Private.CoreLibcontract strictly newer than the selected runtime supplies, or aTargetFrameworkAttributethat declares an incompatible family (anetstandardreference is never a reason to reject) — checked on both theSymbol == nulland the cleanly-bound path (a net48 wrapper retargets onto the net8 refs and binds). Wrong frozen shape →WRAPPER_BINDING; argument-conversion failure / method-group ambiguity / multiple assemblies / source-defined →CALLSITE_BINDING. Plus containing-type / tree / unique-fid / sorted-bijection / distinct-span checks.WrapperLoadContext: framework set = the simple names in the SELECTED runtime dir; non-framework deps resolve ONLY from the ordered slots (first-filename-wins); a missing dep throws →WRAPPER_RUNTIME_UNSUPPORTED; the probe deployment / default context can never satisfy a wrapper dependency.ISOLATION); revalidate deployment/slots/host/runtime before bind AND before every attempt; revalidate all inputs+toolchain before evidence; one atomic rename, no fs op afterwards. Drift categories: plan/candidates →AUTHORITY_BINDING, delta →DELTA_BINDING, source/patch/manifest/postimage drift after binding →ISOLATION(a startup mismatch staysDELTA_BINDING). Failure-path cleanup is strict: the single_remove_root_strictremoves the work root without swallowingOSError— a successful cleanup re-raises the original refusal, a cleanup failure isPUBLICATION(chained), and the out-dir stays absent. Noignore_errors/except: passremain.sha256:<64 hex>for every consumed digest,reference_dir_countand every reference-closure ordinal as a non-negative exact int, reference-closure in exact ordinal order — all →DELTA_BINDING, never a KeyError.Validation
OWN_TIERB_REQUIRED=1, full public CLI): 65/65.tests/run_tests.pygreen, including frozen patch bundle (Step 8) 79/79, gate (Step 9) 69/69, verify-delta (Step 10) 104/104, harness 25/25.ruff check .clean;mypyclean.29615221836— success (incl. the wpf-extractor Step 11 Tier B,lint (ruff + mypy --strict), andtests (py3.11/3.12/3.13)).Evidence (real public CLI, deterministic)
target-result.jsonsha256:f4defa5a303ff85f39582e11e3c1db995798d5225dcd4f17fdca1e3a6c205525; selected wrapperWeakEventsmvid2eee946a-962f-473b-91b1-8da9df39f69a, token0x06000001, sigSystem.Void WeakEvents.AddPropertyChanged(System.ComponentModel.INotifyPropertyChanged, System.ComponentModel.PropertyChangedEventHandler)own-fix: refuse: TARGET_RETAINS: the wrapper retained the subscriber (fake target)sha256:c11c95d7a9d7ab8957c30bae0475687aab0c24ba03941a0717f9d3a2b8c828cd, six checksnot_applicablesha256:f4defa5a…205525==sha256:f4defa5a…205525)Tier B also covers: no-op / twice / throwing →
TARGET_BEHAVIOR; wrong-signature / overloaded →WRAPPER_BINDING; source-defined / ambiguous two-assembly / arg-convert-fail / ambiguous-handler →CALLSITE_BINDING; two converted callsites → pass; net9 (same-family-newer) / net48 (mscorlib cross-family) / missing-dependency / dep-next-to-probe / dep-in-default-context →WRAPPER_RUNTIME_UNSUPPORTED; netstandard2.0 (not rejected by the family predicate) / dep-in-slot → pass; two slot versions prove the frozen first-winning slot; no absolute path in evidence.Changed files (new surface only)
Steps 0–10 remain source-identical —
git diff main...HEADoverfix_candidates.py,fix_plan.py,fix_apply.py,fix_bundle.py,fix_gate.py,fix_delta.pyand the Step 8/9/10 tests is empty. Step 12 not started. Rev F contract + G1–G5 preserved. Not merged — awaiting final arbiter review.🤖 Generated with Claude Code
https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G