Browse Source

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 <ai@example.com>
pull/132/head
Raimund Andree 1 week ago
parent
commit
1c26ea7f4e
  1. 51
      .memory-bank/debugging-insights.md
  2. 42
      .memory-bank/decisions/0027-open-items-before-5.0.0.md

51
.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.

42
.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).

Loading…
Cancel
Save