Twinned from https://github.com/cfsh/ninefold/issues/750 by tools/beads/import_github.py's reconcile pass.
Suggestions from ninefold-reviewer's review of #748 (c7c8b78). Both are advisory residuals from a CONVERGED verdict — nothing is blocked on them, and if #748 folds them into its own branch before merge, close this.
ProbeScratchIsolationTests.TheRedirectIsEstablishedAfterTheScriptIsParsed no longer catches the deletion of the success-path redirectshared/tests/ProbeScratchIsolationTests.cs:66. The F1 fix added a second UserPaths.Redirect = UserDataRoot; — the catch's, at shared/VisualProbe.cs:235 — above the real one at :242. The test uses IndexOf, so it now anchors on the catch's occurrence.
| mutation | caught? | by |
|---|---|---|
| move :242 back above the try | yes | it becomes the first occurrence, so redirect < load |
| delete :235 (the catch's) | yes | AFailedParseStillRedirectsUserPathsAwayFromTheSharedDir |
| delete :242 (the success path) | no | IndexOf still lands on :235, which is after Load, so this test stays green |
| delete both | yes | AFailedParseStillRedirectsUserPathsAwayFromTheSharedDir |
Nothing else covers the third row: UserPathGuardTests.TheGuardWouldNoticeIfTheCallSitesVanished counts UserPaths.Resolve call sites, not Redirect assignments, and EveryUserSchemePathGoesThroughUserPaths sees no new raw literal. So deleting that one line leaves the whole suite green while every probe on the success path runs with Redirect == null and writes its saves and config into the shared app_userdata/Ninefold/ — #128/#358, with writes.
Before c7c8b78 that mutation was caught, by this same test's Assert.True(redirect > 0).
LastIndexOf alone is not the fix — it would then miss the catch-deletion row. Pinning the count is:
```csharp var occurrences = source.Split(RedirectAssign).Length - 1; Assert.True(occurrences == 2, $"expected the redirect on BOTH the success path and the catch (#748 F1); found {occurrences}"); var redirect = source.LastIndexOf(RedirectAssign, StringComparison.Ordinal); ```
catch comment's claim about OutDir is not always trueshared/VisualProbe.cs:232:
> OutDir is still its default here, which is correct — no probe-specific value was ever parsed.
Load sets OutDir = _outDir at :294 and keeps parsing afterwards — resolution, scene, and root.GetProperty("steps") at :359. Of the four throws the same comment enumerates eight lines above ("a script path that does not exist, malformed JSON, no steps key, a wait that is not an int"), the last two throw with OutDir already probe-specific, and the catch then redirects to <clone>/<outDir>/userdata.
Harmless behaviourally — arguably better, since the failing probe redirects to its own dir rather than the shared probe_out/, and neither target is created on that path anyway. It is a wrong sentence on the exact branch a future reader will be reasoning about. Something like *"OutDir may still be its default here, and either answer is correct — neither is the shared dir"* says what is true.
None.
No comments.