Skip to content

feat(p3_stream): static stream cycle detection at fusion time (#142 iii)#188

Merged
avrabe merged 4 commits into
mainfrom
feat/stream-validation
May 25, 2026
Merged

feat(p3_stream): static stream cycle detection at fusion time (#142 iii)#188
avrabe merged 4 commits into
mainfrom
feat/stream-validation

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented May 25, 2026

Summary

Closes part of #142 (sub-issue of #94) — implements check (iii) only: SCC ≥ 3 cycle detection in the directed producer→consumer graph from detected StreamPairs. The legal bidirectional-pipe pattern (size-2 SCCs from two independent streams in opposite directions) is explicitly excluded.

Earlier scope (i) dropped after Mythos finding

An earlier commit on this branch also implemented check (i) (type-compatibility) via a role-list heuristic. The Mythos delta-pass auto-scan on the PR correctly identified it as a false-positive path:

validate_stream_pair_graph uses fusion_connections, which returns every component pair sharing any resolved import (sync or stream); when two components are connected via a sync call but each has unrelated stream roles with mismatched element types, the role-list pass raises a false-positive TypeMismatch …

Acknowledged and fixed in commit 3fecb84: the heuristic was dropped. A precise per-import-edge check needs resolved_imports type lookups (walk import descriptors, find stream<T>-typed imports, compare to matched export). LS-R-11 stays status: open tracking that follow-up; the StreamValidationIssue enum + resolver wiring are left in place so the precise variant can plug in later without a public-signature break.

What this PR actually ships

  • Cycle detection via Tarjan SCC over StreamPairs' producer→consumer edges
  • Error variant Error::StreamValidation(String) that batches every issue from a single fusion attempt
  • Pure validate_from_pairs so tests don't need ParsedComponent fixtures
  • One approved LS-N entry (LS-R-12) with full causal-factor + process-model-flaw analysis
  • One open LS-N entry (LS-R-11) tracking the precise (i) check, with a notes: block explaining the withdrawal and what shape the follow-up needs

Tests (4 new)

  • ls_r_12_stream_three_component_cycle_flagged — 3-component A→B→C→A loop
  • four_component_cycle_flagged — A→B→C→D→A (size-4 SCC)
  • bidirectional_pipe_is_not_flagged_as_cycle — negative: legal A↔B pipe stays clean
  • linear_pipeline_is_not_flagged — negative: acyclic A→B→C pipeline stays clean

Full cargo test -p meld-core --lib — 270 tests pass.

Test plan

  • All four cycle-detection regression tests green
  • Full meld-core lib suite passes
  • Mythos delta-pass auto-scan re-runs after rename + scope reduction
  • LS-N verification gate finds ls_r_12_*

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

Mythos delta-pass required

This PR modifies one or more Tier-5 source files (per
scripts/mythos/rank.md):

meld-core/src/p3_stream.rs
meld-core/src/resolver.rs

Before merge, run the Mythos discover protocol on the
modified Tier-5 files:

  1. Follow scripts/mythos/discover.md
    — one fresh agent session per touched Tier-5 file.
  2. For each finding, the agent must produce both a Kani
    harness and a failing PoC test (per the protocol's
    "if you cannot produce both, do not report" rule).
  3. Attach a comment on this PR with either the findings
    (formatted per discover.md's output schema) or
    NO FINDINGS.
  4. Add the mythos-pass-done label to this PR.

Why this gate exists: LS-A-10
(CABI alignment padding in async-lift retptr writeback) was
found by the v0.8.0 pre-release Mythos pass — but it had
lived in the callback emitter since #128, across six
releases. A PR-time gate would have caught it at review
time instead of at the release boundary.

The gate check on this PR will pass once the label is
applied.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

LS-N verification gate

⚠️ 34/35 verified — 1 missing regression tests

count
Passed (≥1 test, all green) 34
Failed (≥1 test failure) 0
Missing (no ls_*_NN_* test found) 1

Approved loss-scenarios.yaml entries are expected to have a
regression test named ls_<letter>_<num>_* (e.g. LS-A-11
ls_a_11_*). The gate runs each prefix via cargo test --lib --no-fail-fast and aggregates pass/fail/missing.

Failed LS entries

(none)

Missing regression tests
  • LS-R-13

Updated automatically by tools/post_verification_comment.py.
Source of truth: safety/stpa/loss-scenarios.yaml.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

Mythos delta-pass (auto)

NO FINDINGS across 2 Tier-5 file(s)

File Verdict Hypothesis
`` ✅ NO FINDINGS
`` ✅ NO FINDINGS

Auto-run via anthropics/claude-code-action@v1
(SHA-pinned) on the touched Tier-5 files, using the
maintainer's Max-plan OAuth token. See
.github/workflows/mythos-auto.yml and
scripts/mythos/discover.md.

@github-actions github-actions Bot added the mythos-pass-done Mythos delta-pass completed on Tier-5 file changes; findings (or NO FINDINGS) attached to PR label May 25, 2026
@avrabe avrabe changed the title feat(p3_stream): static stream validation at fusion time (#142 i+iii) feat(p3_stream): static stream cycle detection at fusion time (#142 iii) May 25, 2026
avrabe added a commit that referenced this pull request May 25, 2026
#189)

The musl-target-not-installed mitigation already in fuzz.yml's "Install
nightly Rust" comment prevents one failure mode (explicit musl target
in the dtolnay/rust-toolchain action). But the underlying #168 hazard
still surfaces when a drifted smithy runner has crt-static configured
via /etc/cargo/config.toml or an inherited RUSTFLAGS env var — even on
a gnu-host build, cargo-fuzz's `-Z sanitizer=address` then crashes
with "sanitizer is incompatible with statically linked libc."

This just recurred on PR #188 (fuzz_resolver_terminates +
fuzz_fusion_roundtrip), so the meld-side mitigation needs to be
stronger than "trust runner config." Setting RUSTFLAGS at the
workflow env level completely overrides any cargo-config rustflags
(cargo gives env-RUSTFLAGS strict precedence). On clean runners the
target feature was already off, so the flag is a harmless no-op; on
drifted runners it force-disables static-crt up-front before
cargo-fuzz ever calls into rustc.

This doesn't replace the upstream smithy ask in #168 (re-image the
rust-cpu runner pool) — that's still the durable fix. But it stops
the noise on every fuzz PR landing on a drifted runner until smithy
gets to it.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
avrabe and others added 3 commits May 25, 2026 12:15
Implements two of the four build-time checks from #142's scope table:

  (i)   Stream element-type compatibility — fusion-connected components
        where one side has producers and the other consumers, but no
        element-type combination matches. `pair_streams` already drops
        these silently; the validator now surfaces them as
        Error::StreamValidation so fusion fails loudly instead of
        producing a misconfigured module.
  (iii) Multi-component stream cycles — Tarjan SCC over the directed
        producer→consumer graph built from StreamPair entries. SCCs of
        size ≥ 3 are reported as deadlock candidates. Size-2 SCCs are
        explicitly excluded: the bidirectional-pipe pattern (two
        independent streams in opposite directions) is a legal SCC of
        size 2 and must not false-positive.

Validation logic split into a pure `validate_from_roles(roles,
connections, graph)` so the seven new regression tests pin behavior
without having to construct full ParsedComponent fixtures.

Resolver wiring runs immediately after build_stream_pair_graph;
collects every issue from a single fusion attempt and formats them
into a single error message so users can act on the whole punch list.

Two new approved LS-N entries (LS-R-11 type-mismatch, LS-R-12 cycle)
document the failure modes and cite the regression tests that pin them.

Checks (ii) "bounded-channel capacity" and (iv) "resource lifetime
across async boundaries" remain TODO.

Closes part of #142 (i+iii). #142 stays open for ii+iv.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… pattern

The LS-N verification gate (`tools/post_verification_comment.py`)
expects per-entry tests named `ls_<letter>_<num>_*` so that `LS-R-11`
maps to `ls_r_11_*`. The two new tests shipped as
`ls_stream_1_*` / `ls_stream_2_*` and the gate flagged them as
missing.

Rename:
- `ls_stream_1_type_mismatch_detected`
  → `ls_r_11_stream_type_mismatch_detected`
- `ls_stream_2_three_component_cycle_flagged`
  → `ls_r_12_stream_three_component_cycle_flagged`

Update the `fix:` citation strings in `safety/stpa/loss-scenarios.yaml`
for LS-R-11 and LS-R-12 to match.

No behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The mythos-auto delta-pass scan on the original PR correctly flagged a
false-positive path in the TypeMismatch detection: when two components
are sync-connected (via any non-stream resolved import) and each
happens to have stream operations with element types that don't pair,
the role-list pass would still raise a TypeMismatch and the resolver
would convert that into Err(Error::StreamValidation), spuriously
rejecting a valid composition.

Example of the false positive:
  - Component A produces stream<u8> (for some third component)
  - Component B consumes stream<s32> (from some fourth component)
  - A and B are sync-connected via a regular function call
  - role-list pass sees: A has producer roles, B has consumer roles,
    no element-type match → flags TypeMismatch (incorrect)

A precise implementation needs per-import-edge type lookups (walk
resolved_imports, find which imports are stream<T>-typed, compare to
the matched export's element type). That's a different shape of code;
LS-R-11 stays `status: open` tracking the follow-up.

What this PR ships:
  - Check (iii): SCC ≥ 3 cycle detection in the producer→consumer
    graph from StreamPairs. Robust, no false positives — the legal
    bidirectional-pipe pattern (size-2 SCC) is explicitly excluded.
  - StreamValidationIssue enum + Error::StreamValidation + resolver
    wiring left in place so the precise (i) implementation can plug
    in later without a public-signature break.

Test changes:
  - Removed three TypeMismatch tests
    (`ls_r_11_stream_type_mismatch_detected`,
    `type_mismatch_not_reported_when_any_pair_matches`,
    `type_mismatch_not_reported_when_one_side_has_no_streams`).
  - Renamed `validate_from_roles` → `validate_from_pairs` since
    roles + connections are no longer load-bearing for the only
    remaining check.
  - Four cycle-detection tests still pin behavior.

LS-R-11: status flipped from `approved` to `open`, with a notes:
block explaining why the heuristic was withdrawn and what shape the
precise check needs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@avrabe avrabe force-pushed the feat/stream-validation branch from 3fecb84 to 250735e Compare May 25, 2026 10:15
…deep chains

Mythos delta-pass on the previous commit identified that the recursive
`strongconnect` inner function in `strongly_connected_components`
exhausts the default 8 MB Rust thread stack at roughly N ≈ 40 000 nodes
on a linear A→B→C→...→Z chain. Meld's practical fusion input is bounded
by component count and won't hit this in normal operation, but the
hazard is reachable from any pathological or attacker-crafted graph
accepted by the validator.

Initial fix attempt was to delegate to `petgraph::algo::tarjan_scc`.
The new regression test (50 000-node deep chain) still overflowed —
petgraph 0.8's tarjan_scc is also recursive. So the real fix is an
iterative Tarjan: each work-stack frame carries (node, next-successor-
index); on first entry the index is 0 (initialization + SCC-stack
push); on subsequent visits we've just returned from
`successors[index - 1]` and fold that child's lowlink into ours
before scanning further.

  - `meld-core/src/p3_stream.rs::strongly_connected_components`
    rewritten as iterative Tarjan with an explicit work-stack
  - New regression test `deep_linear_chain_does_not_overflow_stack`
    builds a 50 000-component linear chain (well past the recursive
    bound) and asserts no cycles are reported. This pins the
    iterative backend in place — any future refactor to a recursive
    library would fail this test.
  - New approved LS-N entry LS-R-13 documenting the hazard, the
    causal factors (including the "library is iterative" wrong
    assumption), and the regression test that pins the fix.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@avrabe avrabe merged commit fada193 into main May 25, 2026
13 of 15 checks passed
@avrabe avrabe deleted the feat/stream-validation branch May 25, 2026 11:49
@avrabe avrabe mentioned this pull request May 25, 2026
5 tasks
avrabe added a commit that referenced this pull request May 25, 2026
Four-PR cycle. Cycle detection for cross-component streams (#188),
signed/attested release pipeline (#186, first real cut), README
consumer verification recipe (#187), and a defensive fuzz workflow
fix for drifted self-hosted runners (#189).

v0.12.0 is the first meld release published with the new artifact
flow: per-target tar.gz archives, CycloneDX SBOM, cosign-signed
SHA256SUMS.txt + bundle, and SLSA v1 build provenance per archive.
v0.11.0's binaries were the last bare-asset release.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
avrabe added a commit that referenced this pull request May 26, 2026
…t filter

Restores check (i) from #142 after the role-list heuristic was withdrawn from
PR #188 (Mythos delta-pass finding). Walks each fusion connection's
resolved_imports; if at least one edge declares stream<T> in its
ComponentTypeRef signature (Func params/results, Type aliases, Instance
exports — recursing through ComponentValType composites), applies the
role-list pair check. Sync-only connections with unrelated streams are
correctly skipped.

5 new regression tests including the Mythos finding's former false positive
and the true-positive case.

Bundles fuzz workflow layer-2: CARGO_BUILD_TARGET=x86_64-unknown-linux-gnu
defeats the second #168 drift mode where libfuzzer-sys built.rs invokes
x86_64-linux-musl-g++ on runners with [build] target = musl cached.

LS-R-11: open → approved.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
avrabe added a commit that referenced this pull request May 26, 2026
…t filter (#191)

Restores check (i) from #142 after the role-list heuristic was withdrawn from
PR #188 (Mythos delta-pass finding). Walks each fusion connection's
resolved_imports; if at least one edge declares stream<T> in its
ComponentTypeRef signature (Func params/results, Type aliases, Instance
exports — recursing through ComponentValType composites), applies the
role-list pair check. Sync-only connections with unrelated streams are
correctly skipped.

5 new regression tests including the Mythos finding's former false positive
and the true-positive case.

Bundles fuzz workflow layer-2: CARGO_BUILD_TARGET=x86_64-unknown-linux-gnu
defeats the second #168 drift mode where libfuzzer-sys built.rs invokes
x86_64-linux-musl-g++ on runners with [build] target = musl cached.

LS-R-11: open → approved.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@avrabe avrabe mentioned this pull request May 26, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mythos-pass-done Mythos delta-pass completed on Tier-5 file changes; findings (or NO FINDINGS) attached to PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant