audit(xaml): Phase-2 join — XAML203 view-subscription leak (build-free)#117
Merged
Conversation
First slice of the XAML<->C# Phase-2 join from the design note. Links the two fact sources by the deterministic XAML naming convention — NOT the .g.cs build artifact — so it stays build-free (Linux CI, non-compiling solutions). - xaml_join.py: join(xaml-facts, OwnIR) -> XAML203. A view whose x:Class component has an OwnIR subscription flagged released=false, wired from a load-lifecycle handler (Loaded/Initialized/DataContextChanged), is the closed-view-retained leak; anchored at the XAML site, names the C# subscription. released=false is authoritative (the engine already checked for -= across the class), so no XAML Unloaded heuristic. SARIF emit + CLI + 8-check selftest. - xaml_facts.py: also capture x:Name (named_elements) — the join's element->field link. - own-check.sh: add --emit-facts to persist the OwnIR facts; owncheck.py writes own-check.facts.json so the join has a real input in an SDK run. - run_static.py: run the join when xaml-facts.json + own-check.facts.json are both present; fold xaml-join.sarif into the pipeline (+ integration selftest). - taxonomy: XAML203 -> category 2 (subscription leak, P1). - design note + README: document the convention-as-link decision, the first rule, and .g.cs/BAML as deferred build-tier ground-truth (not the mechanism). Binding-path-hotness (XAML200/204, needs the DataContext type) is the next increment, deliberately not guessed here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe audit pipeline now emits OwnIR facts, extends XAML fact collection with named elements, adds a XAML join that reports XAML203, wires the join into static aggregation when both fact files are present, and updates CI, docs, and taxonomy. ChangesXAML Phase-2 join
Sequence Diagram(s)sequenceDiagram
participant XamlFacts as xaml_facts.py
participant OwnCheckPy as owncheck.py
participant OwnCheckSh as own-check.sh
participant RunStatic as run_static.py
participant XamlJoin as xaml_join.py
participant Aggregate as aggregate()
XamlFacts-->>RunStatic: xaml-facts.json
OwnCheckPy->>OwnCheckSh: --emit-facts <facts_path>
OwnCheckSh-->>OwnCheckPy: own-check.facts.json
RunStatic->>XamlJoin: run_join(xaml-facts.json, own-check.facts.json)
XamlJoin-->>RunStatic: xaml-join.sarif and tier status
RunStatic->>Aggregate: include xaml-join SARIF
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.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: 7bb7912e4e
ℹ️ 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".
…-name views (Codex) Two correctness fixes from the Codex review on PR #117: - XAML203 now fires only on event-subscription records. OwnIR's components[].subscriptions is an umbrella list that also carries timer / IDisposable / pool leaks tagged via `resource`; _unreleased previously treated all released=false entries as event subscriptions, mislabeling e.g. an unreleased DispatcherTimer as a cat-2 [resource: subscription token] leak. Restricted to subscription-family resources (untagged += or subscribe/ subscription token); timer/disposable/pool stay own-check's cat-3/1. - Same-simple-name views are disambiguated by code-behind path. The component index keys on the unqualified type name, so Admin.CustomerView and Sales.CustomerView collided and a leak in one could land XAML203 on the other's XAML. _linked_components now requires code-behind-path correspondence (segment-aware suffix match of the .xaml stem vs the .xaml.cs/.cs stem) when the simple name is ambiguous, and links nothing rather than cross-link. Selftest grows to 11 checks (timer-not-XAML203; Sales leak lands on Sales XAML, never Admin's). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb
Owner
Author
|
The two review findings (filter XAML203 to event subscriptions; disambiguate same-simple-name views by code-behind path) are fixed in @coderabbitai review Generated by Claude Code |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a940326101
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@audit/static/tools/owncheck.py`:
- Line 42: Clear stale own-check output in the static tools flow: in
owncheck.py, ensure own-check.facts.json is removed before invoking own-check
and also deleted on any non-success path so run_static only sees facts from the
current successful run. Update the logic around facts_path, own-check
invocation, and the failure handling branches to prevent xaml-join from
consuming leftover data from a previous target or commit.
In `@audit/static/tools/xaml_join.py`:
- Around line 120-123: The simple-name lookup in _component_index() can still
return the wrong file when there is only one basename match, so update the
matching logic to path-check that single candidate through _path_corresponds()
as well. In the branch that handles cands from by_name.get(...) in the XAML join
flow, only return the lone match when its file metadata is missing; otherwise
filter it against doc["file"] the same way as the multi-candidate case to avoid
cross-linking the wrong XAML file.
🪄 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: a0c0fa2a-3879-48b4-923f-98210e5e933f
📒 Files selected for processing (9)
.github/workflows/ci.ymlaudit/README.mdaudit/static/run_static.pyaudit/static/taxonomy/categories.ymlaudit/static/tools/owncheck.pyaudit/static/tools/xaml_facts.pyaudit/static/tools/xaml_join.pydocs/notes/xaml-analyzer-design.mdscripts/own-check.sh
…eck, stale cleanup Round-two review fixes (Codex + CodeRabbit) on the Phase-2 join: - xaml_join: include `resource: "capture"` in the XAML203 filter. capture is a static/process-lived event subscription that retains the subscriber (the OWN014 region-escape case) — exactly the closed-view-retained leak XAML203 targets — so the prior allow-list wrongly excluded it. (Codex) - xaml_join: path-check the unique simple-name match too. The name index drops the namespace, so a lone basename hit could cross-link App.Views.CustomerView.xaml to an unrelated Legacy.CustomerView.cs. _linked_components now path-checks every candidate and only falls back to a lone match with no file metadata. (CodeRabbit) - run_static: gate the join on the fact files THIS run produced (the statuses' `facts` paths), not bare existence — a re-run into an existing --out, or a profile without xaml/own-check (or no SDK), must not join a previous target's stale facts. + selftest: join runs only when both producers ran this invocation; a stale own-check.facts.json on disk is NOT joined. (Codex) - owncheck: clear own-check.facts.json up front so a failed/unavailable run leaves no stale facts behind (own-check.sh writes it only on success). (CodeRabbit) Selftests: xaml_join 13, run_static 22. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zoG4YNCUf7RA5r8GAXMmb
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.
First slice of the XAML ↔ C# Phase-2 join from the design note — the step where the interprocedural engine stops being decoration and starts accusing XAML with evidence. Follows #114 (Phase-1 markup rules) and #115 (the facts layer).
The one design decision: link by convention, not by
.g.csWPF's markup compiler emits a
.g.cs(InitializeComponent / IComponentConnector glue,x:Namefields,event += handlerwiring) — but only intoobj/after a successful markup-compile, which would drag the join into the build-required tier, and a freshly-cloned legacy target (the kind own-check exists for) has no current.g.cs. We don't need it: the wiring it encodes is a fixed contract we synthesize without building —x:Class="App.Views.CustomerView"→ code-behind typeCustomerView(OwnIRcomponents[].name)Loaded="OnLoaded"/Click="OnSave"→ method on that typex:Name="btn"→ generated field{Binding Qty}→ DataContext propertySo the join stays build-free (Linux CI, non-compiling solutions).
.g.csis reserved as an optional build-tier ground-truth cross-check (catch compiler-only edge cases like attached events / EventSetter connection-ids), folded in later like the other build-required tiers — never the mechanism. BAML (post-build binary) is off-roadmap.What's in it
audit/static/tools/xaml_join.py—join(xaml-facts, OwnIR) → XAML203. A view whosex:Classcomponent has an OwnIR subscription flaggedreleased=false, wired from a load-lifecycle handler (Loaded/Initialized/DataContextChanged), is the closed-view-retained leak — anchored at the XAML site, naming the C# subscription.released=falseis authoritative (the acquire/release engine already checked for a matching-=across the whole class, code-behind included), so no XAMLUnloadedheuristic. SARIF emit + CLI + 8-check selftest.xaml_facts.py— also capturex:Name(named_elements): the element→field link the join uses.own-check.sh— new--emit-facts <path>persists the OwnIR facts the extractor already produces;owncheck.pywritesown-check.facts.jsonso the join has a real input in an SDK run. The verdict is unchanged.run_static.py— runs the join wheneverxaml-facts.json+own-check.facts.jsonare both present and foldsxaml-join.sarifinto the pipeline (opportunistic, like the other pickups). + integration selftest.XAML203→ category 2 (subscription leak, P1). Note: the leak lives in the.xaml.cs, a different basename, so XAML203 does not spatially cluster with the own-checkOWN001— the join is the cross-source link, carried in the message..g.cs/BAML as deferred build-tier ground-truth.Example output
For a view
CustomerView.xaml(x:Class,Loaded="OnLoaded") whose code-behind subscribes_bus.CustomerChangedwithout-=:What's deferred (next increments)
.g.csground-truth cross-check — build-tier validation of the convention-derived wiring.Validation
Full audit selftest suite green (normalize/score/report/xaml_check/xaml_facts/xaml_join/run_static/ingest = 173 checks),
ruff check .clean,bash -n own-check.shclean. The join and its wiring gate on Linux CI with no .NET.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit