mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
docs: clarify ownership and cleanup review guidance
Clarify structural parsing, admitted facts and freshness, useful facade and compatibility responsibilities, immutable JSON proof, and partial Evidence retention. Document fixture cleanup ownership, semantic fault injection and the limits of static checks; extend the existing PR owner hint. Correct the session-record architecture label. Validation: docs:check (including nine README consistency tests), rendered docs:build with link validation, touched Markdown formatting and git diff --check passed. Source and installed Vitest cleanup behavior were reviewed. No runtime behavior changed.
This commit is contained in:
1 parent
efab65c2f9
commit
9140deae63
5 files changed
+98
-17
No files matched your search
@@ -36,6 +36,9 @@ unchecked. Maintainers can help resolve open questions.
|
||||
Use "none" or "not applicable" only when established. -->
|
||||
|
||||
- Semantic owner and earliest changed stage:
|
||||
<!-- Name the producer of parsed facts or resources and its affected consumers.
|
||||
Summarize superseded paths removed and any distinct policy, lifecycle,
|
||||
lazy-loading, or compatibility responsibility retained. -->
|
||||
- Contribution scope fit and linked maintainer decision:
|
||||
- CLI and MCP/tool-catalog contract:
|
||||
- Provider, bridge, target-format, or platform compatibility:
|
||||
|
||||
+22
-2
@@ -25,6 +25,18 @@ acquiring targets; optional adapter failures must preserve successful peers. Run
|
||||
`npm run verify:test-discovery` after adding or moving tests. Resolved import
|
||||
boundaries are checked by `npm run verify:module-boundaries`, included in lint.
|
||||
|
||||
Before retiring an alias or facade, trace runtime registration, dynamic imports,
|
||||
JavaScript verifiers, subprocess scripts, and packaged entrypoints as well as
|
||||
TypeScript references. Tests may be the last consumers of a superseded internal
|
||||
path; migrate their distinct assertions to the current production path before
|
||||
removing it. Preserve validation, lazy loading, caller policy, and resource
|
||||
acquisition and cleanup supplied by the old boundary. Published APIs, persisted
|
||||
formats, and independently versioned bridges require an explicit compatibility
|
||||
decision; an internal representation alone does not establish that obligation.
|
||||
See [interpreting boundary facts once](docs/tool-design.md#interpret-boundary-facts-once)
|
||||
for admission ownership and the distinction between repeated validation and
|
||||
independent checks.
|
||||
|
||||
For analysis workflows and agent interfaces, follow the
|
||||
[performance priorities](AGENTS.md#performance). Explain the selected output
|
||||
scope, reuse/cache decisions, and relevant before/after measurements when these
|
||||
@@ -372,9 +384,17 @@ Real-provider execution is uncached.
|
||||
### Temporary files and diagnostics
|
||||
|
||||
Tests use `createTestTempDirectory` from `tests/fixtures/temporaryDirectory.ts`
|
||||
for exact-path, awaited cleanup, including failures and timeouts.
|
||||
for exact-path, awaited cleanup, including failures and timeouts. It registers
|
||||
deletion with Vitest's `onTestFinished`; do not also add its result to a roots
|
||||
array or an `afterEach` removal. `createTestWorkspace` from
|
||||
`tests/support/workspace/workspaceFixture.ts` leaves cleanup with its caller,
|
||||
which must await `removeTestWorkspace`. Keep explicit resource teardown before
|
||||
directory deletion, using the case's `finally` or `afterEach`; Vitest runs
|
||||
`afterEach` before `onTestFinished` and awaits finish hooks in reverse
|
||||
registration order.
|
||||
`npm run verify:test-temp-hygiene` runs the suite under a fresh `TMPDIR` and
|
||||
rejects remaining REA-owned paths. Never glob-clean shared `/tmp/rea-*` content.
|
||||
rejects remaining REA-owned paths. It cannot detect duplicate deletion owners or
|
||||
prove resource-close ordering. Never glob-clean shared `/tmp/rea-*` content.
|
||||
|
||||
`REA_LOG_LEVEL` selects `trace`, `debug`, `info`, `warn`, `error`, `fatal`, or
|
||||
`silent`. Logs go to stderr: MCP defaults to `info`, and one-shot CLI logging
|
||||
|
||||
@@ -23,7 +23,7 @@ graph TD
|
||||
SESSION["SessionProviderRouter + BinarySession<br/>one immutable deep binding per target<br/>disjoint auxiliary routing"]
|
||||
REGISTRY["AnalysisProviderRegistry<br/>sorted deep candidates<br/>deterministic selection, no fallback"]
|
||||
RECORDS["src/application/investigation/<br/>composed InvestigationRecords + narrow ports<br/>EvidenceLedger and Unknown ownership"]
|
||||
SNAPSHOT["BinarySessionRecords + AnalysisSnapshotCache<br/>binary target/profile snapshots<br/>compatibility facade and post-commit notifications"]
|
||||
SNAPSHOT["BinarySessionRecords + AnalysisSnapshotCache<br/>binary target/profile snapshots and session file ownership<br/>record coordination and post-commit notifications"]
|
||||
JS_RECON["src/application/javascript/ + src/artifacts/javascript/<br/>JavaScript artifact reconstruction<br/>directory/ASAR reads<br/>AST-only bundle projection"]
|
||||
end
|
||||
|
||||
@@ -75,7 +75,7 @@ graph TD
|
||||
end
|
||||
|
||||
subgraph "Domain Layer"
|
||||
DOMAIN["src/domain/<br/>Pure side-effect-free modules<br/>evidence, comparisons, application graphs"]
|
||||
DOMAIN["src/domain/<br/>Pure side-effect-free modules<br/>format declarations, Evidence, comparisons, application graphs"]
|
||||
end
|
||||
|
||||
subgraph "Configuration"
|
||||
|
||||
+40
-10
@@ -140,6 +140,12 @@ static tasks. Both lint caches include the website, documentation configuration,
|
||||
and experiment code scanned by the checker; a regression compares the actual
|
||||
lint inventory with Turbo's resolved inputs.
|
||||
|
||||
Static checks locate candidates; they do not establish semantic ownership.
|
||||
`jscpd` finds token clones, and Knip counts test references as uses. Module
|
||||
boundaries and architecture guards enforce their declared import and source
|
||||
rules. Passing them does not prove that two parsers agree, a wrapper is still
|
||||
needed at runtime, or cleanup has one owner.
|
||||
|
||||
Routine iterations and rebases need focused regressions and relevant checks.
|
||||
Before handing off a PR, run `npm run check` and generated-document checks when
|
||||
applicable; CI owns the full suite and coverage. Use the full local gate for
|
||||
@@ -169,6 +175,14 @@ with the consumer preloaded as well as alone. For simulated protocol traces,
|
||||
use an acknowledged event barrier before asserting complete observations;
|
||||
a zero-duration capture does not guarantee queued events have arrived.
|
||||
|
||||
Place faults at the semantic boundary under test: the selected read, EOF, hash
|
||||
completion, or cleanup attempt. Global call ordinals can shift during setup or
|
||||
refactoring without reaching that phase. For controlled asynchronous gates, use
|
||||
`awaitAcknowledgement` from `tests/fixtures/lifecycleAcknowledgement.ts` to race
|
||||
the acknowledgement against triggering-operation settlement and a bounded
|
||||
deadline. Register release gates before launch, then release and join launched
|
||||
work in `finally` before restoring mocks, even when the barrier fails.
|
||||
|
||||
Focused contract tests should import the named contract family they exercise;
|
||||
reserve the full tool catalog for catalog-wide assertions. Resolve contracts by
|
||||
operation name rather than array position.
|
||||
@@ -312,6 +326,13 @@ Trivial helper assertions need no replacement. Remove abandoned production
|
||||
scaffolding only after checking runtime and verifier consumers, including
|
||||
compiled imports.
|
||||
|
||||
After an owner changes, move fault injection and event spies to the path the
|
||||
production consumer actually uses, and assert that the intended boundary was
|
||||
reached. Keep independent expectations from authored input, producer output, or
|
||||
an external oracle; comparing two paths through the same replacement algorithm
|
||||
does not preserve the old oracle. A negative probe inside a conditional or loop
|
||||
must assert that it ran.
|
||||
|
||||
Use representative scenarios rather than Cartesian matrices of equivalent
|
||||
cases. Share setup without hiding inputs or expected evidence. For successful
|
||||
`Result` values, throw the typed error before asserting the value; a preceding
|
||||
@@ -358,6 +379,10 @@ the whole contract.
|
||||
| Snapshots, caches, and comparison | Vary metadata that affects the result while keeping payload bytes equal. Exercise historical records with newly introduced fields omitted, current caller inputs on replay, and reuse across differing selections. Assert correct invalidation or unknown coverage, current request provenance, and unchanged saved Evidence and IDs. |
|
||||
| Public success and error delivery | Exercise affected public CLI/MCP paths, including partial Evidence on failure. Assert retained facts and named contracts; retrieve partial Evidence when MCP returns a reference. If SDK cancellation suppresses the reply, check server settlement and any retained Evidence. An error-code assertion alone misses lost observations. |
|
||||
|
||||
For partial Evidence delivery, verify that same-session readback preserves
|
||||
identity and provenance without rerunning the provider. Include a no-writer
|
||||
control that keeps completed Evidence inline and preserves the original failure.
|
||||
|
||||
When producer order is unspecified, synchronize fixtures on the observed request
|
||||
or completion event. A filename, generated key, or fixed delay does not prove
|
||||
which item completed before cancellation. Preserve real producer ordering and
|
||||
@@ -412,15 +437,16 @@ the affected owners and applicable scenarios below. Include temporary snapshots
|
||||
and one-shot commands. Use public CLI/MCP workflows where practical and a
|
||||
controlled production seam for interleavings that cannot be reproduced reliably.
|
||||
|
||||
| Scenario | Assertion and control |
|
||||
| --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
|
||||
| Target admitted; startup still pending | Status preserves the readiness distinction and observed startup activity; readiness requires the provider's documented proof. |
|
||||
| First handshake is late, absent, invalid, or recovered during close | Initial and recovery paths retain the same authenticated identity facts. Foreign run/token controls cannot authorize cleanup; authenticated idle/native work survives the unauthenticated listener deadline. |
|
||||
| Launch returns an error with a valid process handle | Cover already-exited and still-live owned processes. Verify observed exit separately from creation; failed termination or unavailable identity remains unresolved. |
|
||||
| Cancellation arrives after workload exit, during result reads or temporary-root removal | Return typed cancellation with available partial observations and actual cleanup status. The non-aborted control succeeds; existing primary/cleanup failures keep their precedence. |
|
||||
| Children have mixed health or cleanup outcomes | Missing peer health does not erase an observed failure or activity. Idle plus unknown stays unknown. Retry a failed child without closing a successful peer again, including after a close-option change. |
|
||||
| Cleanup fails after success or primary failure | Public output retains the completed observation or primary diagnostic, remaining resources, and owner recovery action. Restore the denied resource and verify the owning cleanup retry. |
|
||||
| Target exits without an acknowledgement, or transport disappears while it stays alive | Positive owned exit proof resolves target lifetime; other resources still need cleanup. Lost transport alone does not prove exit. Verify unrelated/attached targets survive and same-connection recovery works when supported. |
|
||||
| Scenario | Assertion and control |
|
||||
| --------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| Target admitted; startup still pending | Status preserves the readiness distinction and observed startup activity; readiness requires the provider's documented proof. |
|
||||
| First handshake is late, absent, invalid, or recovered during close | Initial and recovery paths retain the same authenticated identity facts. Foreign run/token controls cannot authorize cleanup; authenticated idle/native work survives the unauthenticated listener deadline. |
|
||||
| Launch returns an error with a valid process handle | Cover already-exited and still-live owned processes. Verify observed exit separately from creation; failed termination or unavailable identity remains unresolved. |
|
||||
| Cancellation arrives after workload exit, during result reads or temporary-root removal | Return typed cancellation with available partial observations and actual cleanup status. The non-aborted control succeeds; existing primary/cleanup failures keep their precedence. |
|
||||
| Children have mixed health or cleanup outcomes | Missing peer health does not erase an observed failure or activity. Idle plus unknown stays unknown. Retry a failed child without closing a successful peer again, including after a close-option change. |
|
||||
| Cleanup fails after success or primary failure | Public output retains the completed observation or primary diagnostic, remaining resources, and owner recovery action. Restore the denied resource and verify the owning cleanup retry. |
|
||||
| Cleanup owners recover after an error was returned | Retain the original child and aggregate errors and project them after each owner releases. A successful peer stays released; the aggregate remains incomplete until all owners release. Preserve the first cause even when rejection is `undefined`. |
|
||||
| Target exits without an acknowledgement, or transport disappears while it stays alive | Positive owned exit proof resolves target lifetime; other resources still need cleanup. Lost transport alone does not prove exit. Verify unrelated/attached targets survive and same-connection recovery works when supported. |
|
||||
|
||||
Shared process-supervision controls separate leader exit from output closure.
|
||||
Cover output during held or failed cleanup, bounded retention and drain timeout,
|
||||
@@ -587,7 +613,11 @@ creation outside the workspace seam and its narrowly documented boundary/package
|
||||
exceptions.
|
||||
|
||||
The shared temporary-directory and workspace helpers preserve caller labels
|
||||
under an owned `rea-` prefix when needed. `npm run verify:test-temp-hygiene`
|
||||
under an owned `rea-` prefix when needed. Their
|
||||
[cleanup ownership differs](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#temporary-files-and-diagnostics):
|
||||
`createTestTempDirectory` registers `onTestFinished` deletion, while
|
||||
`createTestWorkspace` requires caller cleanup. Close resources before removing
|
||||
their directory. `npm run verify:test-temp-hygiene`
|
||||
uses the same ownership rule to find retained fixtures under a private temporary
|
||||
root. Third-party caches remain outside that ownership rule.
|
||||
|
||||
|
||||
+31
-3
@@ -88,6 +88,10 @@ satisfy. A plan without execution does not establish runtime behavior.
|
||||
The format or protocol reader owns interpretation. Pass its parsed facts and
|
||||
explicit uncertainty to consumers; do not make adapters reparse display text,
|
||||
reconstruct omitted values, or infer success from missing diagnostics.
|
||||
Client registration inspections, for example, carry command identity and
|
||||
activation independently of readiness. An owned registration may be disabled
|
||||
or stale; unresolved ownership remains unknown. Consumers use those facts
|
||||
instead of reclassifying the raw command.
|
||||
|
||||
A completed read can establish byte identity even when decoding or format
|
||||
validation fails. State which representation a digest identifies, such as file
|
||||
@@ -220,6 +224,12 @@ stages that never ran.
|
||||
applies rather than duplicate close/finally handling.
|
||||
- Give descriptor-backed reads an owner before opening the file. A rejected
|
||||
close must retain the exact handle and preserve any earlier failure.
|
||||
Carry [`openRegularFile`](https://github.com/morluto/rea/blob/main/src/filesystem/RegularFile.ts)'s
|
||||
admitted handle and initial metadata rather than restatting only to rebuild
|
||||
those facts. Keep fresh descriptor and selected-path identity checks after
|
||||
work, comparing against the original baseline instead of replacing it with
|
||||
a later stat. Preserve explicit bigint reads when ordinary admission metadata
|
||||
cannot supply the required size or timestamp precision.
|
||||
Stable file reads expose completed values separately from cancellation so
|
||||
consumers retain acquired identity facts after successful close or cleanup retry.
|
||||
If an abortable caller stops waiting, keep the underlying work in that owner's
|
||||
@@ -250,9 +260,13 @@ through the producing owner and affected CLI/MCP consumers.
|
||||
|
||||
## Implement and evaluate
|
||||
|
||||
Put provider protocols/parsing in adapters and shared CLI/MCP workflows in the
|
||||
application layer. Share Evidence provenance, unknown projection, and session
|
||||
retention semantics; matching payloads alone do not establish parity.
|
||||
Put provider wire-protocol parsing in adapters and shared CLI/MCP workflows in
|
||||
the application layer. Reusable format declarations may live in pure domain
|
||||
owners: [`peOptionalHeader.ts`](https://github.com/morluto/rea/blob/main/src/domain/peOptionalHeader.ts)
|
||||
decodes PE structure, while consumers retain machine admission, directory-address
|
||||
interpretation, and file/mapping bounds. Share Evidence provenance, unknown
|
||||
projection, and session retention semantics; matching payloads alone do not
|
||||
establish parity.
|
||||
|
||||
Let `createEvidence` own JSON validation and snapshotting of ordinary observation
|
||||
payloads. `EvidenceObservationInput` accepts semantically parsed parameters,
|
||||
@@ -264,6 +278,20 @@ Evidence envelope.
|
||||
`EvidenceObservation` retains the stronger JSON contract used by producer
|
||||
helpers and `createImmutableEvidenceSteps`. The cooperative constructor also
|
||||
requires an authenticated immutable result; preserve that ownership proof.
|
||||
For objects, proof comes from completed sealing by
|
||||
[`immutableJson.ts`](https://github.com/morluto/rea/blob/main/src/domain/immutableJson.ts);
|
||||
`readonly` or `Object.freeze` alone does not establish it. JSON primitives are
|
||||
directly eligible. Seal owned data and complete cooperative sealing before
|
||||
reusing its result.
|
||||
|
||||
When a failed or cancelled operation retains completed, valid partial Evidence,
|
||||
use the shared [`ToolResultDelivery`](https://github.com/morluto/rea/blob/main/src/server/ToolResultDelivery.ts)
|
||||
path with the session's Evidence writer. Keep the original failure and the
|
||||
named contract's target-content marking; replace the inline partial with a
|
||||
retained reference only after recording succeeds. An absent writer or a writer
|
||||
returning a failure `Result` leaves the partial inline, as does a partial that
|
||||
is not valid Evidence. Unexpected recorder exceptions propagate. An `evidence_id`
|
||||
member alone does not establish Evidence validity or retention.
|
||||
|
||||
Measure the resource before choosing a budget and enforce it before the costly
|
||||
step. Account for value alternatives, trace-frame products, retained records,
|
||||
|
||||
Reference in new issue
Block a user