From 1c26ea7f4ec249003722477dd9ee9f17401aab2c Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Wed, 21 Oct 2026 14:19:19 +0000 Subject: [PATCH] docs(memory-bank): record the rc9 review, the known limits, and the lessons Decision 27 gets the review findings of rc9 and what became of each, with the known limits that the maintainer is told about. debugging-insights gets the lessons of the piped optional parameters (the binder skips a value that it cannot convert, and the price of an object parameter), of a differential oracle that covers only part of the input, of agents in parallel worktrees (one shared stash), and of the help generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- .memory-bank/debugging-insights.md | 51 +++++++++++++++++++ .../decisions/0027-open-items-before-5.0.0.md | 42 +++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/.memory-bank/debugging-insights.md b/.memory-bank/debugging-insights.md index 053c132..54cbbb3 100644 --- a/.memory-bank/debugging-insights.md +++ b/.memory-bank/debugging-insights.md @@ -128,3 +128,54 @@ fails without it (Decision 16). gives up after 60 seconds when its runner never started (about 6 seconds normally). Before you blame the load for a stall, repeat the call alone with the same arguments. + +## Optional enum parameters that take pipeline input by property name + +- The binder skips a piped property that it cannot convert to the type of an + OPTIONAL parameter and keeps the default: a typo for `Deny`, a blank CSV + cell, or a misspelled scope silently made an Allow entry or the default + scope (probe `p-unconvertible-optional-params.ps1` on rc8, both editions; a + named call fails loudly; one parameter set with optional parameters does not + help). `Get-FileHash2 -Algorithm` has the same flaw and stays a known limit. +- Decision 27, fix B: the five parameters of the four add and remove cmdlets + are `object`, converted in the cmdlet by `ParameterConversion`. All five + must be `object`: with `AppliesTo` alone, a row with `AppliesTo` and flags + lost its `AmbiguousParameterSet`. The price was found by the review, not by + the author's tests: a `$null`, DBNull, or blank value of the OTHER group + binds too (a CSV with columns of both groups is `AmbiguousParameterSet`); a + named `$null` binds, where rc8 refused it (`MyInvocation.BoundParameters` + shows it in `BeginProcessing`); a script block is no longer delay-bound; the + completion of the names has to be rebuilt (quotes, wildcards, declaration + order); help shows `Object`. Probe every form of a value before you write a + claim, and ask a reviewer for the negative space. +- PowerShell calls the getter of a by-property-name parameter once per + command, when a piped object first binds it (`p-getter-calls.ps1`). A test + that must visit it pipes an object that has the property. A property getter + that throws reads as `$null` without a trace: test it through reflection. + +## A differential oracle that covers part of the input + +- The matcher of `Get-ChildItem2 -Filter` was checked against Windows + PowerShell 5.1 over 55,987 patterns, on 8.3-compliant names only, so that the + short name is the real name. Such names have at most one dot, where the first + and the last dot are the same, so a rule about the LAST dot was invisible: + `*.??` dropped `archive.tar.gz`. The review found it with 849k pairs. Cover + every dimension of the input, and neutralize a quirk of the oracle instead of + avoiding the inputs that trigger it: `fsutil file setshortname` gives a long + name a short name that matches nothing, `fsutil 8dot3name strip` removes it. +- A very long pattern ended the host with a stack overflow (recursion depth = + tokens): bound the input of a recursive matcher (`ValidateLength`). + +## Agents in parallel worktrees + +- `git stash` is ONE list for all worktrees: a `pop` takes the top entry of + whoever created it and moved the edits of one agent into another's tree. The + brief forbids stash, `restore`, `reset`, `clean`, and checkout of a sha; a RED + run on the unchanged product uses a private base worktree. Protect a foreign + stash with `git update-ref` before anything else. +- An agent that ends its turn without a report may have done nothing (a clean + worktree and an empty probe folder said so): write to it and ask. +- Help generation: `Update-MarkdownHelp` reads `Position` from the help file + that the build copies, not from the C# attribute, so a page edit alone drifts: + edit the page, `New-ExternalHelp`, rebuild, then check the docs (the files are + a fixpoint). A pair of asterisks in one paragraph loses them in the XML. diff --git a/.memory-bank/decisions/0027-open-items-before-5.0.0.md b/.memory-bank/decisions/0027-open-items-before-5.0.0.md index 0e0b4f6..4be75d6 100644 --- a/.memory-bank/decisions/0027-open-items-before-5.0.0.md +++ b/.memory-bank/decisions/0027-open-items-before-5.0.0.md @@ -153,3 +153,45 @@ decided; the rest is to be chosen. Recommended defaults are the agent's. Desktop: Access 74/0, Audit 55/2, Inheritance 72/1, Items 90/0, executed and skipped). No OS-specific finding. The machines are saved again, and `OSWin11E` is off. + +## Built in rc9, the review, and the known limits + +- How: four implementers worked in separate worktrees (WI-1 the five piped + parameters and item 5, WI-2 item 7 and the three inconsistencies, WI-3 the + library items, WI-4 the filter), test first, one commit per item. The agent + integrated them, ran the whole suite on the integrated tree, and had three + read-only reviewers read it (brief `rc9\REVIEW-BRIEF.md` in the session + folder). The findings became follow-up commits, and the branch `ai/rc9` was + rebuilt with `git cherry-pick` in the order of each chain, so every fix sits + in the commit of its item and any item can still be dropped. + +What the review found: + +| Finding | What became of it | +| --- | --- | +| Blocker: a `*` before a dot has to stop at the LAST dot, the matcher stopped at the first; `*.??` missed `archive.tar.gz`. The differential check ran on 8.3 names, which have one dot, and could not see it | Fixed; a differential check over 732,630 multi-dot pairs found no mismatch; the tests pin the multi-dot shapes | +| Major: with `object` parameters, a `$null`, DBNull, or blank value of the OTHER parameter group binds in both sets, so a CSV row with a blank cell of the other group is `AmbiguousParameterSet` (rc8 skipped the cell and created the entry) | Documented and pinned. **Flagged:** the alternative is a restructure of the parameter sets, a larger public change; the maintainer was told that the agent goes on with the documentation and has not objected | +| Major: a named `$null` (`-AccessType $rule.AccessTpye`) silently selected the default | Fixed: a terminating `InvalidParameterValue` for a name in `BoundParameters` whose value is `$null` | +| Minor: a script block is no longer delay-bound; the completers returned nothing for a quoted or wildcard word | Documented and pinned; the completers are fixed | +| Minor: since 3.1, `-SecurityDescriptor (Get-Acl $f)` binds, and then threw from the inherited-from lookup and marked the audit section as read; the null guard of `Name` was not pinned; `Get-NTFSInheritance` wrote its object in a `finally` after it rethrew the exception of a later command | Fixed; the guard is pinned through reflection, because a PowerShell property read turns a throwing getter into `$null` | +| Filter: platyPS removed paired asterisks of a paragraph; the PowerShell 7 claim was too broad; the CHANGELOG said nothing of the new `?`; a pattern of more than 32,767 characters ended the host with a stack overflow; one sentence contradicted itself | Fixed: one asterisk per paragraph, the exact shapes, the CHANGELOG sentence, `[ValidateLength(0,32767)]` | + +Known limits, kept and to be stated to the maintainer: + +- `Get-NTFSEffectiveAccess` on a FAT32 volume or on an item with a NULL DACL + ends with an `ArgumentOutOfRangeException` (mask 2097151). Found by a probe, + not caused by rc9, not fixed. +- The error branch of 6b (`Get-NTFSOrphanedAccess` cannot read an item) has no + deterministic trigger: a locked file, a long path, a dangling junction, and a + FAT32 VHD were tried. It is reviewed, not pinned. +- Item 7: 9 of the 17 handlers have no test. The getters' own + `FileSystemOwner.GetOwner` needs the same read right that the test takes + away, and the audit cmdlets write the SACL whatever the DACL says, so the + action never fails first. +- `Get-FileHash2 -Algorithm` keeps the silent default of a piped value that + does not convert (option C was not chosen). The settings `GetInheritedFrom`, + `GetFileSystemModeProperty`, and `IdentifyHardLinks` still read their value + with an unchecked cast, as `EnablePrivileges` did. `PrivilegeEnabler` (3.4) + stays undisposed, a known limit of the .NET API. +- In PowerShell 7, the case folding of `-Filter` differs from NTFS for a few + exotic letters (finding F7 of the review; not fixed).