Repository navigation
docs: record that manifests are located from the repository root only - #29
Merged
Merged
Conversation
A review of PR #28 found behaviour none of the documents stated: deps probes the repository root for known filenames, so a monorepo sub-package lock is not measured — and audit does not notice, because the version declaration added in that PR compares the root manifest and nothing else. Measured rather than read from the code: a fixture whose root package-lock.json pins root-a and whose packages/app/package-lock.json pins sub-b and sub-c reports only root-a from both `audit` and `deps --repo`, with an empty code list, while `euthyna deps --lockfile <sub>` reports sub-b and sub-c. That makes the remedy part of the contract: measure a sub-package by pointing --lockfile at it, or say it was not measured. Not a code change — reporting the root lock while silently ignoring the rest of the workspace would let one manifest stand in for a whole repository's dependency surface. Also corrected two stale statements in AGENTS.md: the suite count (235 → 246, measured today) and the description of what cli.test.js now pins.
|
Sorry @modusensus, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 5 days and 2 hours by commenting |
Reviewer's GuideDocumentation-only PR that records the root-only manifest discovery contract across paired fact-contract and skill documents, and updates AGENTS.md for the current 246-test suite and manifest-version declaration behavior. No runtime code changes are introduced; verification reports 245 passing tests and one deliberate skip. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
@sourcery-ai review |
|
Sorry @modusensus, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 5 days and 2 hours by commenting |
AGENTS.md's test row said "1 deliberate skip on non-Windows". The condition is not the OS: bench.test.js skips the exploit ground truth unless `tar --version` reports bsdtar. Measured on cdece0f — ubuntu node 24: 245 pass + 1 skip, with the reason printed as *defined against bsdtar, not "tar (GNU tar) 1.35"*; windows node 24: 246 pass + 0 skip. And this Windows machine reports 245 + 1 under Git Bash, because D:\Git\usr\bin\tar.exe (GNU tar 1.35) resolves ahead of System32's bsdtar. So the number a reader sees depends on PATH, not platform, and the row now says which machine produced which figure. README's quickstart carried a second copy of the count (207 tests) and was 39 behind. Nothing compares it, so it was removed rather than updated: one number in one row is a fact that can be checked, two are a drift waiting to happen. The command itself is unchanged and still what was run — package.json defines `test` as `node --test`.
Merged
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
1 participant
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.
Docs only — one boundary recorded, three stale or wrong statements corrected. Found while reviewing PR #28 and then while checking the suite count.
The boundary: manifests are located from the repository root only
depsprobes the repository root for the known filenames, so a monorepo sub-package lock is not measured — andauditdoes not notice, because the version declaration PR #28 added compares the root manifest and nothing else.Measured rather than read off the code. A fixture whose root
package-lock.jsonpinsroot-awhilepackages/app/package-lock.jsonpinssub-bandsub-c:So the remedy is part of the contract now: measure a sub-package by pointing
--lockfileat it, or state that it was not measured. Written intodocs/fact-contract.md§6.3 anddocs/fact-contract-zh.md§6.3 (same position in both, line 360/361), and intodependency-audit.md§A.2 of both skill editions.This is deliberately not a code change. Making
auditenumerate every lock in a tree would change what one audit run means and how many facts it reports; the failure mode being guarded against is exactly that — one manifest standing in for a whole workspace's dependency surface. If the recursive search is wanted, it is its own design question.The suite count, and what the skip actually keys on
The AGENTS.md row said "1 deliberate skip on non-Windows". That is not the condition —
test/bench.test.jsskips the exploit ground truth unlesstar --versionreports bsdtar. From the CI logs ofcdece0f:tar (GNU tar) 1.35"And this Windows machine reports 245 pass + 1 skip, because
where tarresolvesD:\Git\usr\bin\tar.exe(GNU tar 1.35) ahead ofC:\Windows\System32\tar.exe. So the number a reader sees depends on PATH, not on the platform — the row now says which machine produced which figure.README.mdcarried a second copy of the count (207 tests) and was 39 behind. Nothing compares it, so it was removed rather than updated: one number in one row is checkable, two are drift waiting to happen. The quickstart command itself is unchanged and still what was run —package.jsondefinestestasnode --test.AGENTS.md's description of what
cli.test.jspins was also out of date, now fixed to name the manifest-version declaration and the exported pure function that makes the fail-closed branch testable.Verification
node --teston this branch → tests 246 / pass 245 / fail 0 / skipped 1, which is the local run the corrected row describes, andtest/docs-pair.test.js+test/skill-mirror.test.jsboth stayed green with the four documents edited in pairs.