Repository navigation
skills: refine contracts and ADR evidence - #22
Conversation
Fold net-new and corrective lessons into the existing skill owners after the earlier Rust and proof-planning integrations. Distinguish acceptance from effects, cleanup from native reclamation, constructor behavior from polling, and decision history from delivery evidence. Amend overly absolute advice and replace duplicate invariant, upstream-fix and notification prose with owner links. Preserve applicability and exceptions; keep the project evidence in the accompanying PR rationale. Bump matching package and plugin versions to 0.5.0 for the broad update.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 719e8af703
ℹ️ 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".
| Before filing a finding, compare existing issue ownership and the exact sections | ||
| changed by open pull requests. Touching the same document does not prove the | ||
| contradiction is fixed. Pin the inspected revision and distinguish inspected | ||
| code, reported checks, checks you ran and unknown coverage, following | ||
| [method](../../method/SKILL.md#2-verifying). |
There was a problem hiding this comment.
Keep finding verification with its existing owners
This paragraph introduces substantive rules for filing review findings and classifying evidence, even though the owner table in skills/engineering/README.md:68-69 assigns review standards to pull-requests and verification/evidence classes to method. Keeping those rules here gives the lessons a second owner; move them to the assigned owners and leave only an ADR-specific link or application sentence.
AGENTS.md reference: AGENTS.md:L32-L33
Useful? React with 👍 / 👎.
| documented guarantee proves no outside user remains. | ||
| - **Returning a resource includes notifying live waiters owed that change.** | ||
| Name who can still wait, what lets them proceed, and what makes them runnable. | ||
| A peer known to be closed needs no wake. If liveness cannot be decided |
There was a problem hiding this comment.
Require proof before skipping a closed peer's wake
Knowing that a peer is closed does not by itself prove that no live waiter needs notification: for example, a send or receive future may already be pending and require a wake to observe the terminal closed result. As written, this rule can legitimize a lost wake and permanent sleep; qualify it with proof that closure already woke or cancelled every waiter, or that no waiter can exist.
AGENTS.md reference: AGENTS.md:L47-L50
Useful? React with 👍 / 👎.
## Motivation The recovered October9–12 ADR audits are the remaining source evidence deferred by [#23](#23). Most of their lessons already live in the ADR reconciliation procedure added by [#22](#22): decision acceptance is separate from delivery, slices need their own evidence, inventories need lifecycle placement, and stale claims must be repaired wherever readers encounter them. One useful distinction can be clearer: a feature name is not proof that the shipped design satisfies the decided contract. A different mechanism may preserve the guarantee, leave a real implementation gap, or require an explicit decision revision. This draft makes that comparison explicit without prescribing the historical proposal as the correct design. ## Solution 1. `8a72865`: amend the existing ADR scope-reconciliation paragraph to compare promised guarantees and distinguish an implementation gap from a decision needing explicit revision. 2. `8a72865`: add one Core Rule2 example that links to that owner; keep all historical project detail here. Only two existing documentation files change. This is a narrow clarification, so package/plugin versions remain `0.6.0`; the repository requires bumps for broad changes. No new skill, duplicate audit checklist, workflow, implementation, ADR file or issue is introduced. **Leave this PR as a draft; do not merge without separate approval.** ### Baseline and source access - Based on the actual default `wip` at `572b154685a75892b54d939e1d2d9b0e6fb22477`, the merged #23 tree. Root `AGENTS.md`, the engineering ownership table, ADR, architecture, method and review owners were inspected. No additional applicable `.agents` instructions were present. - No competing open skills PR was found before editing. Earlier skills issues are older Rust research, not a new ADR integration. - Input is the recovered historical report-message evidence supplied with this task. Full thread text was not accessible in this executor; ZIP issue-body attachments remain unavailable. Neither original issue drafts nor publication receipts are claimed inspected. - Read-only public source checks inspected relevant PR metadata and excerpts of the ADR index, runtime ADR, Read-grants design and physical-persistence design at the October12 snapshot. These corroborate the bounded mechanism comparison; they are not a fresh audit of current Nessa. ### Source-to-owner dispositions | Recovered finding family | Disposition | Existing owner / reason | | --- | --- | --- | | 501 linked once with unique identity but absent from lifecycle table | Already covered | ADR inventory structure requires repository-consistent placement and says a stray link is not index coverage. Uniqueness and membership are separate checks already expressed. | | 252 acceptance versus implementation checklist and tracking state | Already covered | ADR scope reconciliation separates accepted decisions, shipped slices, evidence and remaining gates; reader-surface repair includes tracking issues. | | Partial replay, grants, artifacts, pairing and outbound delivery | Already covered | Per-slice evidence and gates; one merged implementation does not establish full scope. | | A proposed Cedar/semantic-record design versus the shipped metadata grant table and durable journal | **Small amendment** | ADR scope reconciliation now requires matching the promised guarantee instead of the feature name and distinguishing implementation gaps from explicit decision revisions. It does not decide which design is right. | | Stale 0008/0009/202/302/392 prose, index/body conflicts and 688/689 status | Already covered | ADR contradiction repair covers header, current prose, tables, checklists, index, README and tracking issue; placement follows the repository convention. | | Persistence mechanics and 27 ordering cases without decision rationale/rejected alternatives | Already covered | ADR template requires the binding constraint, alternatives and reasons; method's decision artifact also includes rejected options. Implementation instructions and reported tests are not newly relabeled as decision history. | | PR715 already addressed replay documentation; avoid filing it again | Already covered | ADR's existing issue/open-PR comparison checks the exact changed sections, not document overlap. No Nessa issue action is taken. | | Repeat-audit baseline/delta and outcome separation | Coverage practice; no additional skill rule | Pin historical revisions and report new/still-open/reconciled findings in this ledger under existing evidence-class rules. A publication failure is not proof inspection never completed. | | Reported publication failures and disabled ADR automation | Reported operational outcome only | No independent automation-state verification, schedule change or publication retry. Rust research state is outside this patch. | ### Recovered-report coverage ledger These are historical findings at the stated revisions, not claims that the defects remain in current Nessa. Repeated themes are counted once as transferable guidance. | Report message | Historical Nessa snapshot | Recovered coverage and disposition | | --- | --- | --- | | October9 · `2a086b87-e13b-43af-a099-4a2f577bcd82` | `4d1b7278cfe633a6c38094f0bcfb45d4d5043af0` | Reported 36 records:23 done/13 todo;326/344 moved done,252 accepted with implementation separately tracked. 0008,202/302,392,501 and persistence-rationale findings map to existing owners; no duplicate replay-doc work. | | October10 · `2d3f1bcd-f54a-4d96-b836-3caa1a19e0b9` | `88f34e4dabdeca46486dfbd5294e0a2a00a0a5fa` |501 membership,715 stale replay index and720 unchecked grants slice are already covered. The proposed-versus-shipped semantic-contract distinction supports the sole amendment; peer738 and persistence rationale remain historical case evidence. | | October11 · `e4ad69af-9798-4d6f-a666-3a3dbb8aad70` | `bae7a073b597285823b92f037d7079cf4522a716` | Reported 38 records:23 done/13 todo/2 root;688/689 still under review after739,744 pairing shipped while748 outbound was not yet merged at this snapshot. Existing lifecycle/slice/revision rules cover these distinctions. | | October12 · `a452f573-d4e0-4663-a720-3970e6313f15` | `07799e6405812a52297c02bac39525ec0315568e` | Reported37 further commits and38 records; grant/artifact/lease work advanced while checklists lagged;392,501 and rationale gaps persisted. Already-covered themes reinforce the narrow clarification. Reported ADR automation disabling is separate from audit completion and Rust research. | Provenance: recovered messages belong to conversation `6aba0796-656c-83e8-a735-49b9ed36b026`. The identifiers preserve source attribution without putting private conversation links into reusable skills. **Unavailable attachments:** issue-body ZIP attachments were not read, so exact unpublished drafts, their complete acceptance language and publication outcomes were not independently reconstructed. **October13–16:** no report messages were found by the recovery; these dates are not counted as inaccessible completed reports. There is no basis here to invent additional audit runs or claim a current Nessa audit. ### Public supporting evidence - At the [October12 runtime ADR, sliceG](https://github.com/nessalabs/nessa-agent/blob/07799e6405812a52297c02bac39525ec0315568e/docs/adr/todo/252-runtime-roles-and-execution-leases.md#implementation-plan), the wording names a Cedar grant. The [same-revision Read-grants design](https://github.com/nessalabs/nessa-agent/blob/07799e6405812a52297c02bac39525ec0315568e/docs/design/read-grants.md#what-this-slice-does-not-do) explicitly distinguishes its metadata table/journal from in-stream semantic records and policy-bundle changes. [#720](nessalabs/nessa-agent#720) is the delivered slice. This is evidence to compare contracts, not proof that every mechanism difference is a defect or that a chosen implementation must be reverted. - The [pinned ADR index](https://github.com/nessalabs/nessa-agent/blob/07799e6405812a52297c02bac39525ec0315568e/docs/adr/README.md) and [#715](nessalabs/nessa-agent#715), [#739](nessalabs/nessa-agent#739) support the historical projection/lifecycle examples; those rules already exist. - [#744](nessalabs/nessa-agent#744) merged at the October11 report's exact snapshot,15:43UTC. [#748](nessalabs/nessa-agent#748) merged later at16:37UTC. “Not yet merged” is a snapshot observation, not a current outbound-delivery defect. - [#652](nessalabs/nessa-agent#652) and its [pinned physical-persistence design](https://github.com/nessalabs/nessa-agent/blob/07799e6405812a52297c02bac39525ec0315568e/docs/design/bounded-physical-persistence.md) support the distinction between implementation mechanics/reported tests and a decision's rationale. This draft makes no new current missing-ADR claim. ### Validation and limits - **Run:** repository-wide local Markdown path and heading-anchor validation:182 links pass (fenced template examples excluded). - **Run:** all six declared skill paths, required frontmatter and agent descriptors checked; package/plugin versions both remain `0.6.0`. - **Run:** `git diff --check`; refreshed `origin/wip` remains at the stated base before publication. - **Run:** author diff review and independent source-to-owner triage, followed by independent review of the final diff and this description. No material blockers, duplicate owner or evidence/coverage overclaims found. - **Read-only source checks:** the public PRs and pinned documents linked above resolved; selected historical excerpts were inspected. This does not establish runtime behavior or an exhaustive current audit. - **Unavailable:** original full thread text in this executor and ZIP issue-body attachments. The supplied recovered-message evidence is distinguished from source excerpts inspected here. - **Not run:** runtime/native tests, historical ordering cases, benchmarks, installed-skill behavioral evaluation or production defect reproduction. No CI workflows or standard checked-in skill validator exist in this repository; the documentation checks above were run locally. No `nessa-agent` files, issues, PRs or schedules were changed; the separate #953 task was untouched. Prior denied issue publications were not retried. No native/runtime tests, ordering cases or historical audit checks were rerun. The change teaches a review distinction; it does not assert a runtime-validated result or repair historical Nessa defects.
Motivation
The existing skills already incorporate the earlier Rust study corpus in
#20 and proof planning in
#21. Subsequent source inspections
identified a smaller set of missing distinctions and overly absolute rules:
custom future construction can act before polling, native cleanup can fail
without ending external use, and acceptance or a merged implementation slice
does not establish a complete delivered guarantee.
This change adapts those lessons to their existing owners. It preserves the
earlier guidance, fixes contradictions in place, and keeps project evidence in
this description.
Solution
719e8af: amend coding's existing input, identity, cleanup, bounds and shutdown rules;allow configuration justified by a caller contract or a demonstrated policy
need; retain accepted retry work through temporary prerequisite absence.
719e8af: correct Rust future construction and extend existing ownership,allocation, code-generation, overflow, FFI and build-compatibility guidance.
Keep only the Rust-specific forced-poll test under the general notification
owner's link.
719e8af: extend the existing patterns reference for mutation witnesses, compatibilityfallback guarantees and independent stream progress. Keep the upstream-fix
detail there and replace the duplicate PR convention with a link.
719e8af: add the ADR reconciliation procedure to the existing ADR reference: distinguishdecision history, delivered slices, evidence and remaining gates; reconcile
all current-state projections and structural inventories. Link it from the
architecture skill and list its ownership in the engineering README.
719e8af: add core-rule examples pointing to the detail owners, check feature-partitionedsuite discovery, and bump matching package/plugin versions to
0.5.0.Source-to-owner mapping: accepted amendments
Each source below supports the stated mechanism. Merged fixes are evidence for
the lesson, not claims that the defect still affects today's upstream code.
tokio-1.51.x; it does not establish default-branch inclusion or require retaining state when the foreign API proves no reference remains.tokio-1.51.x.Already covered; deliberately not added again
bounded admission, callback lock/reentry safety, exactly-once cleanup,
instrumentation cache keys, generated-code testing, process-global test scope,
published-crate MSRV, real promise boundaries and measured performance limits.
valid negative fixtures and permitted counterparts. No second proof checklist.
patterns. Tokio #8165 reinforces
them; it does not justify another accelerator rule.
contract, parser and conservative-summary owners. Polars mapping identity,
post-rewrite caches, coordinate space, phase absence, batch budgets and explicit
reorderability fit existing identity, state, bounds and optimization owners.
detection and foreign referent lifetime fit existing contract/lifetime owners.
Cache/PGO provenance and surprising performance outcomes remain evidence under
the existing performance method, not new checklists or reported speedups.
draft and final sample disagree. No duplicate discovery claims or uniform
sample claims are made, and alternate-only Zed cases are not used for new text.
Omitted narrower proposals
No permanent project references or 32-card review guide. Persisted shortcut
metadata formats, approximate mergeable sketches (Polars #29660), async-to-sync preparation
handoffs, compatibility no-op tombstones and calendar recurrence recipes remain
case-specific candidates unless a distinct recurring contract warrants them.
The existing general owners already cover much of their mechanism. In
particular, approximate merge behavior must use its actual tolerance and order
contract, not an invented bitwise-equivalence promise.
No stale ADR backlog is imported from before the reconciliations in #356/#357.
No Nessa source or ADR is changed by this skills patch. Reconciliation findings
still require their exact current contradiction and bounded acceptance checks.
Validation and limits
Baseline:
wipat7bb0666936c68fd7eb4662972be164ef52ff55b2, independentlyrechecked against the remote on October 9, 2026. Documentation-only change;
no runtime implementation, dependency, installed-skill or CI changes.
On the connected Mac:
quick_validate.py: all six skills pass.git diff --checkpasses.applicability and precision. The independent review found two P2 issues:
architecture's constructor-only invariant rule contradicted the mutation
refinement, and ADR inventory language conflated decision acceptance with
delivery placement. Both were corrected at their owners. The reviewer re-read
the complete revised diff, confirmed both resolved, and found no further
actionable issues; its independent validation also passed.
Source:
719e8af7032eb2c3d5bd6c1de051d7563a9b0094, based on the baseline above.Ten files, 187 insertions and 43 deletions. This is one cohesive local commit;
the evidence mapping is kept in this description instead of the loaded skills.
The source-study experiments and upstream Rust/native/platform/performance suites
were not rerun. Source inspection and documentation validation do not establish
runtime guarantees or measured improvement in defect detection.