fix: five verified defects from the Broad-Side self-scan (#128-#132) - #145
Merged
Conversation
- #128 writeLibraryConfig now derives its parent directory with dirname(), fixing ENOENT on Windows where the hand-rolled forward-slash match treated every path as a bare filename. - #130 isWithinPath respects roots that already end in a separator (/, C:\), which previously produced a double-separator prefix that rejected every legitimate subpath. - #131 acquireLock closes the lock descriptor on a write failure instead of leaking it until GC. - #129 the phase runner settles the compaction promise with false when no compaction occurred, so continuation stops paying the full COMPACTION_SETTLE_TIMEOUT_MS on every non-compacted continue. - #132 completeValidatedPhase re-validates the output inside the status lock: a stale PASS whose output changed since the caller's validation refuses with a clear error and leaves status untouched. 6 regression tests in tests/broadside-scan-fixes.test.mjs; 348 total.
…-scan-fixes # Conflicts: # CHANGELOG.md
…ards The file's header said each test fails without its fix. Two do: #130 and the #132 stale-PASS refusal. The other three pass against unfixed code — #128 is a Windows-only path bug that POSIX cannot reproduce, #131 covers acquire/release rather than the writeFile-throws path the fix guards, and #129 exercises waitForCompaction rather than runPhase where the fix lives. The fixes are still right; the claim about them was not. Saying so keeps the next reader from assuming those three are protected against a refactor that reverts them. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LmPQsx1esS4uHDzVbbKVZg
TheAmericanMaker
added a commit
that referenced
this pull request
Aug 23, 2026
…127) deriveSlug uses only the trailing path segment of source_repo, so acme/whisper and openai/whisper both produce "whisper" and publishing the second wrote it as v2 of the first. The entry's version history then spanned two unrelated codebases, and because buildIndexEntry reads only the newest version's metadata, index.yaml attributed the whole entry to whichever repo published last. Nothing reported it, at publish time or after. publishEntry now reads the source_repo recorded on the newest version and fails before writing anything when it denotes a different repository. The check sits ahead of the content-hash branch: identical spec bytes take the metadata-only path, which overwrote the other project's source_repo and headline in place, so guarding only the new-version path would have left the quieter half of the bug intact. forceNewVersion does not bypass it — that option means "another version of this entry", not "overwrite a different project". Unreadable or malformed recorded metadata skips the check, since refusing on unknown would turn a corrupt v1 into an entry nobody can publish to. Comparison is normalized so re-publishing one repository spelled another way is unaffected: scheme or none, embedded credentials (git@, user:token@), git@host:path SCP syntax, a default port, a www. prefix, a trailing .git, repeated and trailing slashes, and separators. A non-default port is kept, since two services on one host can differ by port alone. Case folds for hosts, for the repository paths the forges serve over them, and for Windows drive paths — all case-insensitive. It does not fold for absolute POSIX paths: /srv/Repos/tool and /srv/repos/tool are two directories on a case-sensitive filesystem, and folding them let a genuine two-project collision through the guard silently. Pi records the analyzed directory as source_repo, so local paths are the common shape on that surface. Host aliases (ssh.github.com) and provider-specific SSH path layouts stay deliberately distinct; docs/library-format.md says so. The refusal names where each remedy lives rather than telling the caller to pass an option it may not expose — /codecarto-publish takes no arguments and allow_source_repo_change is MCP-only. Closing that gap on Pi is #146. library-format.md previously promised auto-suffixing (-2, -3) for this case. It was never implemented, and silent suffixing is its own surprise for an agent-driven tool, so the doc now describes the refusal and the override. Twelve tests. Three fail with the guard disabled (the two-project collision, the metadata-only path, forceNewVersion) and four more fail against the pre-review normalization (the equivalence set, the case rules both ways, an end-to-end refusal of two local directories differing only in case, and the message contract). Merged main to resolve a CHANGELOG conflict under [Unreleased]; no source overlap with #144 or #145. 395 tests passing, tsc clean. Fixes #123. Follow-ups filed: #146, #147, #148.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five real defects found by running Broad-Side (the batch scout in PR #144) on its own repo, each verified by hand before fixing:
writeLibraryConfigderives the parent dir withdirname()— the forward-slash match treated Windows paths as bare filenames and ENOENT'd on writeisWithinPathrespects roots ending in a separator (/,C:\) instead of producing a double-separator prefix that rejected every subpathacquireLockcloses the lock descriptor whenwriteFilethrows instead of leaking it to GCfalsewhen no compaction fired, so continuations stop paying the full 30s settle timeoutcompleteValidatedPhasere-validates the output inside the status lock — a stale PASS whose output changed refuses with a clear error and leaves status untouched (validations with nooutputPathkeep the legacy synthetic path used by unit tests)6 regression tests in
tests/broadside-scan-fixes.test.mjs, each failing without its fix. 348/348 passing.Closes #128, #129, #130, #131, #132.