Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions IMPLEMENTATION_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,11 @@ Done when:
and `ShellApprovalMatcher`; any retained legacy scan is deny-only and cannot
authorize, create candidates, or widen scope.
The preliminary complete-footprint audit after PR #1947 found 1,484 added
production lines and 52 added control-flow lines. The simplification remains
incomplete until the complete footprint is below its frozen baseline.
production lines and 52 added control-flow lines. Later slices reduced the
post-corpus footprint from 10,085 lines and 663 control-flow lines to 9,693
and 634. Control-flow is now below the frozen 635-line baseline. The final
audit must justify the remaining 721-line delta through tested security or
compatibility obligations instead of deleting required distinctions.
- [x] Shell calls pass through one coordinator. It snapshots immutable parser
and run-scope facts, requests one typed actor batch, and composes grant and
reviewed-safe coverage per candidate before one final result.
Expand Down
21 changes: 17 additions & 4 deletions openspec/changes/simplify-shell-policy-evaluator/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ The exact D-case fixtures, 12 adversarial cases, 11 live cases, and the full pol
- Keep one call-local state for candidates, coverage, evidence, and trace facts.
- Validate actor output once before any grant coverage applies.
- Compute parser-derived path facts once and reuse them across policy phases.
- Reduce total production lines and control-flow lines below the measured baseline.
- Reduce control-flow below the frozen baseline and both measures below the post-corpus implementation.
- Justify every residual layer above the frozen line baseline with a tested security distinction or compatibility obligation.
- Preserve all public, wire, persistence, prompt, trace, and operator contracts.
- Keep every unknown or invalid internal state fail-closed.

Expand Down Expand Up @@ -296,7 +297,7 @@ This change will not remove the public compatibility interface. A later generic

### 9. Complexity reduction is an acceptance gate

The final change must reduce aggregate lines and control-flow lines below 5,136 and 373. The task report will include both counts.
The original seven files must remain below 5,136 lines and 373 control-flow lines. The task report will include both counts.

That original-file measure does not count code that moves into a new file. The final audit must also count the complete production footprint.

Expand All @@ -305,11 +306,23 @@ The complete footprint uses corpus commit `8b4108aa92a229f4727377299d9dd2ed19f70
- `src/Netclaw.Actors/Tools/`
- `src/Netclaw.Security/`

An added file has zero baseline lines. A removed file has zero final lines. The complete footprint must have fewer final lines and control-flow lines.
An added file has zero baseline lines. A removed file has zero final lines.

The preliminary audit after PR #1947 found a failed reduction gate. The complete footprint changed from 6,680 to 8,164 lines.

It also changed from 452 to 504 control-flow lines. The next slices must remove this displacement before the final audit can pass.
It also changed from 452 to 504 control-flow lines. At that checkpoint, further reduction was required before the final audit.

The merged corpus implementation at `d2186d83e0ce2fe0d51ac67ea029eefa579abca3` is the reduction checkpoint. The same complete footprint used 10,085 lines and 663 control-flow lines there.

The final footprint must use fewer lines and control-flow lines than that checkpoint. Its control-flow count must also stay below the frozen 635-line control-flow baseline.

The final report must state any remaining line delta above the frozen 8,972-line baseline. Each residual production layer must map to one of these obligations:

- a tested security distinction that cannot be flattened;
- an exact behavior, trace, or authority contract;
- a public compatibility obligation with a recorded removal path.

No obsolete hierarchy, duplicate authority route, or file move can satisfy this gate. If further deletion would erase a tested distinction or break compatibility, the report must identify that boundary instead of compressing code to meet a line target.

The report will also include method complexity, line coverage, branch coverage, and CRAP risk. It will state the exact tool version and command.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,28 +119,172 @@ The complete changed footprint now uses 9,693 lines and 634 control-flow
lines. The frozen baseline uses 8,972 lines and 635 control-flow lines. The
control-flow gate now passes. The line gate remains open by 721 lines.

## Safe reduction boundary

The merged corpus implementation at `d2186d83e0ce2fe0d51ac67ea029eefa579abca3`
used 10,085 lines and 663 control-flow lines across the same 20-file footprint.
The direct evaluator uses 392 fewer lines and 29 fewer control-flow lines.

The original seven files now use 4,754 lines and 322 control-flow lines. Their
frozen values were 5,136 and 373.

The complete control-flow count is one line below its frozen baseline. The
remaining 721-line delta is not hidden or declared removed. Four added files
own the residual responsibilities:

| File | Lines | Required distinction |
| --- | ---: | --- |
| `ShellApprovalEvidence.cs` | 387 | Snapshots and validates complete actor evidence before authority applies; includes the public compatibility adapter tracked by #1944. |
| `ShellPolicyEvaluation.cs` | 250 | Owns candidate coverage, exact one-time context, and atomic bounded trace changes. |
| `ShellPolicyPathFacts.cs` | 345 | Retains candidate origin, value domain, redirect mode, and distinct real, intent, and fallback bases. |
| `ShellPathRules.cs` | 242 | Applies declared POSIX or Windows path semantics without host-dependent authority. |

Existing production files shed 503 net lines while these four files added
1,224. The bounded audit found no further 721-line deletion that preserves
all tested security facts and the compatibility contract. The latter has a
separate versioned migration in issue #1944.

Owning regressions live in `ShellApprovalEvidenceTests`,
`ShellPolicyPathFactsTests`, and `ShellApprovalMatcherTests`. The fixture and
disposition matrices exercise the composed authority path.

The final gate therefore requires:

- the original-file counts to remain below their frozen values;
- the complete footprint to remain below the post-corpus implementation;
- complete-footprint control-flow to remain below its frozen value;
- each residual layer to retain an owning regression or compatibility removal path;
- no obsolete hierarchy, duplicate authority route, or hidden file move.

## Public and durable compatibility

`dotnet-inspect` 0.18.0 compared Release assemblies from frozen commit
`8b4108aa92a229f4727377299d9dd2ed19f70e07` with the direct evaluator. It
reported no public API changes in `Netclaw.Actors` or `Netclaw.Security`.

Both assembly builds completed with zero warnings and errors. The temporary
baseline worktree was removed after the comparison.

The approval serialization, recovery, store, and wire-codec files are unchanged
from the frozen commit. Their focused suites passed:

- actor serialization and approval rehydration: 58 of 58;
- approval store and entry wire codec: 99 of 99.

## Equivalence replay

The cumulative direct evaluator passed these focused behavior contracts:

- coordinator, D-case, adversarial, live, and disposition replay: 420 of 420;
- security evidence contract: 26 of 26;
- channel prompts, recovery, headless gates, and subagents: 279 of 279.

These runs supplement the full Actors and Security suites. They do not replace
the required native Linux, macOS, and Windows checks.

## Final delivery gates

PR #1965 merged as `bbecae3901fcc8c3eb6b02e2f6891b5ed46030a6`.
All required checks passed.

The final local Release validation passed:

- Security: 946 of 946;
- Actors: 3,396 of 3,396, with one expected Windows-only skip;
- coordinator, D-case, adversarial, live, and disposition replay: 420 of 420;
- security evidence: 26 of 26;
- channel, recovery, headless, and subagent contracts: 279 of 279;
- the full Actors suite retained reminder and webhook coverage;
- strict validation for both active OpenSpec changes;
- copyright headers, changed-file formatting, diff checks, and changed-file Slopwatch.

The PR validation matrix passed on Ubuntu, macOS, and Windows. Windows ran the
native PowerShell host tests. Linux and macOS native smoke jobs passed. The
Linux screenshot regression, Docker build, install smokes, CodeQL, and
auto-format checks also passed.

The final adversarial audit found no changed authority route, precedence
change, public API drift, durable-schema drift, or executable-private parser.
Every residual production layer has an owning regression or issue #1944.

## Production boundary audit

The refactor-only production diff adds no executable name. Comparing literal
command checks with the merged corpus found only the `s/` shell-token form; it
is syntax classification, not an executable branch. Existing shell-invoker
names retain their prior behavior.

Parser `SourceOccurrence` references remain inside call-local projection and
reviewed-safe evaluation. Projection clears them before building the actor
candidate snapshot. The actor still receives the canonical phrase and directory
facts required for exact, folder, and global grant matching.

Trace and persistence receive no raw command, argument, redirect, environment,
or secret value. The approval serialization and persistence surfaces are
unchanged from the frozen commit.

## Preliminary coverage and risk

The audit used `dotnet-coverage` 18.10.0 and `crap4dotnet` 0.1.1.

The actor suite passed 3,411 cases with one expected Windows-only skip. It reported 68.81% line coverage and 45.54% branch coverage.
The direct-evaluator actor suite passed 3,396 cases with one expected Windows-only skip. It reported 68.57% line coverage and 45.30% branch coverage.

The security suite passed 927 cases. It reported 62.04% line coverage and 51.99% branch coverage.
The security suite passed 946 cases. It reported 62.42% line coverage and 52.36% branch coverage.

The largest new risk values came from these methods:

| File | Method | Complexity | Coverage | CRAP |
| --- | --- | ---: | ---: | ---: |
| `ShellPolicyEvaluation.cs` | `RunAsync` | 12 | 0.00%* | 156.00 |
| `ShellPolicyCoordinator.cs` | `EvaluatePolicyAsync` | 27 | 0.00%* | 756.00 |
| `ShellPathRules.cs` | `TryGetWindowsDepth` | 12 | 0.00% | 156.00 |
| `ShellPolicyCoordinator.cs` | `EvaluateCoreAsync` | 9 | 0.00%* | 90.00 |
| `ShellPolicyPathFacts.cs` | `Resolve` | 8 | 0.00% | 72.00 |

`crap4dotnet` does not map async state-machine coverage to the source method. The zero values remain repeatable comparison data.
`crap4dotnet` does not map async state-machine coverage to the source method.
The zero values remain repeatable comparison data, not evidence that the
coordinator path lacks tests. The full actor suite exercises that path.

`ShellPolicyEvaluation` has no method above CRAP 9.0. Its highest-complexity
methods, `Complete` and `Cover`, have 75% and 100% mapped coverage.

## Commands

```bash
dotnet tool install dotnet-inspect --version 0.18.0 --tool-path "$TOOL_DIR"
"$TOOL_DIR/dotnet-inspect" diff --library \
BASELINE/Netclaw.Actors.dll..CURRENT/Netclaw.Actors.dll --oneline
"$TOOL_DIR/dotnet-inspect" diff --library \
BASELINE/Netclaw.Security.dll..CURRENT/Netclaw.Security.dll --oneline

dotnet test src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csproj \
-c Release --no-build --no-restore \
--filter 'FullyQualifiedName~SerializationRoundTripTests|FullyQualifiedName~ApprovalRehydrationTests'
dotnet test src/Netclaw.Configuration.Tests/Netclaw.Configuration.Tests.csproj \
-c Release --no-restore \
--filter 'FullyQualifiedName~ToolApprovalStoreTests|FullyQualifiedName~ApprovalEntryWireCodecTests'

baseline=8b4108aa92a229f4727377299d9dd2ed19f70e07
corpus=d2186d83e0ce2fe0d51ac67ea029eefa579abca3
mapfile -t files < <(
git diff --name-only "$baseline"..HEAD -- \
'src/Netclaw.Actors/Tools/*.cs' 'src/Netclaw.Security/*.cs'
)
for revision in "$baseline" "$corpus" HEAD; do
lines=0
control_flow=0
for file in "${files[@]}"; do
git cat-file -e "$revision:$file" 2>/dev/null || continue
source=$(git show "$revision:$file")
lines=$((lines + $(printf '%s\n' "$source" | wc -l)))
control_flow=$((control_flow + $(
printf '%s\n' "$source" \
| rg -c '^\s*(if|for|foreach|while|switch)\b' || true
)))
done
printf '%s files=%s lines=%s control=%s\n' \
"$revision" "${#files[@]}" "$lines" "$control_flow"
done

dotnet-coverage collect 'dotnet test src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csproj -c Release --no-build --no-restore' \
-f cobertura -o /tmp/netclaw-final-actors.cobertura.xml
dotnet-coverage collect 'dotnet test src/Netclaw.Security.Tests/Netclaw.Security.Tests.csproj -c Release --no-build --no-restore' \
Expand Down
3 changes: 2 additions & 1 deletion openspec/changes/simplify-shell-policy-evaluator/proposal.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## Why

PRD-002 and PRD-006 require shell authorization to remain fail-closed, explainable, and practical for routine agent work. The current policy now behaves correctly, but seven core files contain 5,136 lines and about 373 branch points.
PRD-002 and PRD-006 require fail-closed, explainable shell authorization. Before this refactor, seven core files contained 5,136 lines and about 373 branch points.

The live corpus now supplies a stable contract for a behavior-compatible refactor. This change reduces policy complexity before more exceptions make the evaluator harder to audit.

Expand All @@ -14,6 +14,7 @@ The live corpus now supplies a stable contract for a behavior-compatible refacto
- Preserve every current allow, prompt, deny, correction, trace, and grant outcome.
- Use the exact D-case, adversarial, and live regression fixtures as equivalence tests.
- Deliver the refactor as small dependency-ordered production slices.
- Report the frozen baseline, post-corpus peak, and safe final footprint without hiding new files.

In scope:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ The refactor SHALL preserve all current decisions, deny reasons, allow reasons,

### Requirement: Refactor reduces policy complexity

The completed change SHALL reduce aggregate production lines and control-flow lines below the frozen baseline. It SHALL report method complexity and coverage risk.
The completed change SHALL reduce original-file lines and control-flow below their frozen baselines. The complete footprint SHALL reduce lines and control-flow below the post-corpus implementation.

Complete-footprint control-flow SHALL also remain below its frozen baseline. The final evidence SHALL report method complexity, coverage risk, and any residual line delta above that baseline.

It SHALL not add a public API, durable schema, command parser, or duplicate policy scan.

Expand All @@ -187,10 +189,18 @@ It SHALL not add a public API, durable schema, command parser, or duplicate poli
- **WHEN** all refactor slices are complete
- **THEN** the task evidence SHALL report before and after production line and control-flow counts
- **AND** the after counts SHALL be lower than 5,136 lines and 373 control-flow lines
- **AND** the complete changed production footprint SHALL have fewer lines and control-flow lines than the frozen corpus commit
- **AND** the complete footprint SHALL be lower than 10,085 lines and 663 control-flow lines
- **AND** complete-footprint control-flow SHALL be lower than the frozen 635-line baseline
- **AND** an added production file SHALL contribute zero lines to the baseline count
- **AND** each residual production layer above the frozen line baseline SHALL map to a tested security distinction, exact behavior contract, or recorded compatibility obligation
- **AND** the evidence SHALL report method complexity, coverage, and CRAP risk with a versioned command

#### Scenario: Safe reduction boundary

- **WHEN** another deletion would erase a tested path, evidence, authority, trace, or compatibility distinction
- **THEN** the task evidence SHALL identify that distinction and its owning tests or follow-up
- **AND** the implementation SHALL NOT compress or merge the distinction only to satisfy a line target

#### Scenario: Public and durable compatibility

- **WHEN** the final API and persistence audits run
Expand Down
Loading
Loading