Skip to content

feat(alpha): first-run contract, flagship leak demo, and the retention-path witness MVP (arcs A1–A3) - #309

Merged
PhysShell merged 11 commits into
mainfrom
claude/complex-project-tasks-viyycs
Jul 26, 2026
Merged

feat(alpha): first-run contract, flagship leak demo, and the retention-path witness MVP (arcs A1–A3)#309
PhysShell merged 11 commits into
mainfrom
claude/complex-project-tasks-viyycs

Conversation

@PhysShell

@PhysShell PhysShell commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Что и зачем

Подготовительный пакет Owen Alpha — три закрытые арки, каждая с исполняемым доказательством:

A1 — контракт первого запуска. Закрыта настоящая soundness-дыра оболочки: краш Python-ядра давал exit 1 (traceback), и owen check без --fail-on-finding маппил его в 0 — clean scan. Теперь: ядро — catch-all → exit 70 (EX_SOFTWARE), одна вежливая строка (OWNLANG_DEBUG=1 печатает traceback, но код остаётся 70); CLI — top-level catch → exit 5, diagnostic-отчёт в ~/.owen/diag/last-failure.json (без исходников; facts — только явным --emit-facts); --debug меняет объём диагностики, не машинную семантику (exit 5 в обоих режимах). Вывод extractor'а буферизуется — контрактные коды проходят как раньше, неожиданный код фреймится вежливо. Опечатки флагов — usage error, --help документирует полную таблицу кодов. Плюс: EN issue-шаблон «owen CLI problem», секция Known limitations (unsupported-by-design ≠ баг), публикуемый benchmark-scorecard (--json + step summary: числа с корпусом, ревизией и методикой).

A2 — flagship-репро. Сюжет #278 в ~80 кроссплатформенных строках (examples/flagship/console/): статический publisher, подписка в конструкторе, -= существует за if (!keepAlive) — а каждый Close передаёт true. bad/ сам печатает «1000 still subscribed», ok/ (release в Dispose()) — «0». Оба dotnet run-able и owen check-able; opt-in hold-хук OWEN_FLAGSHIP_HOLD=1 для witness/CI.

A3 — witness MVP. audit/runtime/RetentionPath извлечён из PR #280 (минус dominator tree и устаревший corpus-срез), ретаргет net472→net8.0 (Linux/Windows live-attach через ClrMD). Два содержательных фикса, найденных живыми прогонами: (1) двухфазный BFS — durable-корни (handles/statics) обходятся до исчерпания раньше transient (stack/finalizer), иначе локаль Main в регистре присваивала статический publisher как [stack] и маскировала static-event retention (инвариант с доказательством зафиксирован в коде); (2) вердикт консультируется с классификациейRETAINED/exit 1 требует durable-retainer'а, чисто transient-достижимость — OBSERVED_ONLY/exit 0 (loop-local — не утечка). Классификатор — явная граница Retainer.Classify: известные ClrRootKind перечислены, delegate-evidence по структуре пути (тип «...StackMachine...» — не evidence), неизвестный kind → unsupported-root: — честный отказ. JSON-артефакт (verdict + collector identity) пишется для каждого вердикта.

Пост-ревью (Codex + follow-up). 2f064dd: таргетируются ВСЕ инстансы типа (не первые N в порядке кучи), сигнатура группы путей несёт классификацию корня и поля хопов, обрезка --max-hops доматывает parent-chain до истинного корня, rc-70 ядра пишет diagnostic-отчёт через общий CrashReport. f389b92 (завершение инварианта): вердикт берётся из точного root-kind-ценза по каждому достижимому инстансу — --sample/--max-hops ограничивают только отображение и математически не могут изменить вердикт/exit code; durable-инстансы разрешаются первыми (RETAINED всегда с durable-путём на экране); доли рендерятся от разрешённых путей.

Demo-оркестратор scripts/flagship-demo.sh — тупой по контракту: build → hold → witness через публичный CLI с timeout → машинная JSON-валидация против human-вердикта (расхождение — fail) → семантические якоря, не дословные пути → одна стабильная итоговая строка → trap-cleanup. Gate A запускает его для обоих вариантов на Linux (с CI-only послаблением Yama ptrace_scope; сам скрипт sudo-free — attach, запрещённый политикой ОС, остаётся честным exit 2).

Тип изменения

  • feat — новая возможность
  • fix — исправление бага
  • docs — документация
  • refactor / chore / test / ci — без изменения поведения

Как проверено

  • python tests/run_tests.py — все сьюты, включая новый tests/test_cli_contract.py (4 проверки контракта ядра)
  • ruff check . и mypy
  • селфтесты затронутых скриптов — scripts/benchmark.py --selftest расширен (scorecard, 17 проверок) и гоняется в lint-джобе; RetentionPath selftest (16 фикстур: классификатор, вердикт, когерентность ClrMD- и строкового уровней доктрины) гоняется в gate A на обеих платформах
  • Локально на установленном .NET 8 SDK: сборка CLI/witness/примеров, живой attach (bad → RETAINED [static-event], 6 hops; с --sample 50 против 1000 вью — «1,000 durably retained (50 paths resolved for display)»; ok → OBSERVED_ONLY), оба варианта оркестратора зелёные
  • CI ветки и PR зелёный на каждом коммите пакета, включая gate-A crash-инъекции, TMPDIR top-level-exception smoke и end-to-end demo на раннере (head f389b92)

Связанные issue

Refs #250/#252/#253/#254 (Owen Alpha: этот пакет закрывает инженерную часть подготовки; владельческий гейт #252 — лицензия/версия/environments/секрет — остаётся единственным блокером публикации), #278 (сюжет flagship-репро), #280 (источник witness-ядра; dominator tree и старый corpus-срез сознательно не взяты), #270 (runtime witnesses).

Чеклист

  • изменение покрыто тестом/селфтестом (или объяснено, почему нет)
  • README/docs обновлены при необходимости (CLI README: exit-таблица + Known limitations; spec/CLI.md: контракт exit 70; examples/flagship/README.md)
  • коммиты в conventional-commit стиле (feat:, fix:, docs: …)

Generated by Claude Code

claude added 9 commits July 26, 2026 10:49
… never a trace, never a clean scan

The alpha's worst first-run bug: an internal crash of the Python core
exited 1 (traceback), and `owen check` without --fail-on-finding mapped
rc<2 to 0 — a crash reported as a CLEAN scan. The whole failure surface
is now under contract:

Core (python -m ownlang):
- ownlang.__main__.run(): catch-all — internal crash exits 70
  (EX_SOFTWARE) with one actionable line, no traceback; OWNLANG_DEBUG=1
  prints the full traceback but KEEPS exit 70 (a re-raise would exit 1
  and reopen the clean-scan hole). Deliberate contract errors keep their
  codes. Documented in spec/CLI.md; pinned by tests/test_cli_contract.py.
- ownir.load(): an unreadable facts file is a polite OwnIRError (exit 2),
  not an OSError traceback.

owen CLI:
- top-level catch (Program.cs) + CrashReport: internal error -> exit 5,
  short message, deterministic diagnostic report at
  ~/.owen/diag/last-failure.json (tool/OS/runtime, command line, stage,
  cause — NO source contents; facts sharing stays explicit --emit-facts);
  --debug / OWEN_DEBUG=1 re-throws the full cause.
- core exit 70 -> framed politely -> exit 5 (the clean-scan hole closed).
- extractor output is captured: contract codes (0/2/4) pass through
  verbatim; an unexpected exit code is framed as an internal error with
  the raw trace in the report (or on stderr under --debug) -> exit 5.
- unknown options are a usage error (exit 2) with a hint, not phantom
  paths ('owen check --verbose .' used to exit 4 'path does not exist').
- --emit-facts write failures are a polite exit 2.
- --help documents the full exit-code table incl. 5 and --debug.

Gate A (CI): new smoke steps — unknown option is exit 2; --help documents
the contract; a crash-injected core (OWEN_PYTHON wrapper, Linux) surfaces
as exit 5 politely and never as a clean scan, and --debug shows the full
cause while still exiting 5.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
The bug-report template asks exactly what triage needs (version, OS/SDK,
command, exit code, output, optional diagnostic report / sanitized
facts — with an explicit review-before-attaching note), and routes the
documented exit-3/exit-4 behaviors away from the bug queue. The Known
limitations section separates unsupported-by-design (frontend scope,
symbol-tolerant analysis of non-compiling projects, alpha rule families,
static-only evidence, versioned vocabulary, Python requirement) from
bugs, per the alpha first-run contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
Benchmark publication (A1): scripts/benchmark.py gains --json — the
publishable scorecard carrying the aggregate numbers WITH the corpus
dirs/case counts, the git revision and the methodology (a metric without
those is just a nice percentage nobody can audit). Under GitHub Actions
the same numbers land in the job step summary; the corpus-benchmark CI
job uploads the scorecard as a 90-day artifact. Shape and determinism
pinned in --selftest.

Flagship console repro (A2, examples/flagship/): the #278 shape reduced
to ~80 cross-platform lines — a static publisher, a view subscribing in
its ctor, an unsubscribe that EXISTS behind 'if (!keepAlive)' while every
close path passes keepAlive: true. bad/ runs and prints '1000 still
subscribed'; ok/ moves the release into Dispose() (provable teardown,
mirroring the corpus-pinned predicate family) and prints 0. Both are
dotnet-runnable and owen-checkable; gate A smoke-asserts bad → exit 1 +
OWN001 and ok → exit 0 against the installed tool on Linux and Windows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
The published contract says every internal failure exits 5, but the
top-level handler re-threw under --debug/OWEN_DEBUG=1 — the process then
died with a runtime-chosen, platform-dependent code. Debug now changes
the VOLUME of diagnostics (the full exception is printed to stderr), not
the machine semantics: report written, exit 5, both modes. Gate A gains
the previously-untested top-level-exception smoke (TMPDIR pointed at a
missing directory fails Path.GetTempFileName before any inner catch):
polite + no stack by default, full stack under --debug, exit 5 in both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
…inus the dominator tree

The runtime-witness half of the flagship demo, resurrected as the minimal
core rather than a rebase of the geological layer: Heap.cs (mark-from-roots
census, sampled ranked retainers with the field named at every hop,
stack/finalizer vs real retention) + Program.cs (census / roots,
--pid|--dump, --type, --sample/--max-hops limits, --out runtime.json) +
the csproj. Deliberately NOT taken: Dominators.cs (444 lines of separate
proof surface the hero demo does not need), the pre-#278/#305/#306 corpus
slice, and the old runtime-contract docs.

Adapted to the A3 contract:
- verdict vocabulary — RETAINED (a root path was established, exit 1) |
  OBSERVED_ONLY (instances exist, no path established — never called a
  proven leak, exit 0) | ABSENT; usage documents it;
- the JSON artifact carries verdict + collector identity (mode
  attach/dump, target, runtime, OS — no timestamps: identical heaps must
  yield identical artifacts);
- a failed read stays exit 2, distinct from clean(0)/retained(1);
- net8.0 cross-platform (ClrMD 3.x live-attach works on Linux and
  Windows; legacy net472 heaps are dump-only from here — the PR #280
  live-attach build can be resurrected the day a live legacy target
  matters).

Gate A builds the project and smokes the honest-failure surface on both
platforms: bare usage exits 2 with the verdict vocabulary documented, and
a failed attach exits 2 — never a clean scan.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
…ry; live static-event verified

The first live net8 run exposed two masked layers, both now fixed and
pinned:

1. Root seeding masked the durable evidence. The static publisher is BOTH
   a stack-root object (a Main local holds it in a register) and reachable
   from the pinned statics object[]; single-pass seeding claimed it as a
   [stack] root before BFS could reach it durably, so the flagship's
   static-event retention read as 'live right now, not retention' and the
   dominant-retainer callout was suppressed. FindRetainers now runs TWO BFS
   PHASES: durable roots (handles/statics) are seeded and walked to
   exhaustion before any transient root (stack/finalizer) enters the graph
   — retention analysis prefers durable evidence; the stack only explains
   what nothing durable can.

2. The classifier is now an explicit boundary (Retainer.Classify), pure
   over (root kind, path evidence): every known ClrRootKind is named;
   delegate evidence (EventHandler/MulticastDelegate hop types, the
   _invocationList FIELD — types checked as types, fields as fields, so a
   type merely named '...StackMachine...' is not evidence) upgrades
   pinned/handle roots to static-event; an UNKNOWN kind reports
   'unsupported-root:<kind>' — honest refusal, never silently non-root.

Pinned at two levels:
- 'RetentionPath selftest' — 7 heap-free classifier fixtures: the verbatim
  live net8 static-event path, the stack-flavoured-names negative
  neighbour, stack/finalizer doctrine (delegate evidence does NOT upgrade
  a genuine stack root), plain static-field, and the honest-refusal case.
  Gate A runs it on both platforms.
- Gate A end-to-end (Linux): the flagship bad app held live
  (OWEN_FLAGSHIP_HOLD=1 — new opt-in hook on both samples, also what the
  demo orchestrator will use), retention-path attaches and must report
  verdict RETAINED via static-event with exit 1, in the human output and
  the JSON artifact both.

Verified live locally before CI: the same attach that read
'[stack], 5 hops / no single dominant retainer' now reads
'[static-event], 6 hops (object[] -> AppSettings -> PropertyChanged ->
_invocationList -> handler -> DocumentView) / 100.0% hang off ONE
reference'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
scripts/flagship-demo.sh — the reproducible demo entrypoint, kept a DUMB
orchestrator per the arc contract: build sample+witness, hold the app
live (OWEN_FLAGSHIP_HOLD), attach retention-path through its PUBLIC CLI
with a timeout, machine-validate the JSON artifact (python, not grep)
against the human verdict — a disagreement is a failure in itself —
assert stable semantic anchors (AppSettings / PropertyChanged /
_invocationList / DocumentView / static-event), never verbatim paths or
addresses, print ONE stable summary line, clean up via trap. No
classification logic of its own.

Running the ok variant through it immediately caught a real verdict gap:
the last loop iteration's view stays live in Main's frame, and the
witness reported RETAINED for a [stack]-only path — the verdict did not
consult the classification. Per the transient-root doctrine a stack path
proves 'live right now', not retention: 'roots' now requires at least
one DURABLE retainer for RETAINED/exit 1; all-transient reachability is
OBSERVED_ONLY/exit 0 with the transient paths shown as explanation.
'unsupported-root:*' counts as durable on purpose — unknown evidence
surfaces loudly, never quietly demotes the verdict.

Also pinned in code: the two-phase-BFS invariant (durable traversal
claims reachable targets; transient traversal may share intermediates
but cannot overwrite durable ownership — with the exhaustion proof).

Gate A (Linux) now runs the orchestrator itself for both variants —
bad: 'leak DEMONSTRATED: static-event retention'; ok: 'fix VERIFIED: no
established retention' — so the demo entrypoint and the CI proof are the
same artifact. Both verified live locally before push.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
The orchestrator failed on the runner exactly at the predicted risk
point: ubuntu runners ship kernel.yama.ptrace_scope=1, which blocks
same-user non-ancestor PTRACE_ATTACH, so ClrMD could not attach to the
held flagship app (locally the session runs as root, where Yama does
not apply). The step now sets scope 0 first — a CI-runner-only
relaxation; the demo script itself stays sudo-free, and a real user
under scope 1 gets the witness's polite exit 2, never silence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
…nned; ok-demo checks the user contract

Arbiter refinements on the A3/demo vertical, all three:

1. The JSON artifact is no longer RETAINED-only: ABSENT and
   OBSERVED_ONLY write the same runtime.json (verdict + collector
   identity + any transient paths) when --out is given — the ok side of
   a demo is now as machine-checkable as the leak side, via one shared
   WriteArtifact writer.

2. The verdict rule is a pure function (IsDurableKind / VerdictOf) and
   the selftest pins it deterministically: stack-only and
   finalizer-only reachability are OBSERVED_ONLY; one durable retainer
   makes RETAINED; unsupported-root:* surfaces as RETAINED, never a
   quiet demotion. 11 heap-free checks now.

3. The ok-demo validator asserts the USER contract only — exit 0,
   JSON verdict in {ABSENT, OBSERVED_ONLY}, zero durable retainers,
   human/JSON agreement — instead of insisting on OBSERVED_ONLY: which
   of the two shows up depends on JIT liveness of a loop local, an
   internal CLR decision that is not a public API. The verdict
   SEMANTICS stay proven by the deterministic selftest; the live demo
   proves the absence of a false RETAINED. Also: trap extended to
   INT/TERM.

Verified live locally: selftest 11/11, demo bad 'leak DEMONSTRATED:
static-event retention', demo ok 'fix VERIFIED: no established
retention (OBSERVED_ONLY)'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@PhysShell, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 541def7f-e2ce-4455-ba9b-009b9323ef48

📥 Commits

Reviewing files that changed from the base of the PR and between 0b2d970 and f389b92.

📒 Files selected for processing (20)
  • .github/ISSUE_TEMPLATE/owen_cli_report.yml
  • .github/workflows/ci.yml
  • audit/runtime/RetentionPath/Heap.cs
  • audit/runtime/RetentionPath/Program.cs
  • audit/runtime/RetentionPath/RetentionPath.csproj
  • examples/flagship/README.md
  • examples/flagship/console/bad/BadDocumentApp.csproj
  • examples/flagship/console/bad/DocumentApp.cs
  • examples/flagship/console/ok/DocumentApp.cs
  • examples/flagship/console/ok/OkDocumentApp.csproj
  • frontend/roslyn/OwnSharp.Cli/CheckCommand.cs
  • frontend/roslyn/OwnSharp.Cli/CrashReport.cs
  • frontend/roslyn/OwnSharp.Cli/Program.cs
  • frontend/roslyn/OwnSharp.Cli/README.md
  • ownlang/__main__.py
  • ownlang/ownir.py
  • scripts/benchmark.py
  • scripts/flagship-demo.sh
  • spec/CLI.md
  • tests/test_cli_contract.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/complex-project-tasks-viyycs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3774e5f6c2

ℹ️ 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 thread audit/runtime/RetentionPath/Heap.cs Outdated
Comment on lines +125 to +130
foreach (var o in Heap.EnumerateObjects())
{
if (!o.IsValid || o.Type?.Name == null) continue;
if (!IsType(o.Type.Name, typeName)) continue;
totalOfType++;
if (targets.Count < sample) targets[o.Address] = o.Type.Name;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Sample across the complete target population

Selecting the first sample objects in heap-enumeration order can produce a false OBSERVED_ONLY verdict. For example, if 200 older instances are unreachable garbage and a later instance is held by a static, the default sample contains only garbage, SampledRetained becomes zero, and the actual durable retention is never examined. Use an unbiased sample spanning all matching instances, or continue until enough reachable instances have been collected.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 2f064dd: ALL matching instances are targeted now — the verdict is exact over the whole population; only path resolution stays sampled (bounded at --sample paths). A later durably-held instance can no longer hide behind older garbage.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up in f389b92 — the 2f064dd fix made reachability exact but left durable/transient classification on the resolved-path sample, which re-admitted the same bias one level up (200+ finalizer-queue-reachable corpses ahead of one durably-held instance in heap order → false OBSERVED_ONLY). Now a root-kind census walks the parent chain for EVERY reachable instance (dictionary hops only, no name resolution) and the verdict consults the exact durable count; path resolution stays bounded by --sample, durable instances resolved first so RETAINED always ships with a durable root path. Rendered shares now divide by resolved paths, not the population. Selftest 11 → 16: the ClrMD-level transient split and the string-level verdict rule are pinned to agree on every root kind.


Generated by Claude Code

Comment thread audit/runtime/RetentionPath/Heap.cs Outdated
Comment on lines +218 to +222
string signature = string.Join(" -> ", hops.Select(h => h.Type));

if (!groups.TryGetValue(signature, out var retainer))
{
retainer = new Retainer(hops, kind);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve root classification when grouping paths

The grouping key contains only hop type names, while the retained Retainer keeps the root kind of whichever matching path is encountered first. If stack-rooted and durably rooted instances have the same type sequence, they are merged and later verdict logic classifies the whole group using that first root; this can either hide a real durable retainer or turn transient reachability into a leak. Include the root kind—and ideally the distinguishing fields—in the signature.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 2f064dd: the group signature now carries Retainer.Classify(kind, hops) plus the per-hop fields (Hop.ToString), so stack-rooted and durably-rooted paths sharing a type sequence can no longer merge and inherit the first path's classification.


Generated by Claude Code

Comment on lines +247 to +251
if (!parent.TryGetValue(cur, out ulong p) || p == 0) break;
cur = p;
if (chain.Count > maxHops) break;
}
kind = rootKind.TryGetValue(cur, out var k) ? k : ClrRootKind.None;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Retain the real root kind when truncating long paths

When a path exceeds maxHops, unwinding stops at an intermediate object, so rootKind lookup yields ClrRootKind.None. Classify then turns that into unsupported-root:None, which IsDurableKind treats as durable; consequently, a target reachable only from a stack path longer than the limit is incorrectly reported as RETAINED. Path truncation must not discard the root classification used for the verdict.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 2f064dd: on truncation the (acyclic) parent chain is now walked to the true root WITHOUT recording hops — the rendered path stays bounded at --max-hops, but the verdict sees the real root kind. A long stack-only path can no longer surface as unsupported-root:None → durable → false RETAINED.


Generated by Claude Code

@@ -0,0 +1,34 @@
<Project Sdk="Microsoft.NET.Sdk">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep active retention-witness development in OwnAudit

This adds a new audit collector and makes it an actively built and executed CI component inside audit/, even though the repository explicitly designates the separate OwnAudit repository as the home and source of truth for active audit development until consolidation. Implementing the witness here creates the divergent source-of-truth state that the repository rule is intended to prevent; move this work to OwnAudit or perform the documented consolidation first.

AGENTS.md reference: AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placement was deliberate, not a drive-by, so answering rather than moving: (1) the decoupling half of the AGENTS.md rule IS honored — RetentionPath has zero coupling to ownlang and is consumed only through its public CLI/JSON; (2) the witness core originates from this repo's own PR #280, which placed it in audit/runtime/ — extracting its minimal core onto the current head (explicitly directed during the A3 arc) continues that lineage rather than forking OwnAudit; (3) the flagship demo and its gate-A end-to-end smoke need the witness buildable next to examples/flagship/, which OwnAudit cannot provide without cross-repo CI. Whether the consolidation endpoint is "witness moves to OwnAudit" or "audit development consolidates here" is an owner decision the AGENTS.md note itself defers ("currently lives") — flagging it for that decision rather than relocating unilaterally.


Generated by Claude Code

Console.Error.WriteLine(
" Re-run with --debug (or OWEN_DEBUG=1) for the full traceback, and " +
"please report it: https://github.com/PhysShell/Own.NET/issues/new/choose");
return CrashReport.ExitCode;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Write the promised diagnostic report for core crashes

When the Python core catches an internal exception and exits 70, this branch returns exit 5 directly without invoking CrashReport.Child or otherwise writing ~/.owen/diag/last-failure.json. Thus the most important internal-error path contradicts the new help text and failure contract, and users may be directed to submit a report that is absent or stale from an earlier failure.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 2f064dd: the rc-70 branch now goes through CrashReport.Child("analysis core", …), so the diagnostic report at ~/.owen/diag/last-failure.json is written on the most important internal-error path too. The gate-A crash smoke additionally asserts the report is mentioned.


Generated by Claude Code

claude added 2 commits July 26, 2026 16:26
…gnature, truncation root kind, core-crash report

Four verified findings from the #309 review, all real:

1. Sampling bias (P1): taking the first `sample` instances in
   heap-enumeration order could catch only old garbage and miss a later
   durably-held instance — a false OBSERVED_ONLY. ALL matching instances
   are targeted now: the verdict is exact over the whole population, and
   only PATH RESOLUTION is sampled (bounded at `sample` paths).

2. Group signature (P1): grouping by hop type names merged stack-rooted
   and durably-rooted instances sharing a type sequence, classifying the
   whole group by whichever path came first. The signature now carries
   the classification and the per-hop fields.

3. Truncation (P1): stopping the unwind at maxHops yielded
   ClrRootKind.None -> 'unsupported-root:None', which the verdict counts
   as durable — a long stack-only path read as RETAINED. The parent
   chain is now walked to the true root for the verdict; only the
   RENDERED path stays bounded.

4. Core-crash report (P2): the rc-70 branch returned exit 5 without
   writing ~/.owen/diag/last-failure.json, contradicting the published
   contract on the most important internal-error path. It now goes
   through CrashReport.Child (null capture — the core's own polite line
   is already on stderr), and the gate-A crash smoke asserts the report
   is mentioned.

The fifth finding (AGENTS.md: audit development lives in OwnAudit) is a
placement decision answered on the thread: the decoupling half of the
rule is honored (no ownlang coupling, public CLI/JSON only), the witness
core originates from PR #280 which placed it in this repo's audit/, and
the extraction here followed an explicit owner-side direction; whether
to relocate to OwnAudit stays an owner decision, not a drive-by move.

Verified locally: builds clean, selftest 11/11, flagship-demo bad
'leak DEMONSTRATED' / ok 'fix VERIFIED (OBSERVED_ONLY)'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
…o longer flip the verdict

Completing the Codex P1 sampling fix one level up. After 2f064dd the
REACHABILITY count was exact, but durable/transient classification still
came only from the resolved-path sample (first --sample reachable
instances in heap order). A type with 200+ finalizer-queue-reachable
corpses sitting ahead of one durably-held instance would still read as a
false OBSERVED_ONLY — the same bias, moved from discovery to
classification.

Now FindRetainers runs a root-kind census over EVERY reachable instance
(parent-chain walks only — dictionary hops, no type/field resolution),
and the verdict consults the exact DurableRetained count. Path
resolution stays bounded by --sample, resolving durably-rooted
instances first, so RETAINED never ships without a durable root path on
display. --sample/--max-hops are clamped to >= 1: a pathological
`--sample 0` must not suppress the path a RETAINED verdict promises.

Display honesty fixed with it: rendered shares now divide by the
resolved-path count, not the full population (200 resolved paths out of
1,000 reachable used to render a 100%-dominant retainer as 20%), and
the human output states all three numbers separately — on heap,
reachable, durably retained, paths resolved.

The invariant is pinned in code at the API boundary: sampling and hop
limits affect evidence PRESENTATION, never discovery, classification,
aggregation, or the exit code. Selftest grows 11 -> 16 checks: the
ClrMD-level transient split (BFS seeding + census) and the string-level
verdict rule must classify every root kind consistently, unknown kinds
included.

Verified locally: build clean, selftest 16/16, flagship-demo bad
(RETAINED via static-event) and ok (OBSERVED_ONLY) both pass; manual
attach with --sample 50 against 1,000 leaked views reports "1,000
durably retained (50 paths resolved for display)".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
@github-actions

Copy link
Copy Markdown

@coderabbitai review

@PhysShell
PhysShell merged commit 1f08a72 into main Jul 26, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants