Next/v0.1.1 - #14
Merged
Merged
Conversation
- Add all-terms regression coverage for LocatedFile IRIs reached through file-link predicates. - Confirm unsafe LocatedFile IRIs are skipped before designator validation. - Serialize VANN preferred namespace URIs as literals in sflo ontologies. - Type published ontology netadata Turtle URLs as LocatedFile/RdfDocument resources.
- Treat mesh metadata and inventory as the complete root publication bootstrap - Stop creating empty _mesh/_config/config.ttl for branch-published roots - Accept ordinary mesh-create bootstraps during gh-pages deploy - Keep failing closed when required mesh bootstrap files are missing - Update CLI docs and deploy tests for config-free publication meshes
- Replace the public `weave deploy gh-pages` command with `weave prepare gh-pages` - Update CLI help, errors, dry-run output, and audit operation ids - Update fixture ladder command generation and CLI/integration tests - Refresh branch-published conformance manifests to replay `prepare gh-pages` - Keep root publication meshes config-free and compatible with ordinary mesh bootstraps
Add prepare gh-pages payload naming flags so branch-published sources can materialize under release/history/state/manifestation paths, and record an immutable source commit alongside the source ref. Keep root GitHub Pages mesh bootstrap lighter by treating _mesh/_config/config.ttl as optional. Refresh ancestor ResourcePages during targeted weave generation. Child lists are display-only navigation over the mesh, not versioned payload state, so updating ancestors keeps sparse shortcut pages like the root welcome page accurate when descendants are added without broadening the versioning slice. Tighten ResourcePage rendering for SFLO dogfooding: document slashless root RDF metadata extraction, open properties by default, move history below source details, and wrap long predicate labels so property keys cannot overlap values. Update CLI docs and tests for the prepare gh-pages release publication path.
Add prepare gh-pages payload naming flags so branch-published sources can materialize under release/history/state/manifestation paths, and record an immutable source commit alongside the source ref. Keep root GitHub Pages mesh bootstrap lighter by treating _mesh/_config/config.ttl as optional. Refresh ancestor ResourcePages during targeted weave generation. Child lists are display-only navigation over the mesh, so updating ancestors keeps sparse shortcut pages like the root welcome page accurate when descendants are added without broadening the versioning slice. Tighten ResourcePage rendering for SFLO dogfooding: document slashless root RDF metadata extraction, open properties by default, move history below source details, and wrap slightly smaller predicate labels so property keys cannot overlap values. Update CLI docs and tests for the prepare gh-pages release publication path.
- Emit per-knop progress during root weave planning, with --silent for quiet runs. - Keep root weave on a single version-planning pass while preserving pre-write RDF validation and input-error behavior. - Treat rdfs:Class child identifiers as Classes alongside owl:Class. - Add CLI and ResourcePage regression coverage for progress output and SFLO-style class grouping.
- thread Knop metadata and inventory history policies through extracted-Knop weaving - omit extracted support history snapshots and ResourcePages under current-only policy - update sidecar extracted-term integration expectations - record the decision to keep KnopInventory current-only explicit
- render predicate labels as subtle anchors while preserving URL tooltips - keep predicate links visually quiet with inherited color and light underline styling - update ResourcePage renderer tests for linked property and blank-node labels
Carry Knop support artifacts through inventory rendering so reference catalogs created by extract --all-terms --add-source-references survive the first weave of each extracted term. This keeps the generated ResourcePage connected to its canonical source reference instead of orphaning _knop/_references/references.ttl. Also cover the extract-to-weave flow with a regression test that verifies the Knop inventory retains hasReferenceCatalog and the rendered page shows the Canonical references panel.
- stop treating rdfs:label and skos:prefLabel as ResourcePage titles - keep stronger title/name predicates for resources that explicitly name themselves - preserve labels as visible RDF properties so ontology terms foreground their actual designator segment - update rendering tests for ontology and SHACL term pages
- add skos:prefLabel and rdfs:label as fallback summary predicates - keep labels out of ResourcePage title extraction - cover preferred-label and label-only summary fallback behavior
- report when prepare gh-pages would make no publication file changes - verify settled prepare dry-runs stay empty for unchanged source bindings - clarify detached publication root semantics in CLI docs
- Drop gh-pages --cname CLI and runtime handling - Treat custom-domain host files as manually managed for now - Preserve manually managed host files in deploy tests - Update current docs/specs/tasks to make GitHub Pages profile .nojekyll-only - Sketch deferred manifest-driven integrate workflow
- delete the prepare gh-pages CLI entry point and CLI-only helpers - replace e2e coverage with a rejection test for the removed command - update CLI and SFLO examples to stop advertising runnable prepare usage - clarify API/spec wording for weave orchestration versus version/validate/generate phases
- add mesh create --publication-profile for auto, none, and GitHub Pages - stop core mesh creation from inferring .nojekyll from meshBase - persist resolved publication profiles in MeshConfig - preserve publication profile facts when local path grants rewrite config - update mesh create docs and focused tests
- Delete the old gh_pages deploy bootstrap runtime and export - Remove dedicated integration tests for that deleted surface - Keep publication-profile mesh create as the retained GitHub Pages path - Update fixture-ladder fallback wording away from prepare.ghPages
- Add runtime publication preset validation for configured GitHub Pages meshes - Add validate mesh/publication CLI scopes - Add weave --validate-before and --validate-after - Cover validation scopes and host preset checks in runtime and e2e tests - Document the new validation CLI forms
- Add a mesh publication preset helper for host-control files - Route GitHub Pages .nojekyll creation through the publication profile - Remove the legacy includeNoJekyll mesh-create bypass - Update mesh-create tests for host-neutral core bootstrap
- Emit artifactResolutionMode_working for mutable extraction and page-source bindings - Keep legacy artifactResolutionMode_current readable as a working-source alias - Accept latestState in extraction source resolution paths - Update tests and roadmap wording for Working/LatestState semantics
- treat exact source-state coordinates as exact without an artifact-resolution mode - switch extract/weave runtime contracts from pinned to exact source resolution - omit hasArtifactResolutionMode for exact ExtractionSource bindings - keep working/latest-state mode handling explicit - update docs, tests, fixture replay normalization, and ResourcePage wording
- treat exact source-state coordinates as exact without an artifact-resolution mode - switch extract/weave runtime contracts from pinned to exact source resolution - omit hasArtifactResolutionMode for exact ExtractionSource bindings - keep working/latest-state mode handling explicit - update docs, tests, fixture replay normalization, and ResourcePage wording
- add repository-backed source metadata flags to weave integrate - write Knop source registries for integrated payload sources - record working resolution mode, repository locator facts, and digest evidence - add publication validation for conservative host-local path leakage - update CLI docs and focused integration/e2e coverage
- add host-local workingLocalRelativePath grant rendering for ~/.sf-local-access.ttl - let integrate grant separate source checkouts without widening mesh config - cover detached source checkout integration with repository provenance - update CLI docs and SFLO dogfooding examples
- Resolve payload-backed ResourcePageSource latestState requests through currentArtifactHistory - Treat requested target histories as latest-in-that-history page-source bindings - Add fail-closed coverage for missing settled history - Rename the removed prepare command guard away from the old gh-pages/deploy naming
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughReplace current/pinned with working/exact semantics, add publication profiles and validation, implement payload version intents and integrate source-binding, update CLI flags/wiring (remove gh-pages deploy), switch manifestation predicate, and synchronize tests, fixtures, CI, and docs. ChangesResolution, publication, intents, and CLI/runtime refactor
Estimated code review effort Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/cli/run.ts (1)
865-888: ⚡ Quick winClarify integrate help text to match enforced source-binding validation.
The CLI enforces repository-backed metadata (
--source-repository-url,--source-repository-ref,--source-repository-path) whenever source-binding options are used, but the option help text doesn’t make that coupling explicit.Proposed docstring tweak
- .option( - "--source-binding-id <id:string>", - "Fragment id for the source binding in the Knop source registry.", - ) + .option( + "--source-binding-id <id:string>", + "Fragment id for a repository-backed source binding in the Knop source registry (requires --source-repository-url, --source-repository-ref, and --source-repository-path).", + ) @@ - .option( - "--source-digest <digest:string>", - "Digest to record for the integrated source bytes. Defaults to a computed sha256 digest when repository source metadata is supplied.", - ) + .option( + "--source-digest <digest:string>", + "Digest to record for repository-backed integrated source bytes. Defaults to a computed sha256 digest when repository source metadata is supplied.", + )Also applies to: 1360-1389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/run.ts` around lines 865 - 888, Update the CLI option help strings so they explicitly state that source-binding options require repository-backed metadata; for each option name shown (e.g. "--source-binding-id", "--source-repository-url", "--source-repository-ref", "--source-repository-commit", "--source-repository-path", "--source-digest") clarify in their descriptions that when any source-binding option is supplied you must also provide repository metadata (--source-repository-url, --source-repository-ref, --source-repository-path) and that --source-digest will default to a computed sha256 when repository metadata is provided; change only the option help text adjacent to those .option(...) calls in run.ts (and the same block at the other occurrence) to make this coupling explicit.src/core/integrate/integrate.ts (1)
542-563: 💤 Low valueConsider extracting
escapeTurtleStringto a shared utility.This function is duplicated in
extract.ts. For maintainability, consider moving it to a shared module likesrc/core/rdf/turtle_utils.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/integrate/integrate.ts` around lines 542 - 563, The escapeTurtleString implementation is duplicated (in integrate.ts and extract.ts); extract it into a single shared utility and update both callers to import it. Create a new module (e.g., turtle utility) that exports escapeTurtleString, move the function body there, replace the local implementations in integrate.ts and extract.ts with an import of the exported escapeTurtleString, and remove the duplicated code so both files reuse the shared function.src/core/mesh/create_test.ts (1)
53-69: ⚡ Quick winAdd a config-content assertion in the auto GitHub Pages test.
This case checks resolved profile and file set, but not the rendered
config.ttlprofile triple. A rendering regression could slip through here.💡 Suggested test hardening
Deno.test("planMeshCreate resolves auto publication profile from GitHub Pages mesh base", () => { const plan = planMeshCreate({ meshBase: "https://semantic-flow.github.io/mesh-alice-bio/", publicationProfile: "auto", }); assertEquals(plan.publicationProfile, "githubPages"); assertEquals( plan.files.map((file) => file.path), [ "_mesh/_meta/meta.ttl", "_mesh/_inventory/inventory.ttl", "_mesh/_config/config.ttl", ".nojekyll", ], ); + assertStringIncludes( + plan.files.find((file) => file.path === "_mesh/_config/config.ttl") + ?.contents ?? "", + "sfcfg:hasPublicationProfile sfcfg:publicationProfile_githubPages", + ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/mesh/create_test.ts` around lines 53 - 69, The test Deno.test("planMeshCreate resolves auto publication profile from GitHub Pages mesh base") checks plan.publicationProfile and file paths but omits asserting the rendered config TTL content; update the test that calls planMeshCreate to also locate the generated "_mesh/_config/config.ttl" file in plan.files (use the existing plan variable and files array) and assert that its content includes the publication profile triple indicating "githubPages" (i.e., the rendered profile triple produced by the config rendering logic), so the test fails on regressions in config.ttl rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 17-21: Remove the duplicate rename rule and fix typos: keep a
single rule that says "Do not rename `*.task.*` notes to `*.completed.*` unless
the user explicitly asks you to; if a user asks and it changes wikilinks, update
affected references" (referencing the existing
`documentation/notes/wd.task.*.md` / `*.task.*` wording), change "no'.md'" to
"no '.md' extension" and correct "wikilnks" to "wikilinks", and ensure the
surrounding list item formatting matches the other bullets.
In `@documentation/notes/wd.general-guidance.md`:
- Line 45: The new guidance redirects developer task notes to
"dependencies/.../weave-dev-archive/notes", which conflicts with the repo policy
that developer-targeting notes belong under documentation/notes/wd.*; update the
paragraph so it explicitly instructs authors to place task notes in
documentation/notes with filenames like
"wd.task.2026-YYYY-MM-DD_HHmm-task-slug.md" (or keep the existing wd.* pattern),
keep the template reference ([[template.task]]) unchanged, and replace the
example path "dependencies/.../weave-dev-archive/notes" with
"documentation/notes/wd.*" to restore alignment with the repository
documentation policy.
In `@src/core/payload/version_intent.ts`:
- Around line 252-267: The ambiguity check treats duplicate identical
current-history triples as distinct; before checking values.length > 1 you
should deduplicate the computed values array so identical toMeshRelativePath
results are collapsed. Locate the block that builds values (the quads.flatMap
that filters by subjectIri and SFLO_CURRENT_ARTIFACT_HISTORY_IRI and maps with
toMeshRelativePath) and replace the raw values used in the
PayloadVersionIntentInputError length check with a deduplicated collection
(e.g., via a Set or Array.from(new Set(...))); keep the subsequent use of value
and the isDirectChildPath(designatorPath, value) check unchanged.
In `@src/core/weave/weave.ts`:
- Around line 2481-2499: The current guard blocks any sflo:hasArtifactHistory
presence (existingHistoryPaths) even when those are intent-only; update the if
condition in weave.ts so that only declared ArtifactHistory subjects block
first-payload handling: replace the existingHistoryPaths.length check with a
check that at least one path in existingHistoryPaths satisfies
isDeclaredArtifactHistory(quads, meshBase, path) (analogous to the
currentHistoryPath check), leaving the currentHistoryPath check as-is; edit the
conditional around resolveNamedNodeObjectPaths/resolveOptionalNamedNodePath and
isDeclaredArtifactHistory so planFirstPayloadWeave() remains reachable for
intent-only histories detected by detectPendingWeaveSlice().
- Around line 6705-6716: The resolver only checks for
SFLO_HAS_WORKING_LOCATED_FILE_IRI via resolveOptionalNamedNodePath and throws if
missing, but renderFirstReferenceCatalogWovenKnopInventoryTurtle can encode the
working file as a literal using SFLO_WORKING_LOCAL_RELATIVE_PATH; update the
logic around referencesFilePath so it first tries
resolveOptionalNamedNodePath(quads, options.meshBase, referenceCatalogPath,
SFLO_HAS_WORKING_LOCATED_FILE_IRI, errorMessage) and if that returns undefined
then attempt to read the literal predicate SFLO_WORKING_LOCAL_RELATIVE_PATH (use
the existing literal-resolution helper or add a small helper to read a literal
path from quads for the same subject), set referencesFilePath from whichever is
found, and only throw WeaveInputError(errorMessage) if neither predicate yields
a path; keep return { referenceCatalogPath } unchanged and reference the symbols
resolveOptionalNamedNodePath, referencesFilePath,
SFLO_HAS_WORKING_LOCATED_FILE_IRI, SFLO_WORKING_LOCAL_RELATIVE_PATH, and
errorMessage when making the change.
In `@src/runtime/integrate/integrate.ts`:
- Around line 353-370: Compute the actual digest via
sha256FileDigest(absoluteSourcePath) and if request.sourceDigest is provided,
compare it to the computed value; if they differ, fail fast (throw or return an
error) instead of silently recording provenance. Update the code around
sourceDigest, IntegrateRepositorySource and expectedContentDigest so that when
request.sourceDigest exists it is validated against the computed digest and only
used if it matches; include the computed digest in logs/error to aid debugging
and ensure repositorySource.contentDigest and expectedContentDigest always
reflect the verified (computed) value.
- Around line 428-449: The current branch routes workspace-root grants through
ensureMeshConfigWorkingDirectoryAccessRule using pathPrefix =
relative(options.meshRoot, absoluteAccessDirectory), but when
absoluteAccessDirectory equals the workspace root this produces ".." which the
mesh-config writer rejects; change the condition so that when
absoluteAccessDirectory is exactly the workspace root (or when pathPrefix ===
"..") you do NOT call ensureMeshConfigWorkingDirectoryAccessRule but instead
call ensureHostLocalWorkingDirectoryAccessRule (i.e., adjust the isWithinRoot
check around isWithinRoot(..., options.localPathPolicy.workspaceRoot) or add an
explicit equality/pathPrefix check) so workspace-root grants go to
ensureHostLocalWorkingDirectoryAccessRule with the same arguments
(options.localPathPolicy, absoluteAccessDirectory).
---
Nitpick comments:
In `@src/cli/run.ts`:
- Around line 865-888: Update the CLI option help strings so they explicitly
state that source-binding options require repository-backed metadata; for each
option name shown (e.g. "--source-binding-id", "--source-repository-url",
"--source-repository-ref", "--source-repository-commit",
"--source-repository-path", "--source-digest") clarify in their descriptions
that when any source-binding option is supplied you must also provide repository
metadata (--source-repository-url, --source-repository-ref,
--source-repository-path) and that --source-digest will default to a computed
sha256 when repository metadata is provided; change only the option help text
adjacent to those .option(...) calls in run.ts (and the same block at the other
occurrence) to make this coupling explicit.
In `@src/core/integrate/integrate.ts`:
- Around line 542-563: The escapeTurtleString implementation is duplicated (in
integrate.ts and extract.ts); extract it into a single shared utility and update
both callers to import it. Create a new module (e.g., turtle utility) that
exports escapeTurtleString, move the function body there, replace the local
implementations in integrate.ts and extract.ts with an import of the exported
escapeTurtleString, and remove the duplicated code so both files reuse the
shared function.
In `@src/core/mesh/create_test.ts`:
- Around line 53-69: The test Deno.test("planMeshCreate resolves auto
publication profile from GitHub Pages mesh base") checks plan.publicationProfile
and file paths but omits asserting the rendered config TTL content; update the
test that calls planMeshCreate to also locate the generated
"_mesh/_config/config.ttl" file in plan.files (use the existing plan variable
and files array) and assert that its content includes the publication profile
triple indicating "githubPages" (i.e., the rendered profile triple produced by
the config rendering logic), so the test fails on regressions in config.ttl
rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce0dd1df-1c4a-4e97-8362-2a782b93dfb0
📒 Files selected for processing (65)
AGENTS.mdREADME.mddeno.jsondocumentation/notes/dev.release-runbook.mddocumentation/notes/release-notes.v0.0.2.mddocumentation/notes/release-notes.v0.1.0.mddocumentation/notes/roadmap.mddocumentation/notes/wd.codebase-overview.mddocumentation/notes/wd.decision-log.mddocumentation/notes/wd.general-guidance.mddocumentation/notes/wd.todo.mddocumentation/notes/wu.cli-reference.examples.sflo.mddocumentation/notes/wu.cli-reference.mddocumentation/notes/wu.repository-options.mdscripts/fixture-ladder.tssrc/cli/run.tssrc/core/extract/extract.tssrc/core/extract/extract_test.tssrc/core/integrate/integrate.tssrc/core/integrate/integrate_test.tssrc/core/knop/add_reference.tssrc/core/knop/add_reference_test.tssrc/core/knop/create.tssrc/core/mesh/create.tssrc/core/mesh/create_test.tssrc/core/mesh/mod.tssrc/core/mesh/publication_preset.tssrc/core/payload/mod.tssrc/core/payload/version_intent.tssrc/core/payload/version_intent_test.tssrc/core/weave/mesh_support_pages.tssrc/core/weave/weave.tssrc/core/weave/weave_test.tssrc/runtime/deploy/gh_pages.tssrc/runtime/deploy/mod.tssrc/runtime/extract/extract.tssrc/runtime/integrate/integrate.tssrc/runtime/mesh/inventory.tssrc/runtime/mesh/inventory_test.tssrc/runtime/mod.tssrc/runtime/operational/local_path_policy.tssrc/runtime/operational/local_path_policy_test.tssrc/runtime/payload/mod.tssrc/runtime/payload/version_intent.tssrc/runtime/publication/mod.tssrc/runtime/publication/presets.tssrc/runtime/weave/page_definition.tssrc/runtime/weave/pages.tssrc/runtime/weave/pages_test.tssrc/runtime/weave/weave.tstests/e2e/deploy_gh_pages_cli_test.tstests/e2e/extract_cli_test.tstests/e2e/integrate_cli_test.tstests/e2e/mesh_create_cli_test.tstests/e2e/prepare_removed_cli_test.tstests/e2e/weave_cli_test.tstests/integration/deploy_gh_pages_test.tstests/integration/extract_test.tstests/integration/mesh_create_test.tstests/integration/validate_version_generate_test.tstests/integration/weave_test.tstests/scripts/fixture_ladder_test.tstests/support/mesh_alice_bio_fixture.tstests/support/mesh_branch_fantasy_rules_fixture.tstests/support/mesh_sidecar_fantasy_rules_fixture.ts
💤 Files with no reviewable changes (4)
- tests/e2e/deploy_gh_pages_cli_test.ts
- src/runtime/deploy/mod.ts
- src/runtime/deploy/gh_pages.ts
- tests/integration/deploy_gh_pages_test.ts
| ## Task notes | ||
|
|
||
| Before starting substantial code changes, a task note should be written and refined. Task notes live in `documentation/notes/` with a filename like `wd.task.2026.2026-MM-DD_HHmm-task-slug.md` and the template lives in [[template.task]]. Actionable "To-Do" items should be pre-pended with markdown checkboxes (`[ ]`) to track completion (`[x]`), cancellation (`[c]`), or deferment (`[d]`). | ||
| Before starting substantial code changes, a task note should be written and refined. Task notes live in `dependencies/github.com/semantic-flow/weave-dev-archive/notes` with a filename like `wa.task.2026.2026-MM-DD_HHmm-task-slug.md` and the template lives in [[template.task]]. Actionable "To-Do" items should be pre-pended with markdown checkboxes (`[ ]`) to track completion (`[x]`), cancellation (`[c]`), or deferment (`[d]`). |
There was a problem hiding this comment.
Keep task-note location guidance aligned with repo policy.
This change points developer task notes to dependencies/.../weave-dev-archive/notes, which conflicts with the current repository guidance for developer-targeting notes.
As per coding guidelines "documentation/notes/wd.*.md: Keep developer-targeting notes in documentation/notes/wd.* files."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@documentation/notes/wd.general-guidance.md` at line 45, The new guidance
redirects developer task notes to "dependencies/.../weave-dev-archive/notes",
which conflicts with the repo policy that developer-targeting notes belong under
documentation/notes/wd.*; update the paragraph so it explicitly instructs
authors to place task notes in documentation/notes with filenames like
"wd.task.2026-YYYY-MM-DD_HHmm-task-slug.md" (or keep the existing wd.* pattern),
keep the template reference ([[template.task]]) unchanged, and replace the
example path "dependencies/.../weave-dev-archive/notes" with
"documentation/notes/wd.*" to restore alignment with the repository
documentation policy.
- validate repository-backed integrate source digests against computed bytes - route workspace-root source grants through host-local access config - accept intent-only payload histories during first-payload weave - resolve reference catalog working paths from workingLocalRelativePath literals - add regression tests for digest, grant, history-intent, and config rendering cases
- add explicit GitHub Pages profile expectations to fixture ladder planning tests - run branch-publication fixture commands with isolated HOME - document stale prepare replay handling in fixture ladder regeneration notes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/payload/version_intent.ts`:
- Around line 439-444: Update isDirectChildPath to reject dot-segment child
paths by locating the candidate child segment (the portion after parentPath/ or
the whole childPath when parentPath is empty), decode percent-encodings with
decodeURIComponent, and return false if that decoded segment is "." or ".." or
empty; keep the existing checks (startsWith and no additional slashes) but add
this decoded-segment validation so encoded forms like "%2e" or "%2e%2e" are also
rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f98cc12e-8e96-42d1-a9f2-e196e41aa81d
📒 Files selected for processing (24)
.github/workflows/ci.ymlAGENTS.mddeno.jsondocumentation/notes/release-notes.v0.1.1.mddocumentation/notes/wd.general-guidance.mddocumentation/notes/wd.testing.fixture-ladder-regeneration.mddocumentation/notes/wd.testing.mddocumentation/notes/wd.todo.mddocumentation/notes/wu.cli-reference.examples.sflo.mdscripts/fixture-ladder.tssrc/cli/run.tssrc/core/extract/extract.tssrc/core/integrate/integrate.tssrc/core/mesh/create_test.tssrc/core/payload/version_intent.tssrc/core/payload/version_intent_test.tssrc/core/rdf/mod.tssrc/core/rdf/turtle.tssrc/core/weave/weave.tssrc/core/weave/weave_test.tssrc/runtime/integrate/integrate.tstests/e2e/integrate_cli_test.tstests/integration/integrate_test.tstests/scripts/fixture_ladder_test.ts
✅ Files skipped from review due to trivial changes (5)
- src/core/rdf/mod.ts
- documentation/notes/wd.testing.md
- documentation/notes/wd.todo.md
- AGENTS.md
- documentation/notes/wu.cli-reference.examples.sflo.md
🚧 Files skipped from review as they are similar to previous changes (9)
- scripts/fixture-ladder.ts
- src/core/integrate/integrate.ts
- src/core/payload/version_intent_test.ts
- documentation/notes/wd.general-guidance.md
- src/runtime/integrate/integrate.ts
- tests/e2e/integrate_cli_test.ts
- src/cli/run.ts
- src/core/weave/weave_test.ts
- src/core/extract/extract.ts
- create deterministic payload-source bindings for extra-mesh working locators - remove user-facing source binding id handling from integrate - keep floating working bindings free of repository and digest evidence - update integrate CLI/runtime docs and focused branch fixture tests
Support current-only woven KnopInventory inputs in knop add-reference and current-only first ReferenceCatalog weaving. Add focused coverage for those paths and update branch-published fixture helpers/tests to materialize source and publication workspaces separately. Align tests with the live a.* fixture ladders by preventing Alice fixture reads from falling back to historical un-prefixed refs. Update fixture ladder planner, mesh-create, sidecar, and branch fixture expectations after regenerating the live ladders. Document the regeneration playbook updates learned during the ladder repair.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/run.ts`:
- Around line 1356-1403: The resolveIntegrateSourceBindingOptions function
currently treats whitespace-only flag values as provided; trim and normalize the
incoming string fields (sourceRepositoryUrl, sourceRepositoryRef,
sourceRepositoryCommit, sourceRepositoryPath, sourceDigest) before any presence
checks so that values like " " are treated as not provided, and explicitly
detect an empty trimmed sourceDigest to avoid silently dropping it; update the
presence checks (the provided and repositoryMetadataProvided booleans) and the
required-field guard (the check that throws IntegrateInputError about missing
--source-repository-url, --source-repository-ref, and --source-repository-path)
to use trimmed values, and return the trimmed strings (or omit fields) in the
returned LocalIntegrateSourceBindingRequest so downstream logic no longer
receives whitespace-only values.
In `@src/core/payload/version_intent.ts`:
- Around line 446-463: The direct-child validation decodes childSegment into
decodedChildSegment but doesn't reject cases where decoding yields a slash
(e.g., "%2Fnested" or "a%2Fb"); update the validation in the same block to
return false if decodedChildSegment.includes("/") (i.e., add a check after
decodeURIComponent and before the "."/".." checks), so decodedChildSegment that
represents deeper paths is rejected; reference variables: childSegment and
decodedChildSegment in the existing validation code.
In `@tests/integration/branch_fantasy_rules_fixture_test.ts`:
- Around line 77-92: The test "branch Fantasy Rules manifests use working-only
source bindings for branch-published sources" is failing because the assertion
on integrateInvocation (the first invocation from readSingleTransitionCase for
each entry in branchSourceBindings) expects "--source-binding-id" to be absent
but current manifests/replay data include it; update the assertion or manifests
to match the intended contract: either regenerate the conformance manifests so
integrateInvocation.argv no longer contains "--source-binding-id", or change the
assertion in the test (around
assertEquals(integrateInvocation?.argv?.includes("--source-binding-id"), false))
to the correct expectation (true or to validate presence plus correct value),
ensuring you update the test logic that iterates branchSourceBindings and checks
integrateInvocation accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e1b4c1df-57a1-4fd9-8e6e-3c781d0d0bd1
📒 Files selected for processing (23)
documentation/notes/release-notes.v0.1.1.mddocumentation/notes/wd.codebase-overview.mddocumentation/notes/wd.runtime.mddocumentation/notes/wd.testing.fixture-ladder-regeneration.mddocumentation/notes/wu.cli-reference.examples.sflo.mddocumentation/notes/wu.cli-reference.mdsrc/cli/run.tssrc/core/integrate/integrate_test.tssrc/core/knop/add_reference.tssrc/core/knop/add_reference_test.tssrc/core/payload/version_intent.tssrc/core/payload/version_intent_test.tssrc/core/weave/weave.tssrc/core/weave/weave_test.tssrc/runtime/integrate/integrate.tstests/e2e/integrate_cli_test.tstests/e2e/mesh_create_cli_test.tstests/integration/branch_fantasy_rules_fixture_test.tstests/integration/integrate_test.tstests/integration/mesh_create_test.tstests/scripts/fixture_ladder_test.tstests/support/mesh_alice_bio_fixture.tstests/support/mesh_branch_fantasy_rules_fixture.ts
✅ Files skipped from review due to trivial changes (3)
- documentation/notes/wd.runtime.md
- documentation/notes/wu.cli-reference.examples.sflo.md
- src/runtime/integrate/integrate.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/support/mesh_alice_bio_fixture.ts
- tests/e2e/mesh_create_cli_test.ts
- src/core/weave/weave_test.ts
- src/core/payload/version_intent_test.ts
- tests/integration/mesh_create_test.ts
- tests/e2e/integrate_cli_test.ts
- tests/scripts/fixture_ladder_test.ts
- src/core/weave/weave.ts
Comment on lines
+1356
to
+1403
| function resolveIntegrateSourceBindingOptions( | ||
| options: { | ||
| sourceRepositoryUrl?: string; | ||
| sourceRepositoryRef?: string; | ||
| sourceRepositoryCommit?: string; | ||
| sourceRepositoryPath?: string; | ||
| sourceDigest?: string; | ||
| }, | ||
| ): LocalIntegrateSourceBindingRequest | undefined { | ||
| const provided = [ | ||
| options.sourceRepositoryUrl, | ||
| options.sourceRepositoryRef, | ||
| options.sourceRepositoryCommit, | ||
| options.sourceRepositoryPath, | ||
| options.sourceDigest, | ||
| ].some((value) => value !== undefined); | ||
| if (!provided) { | ||
| return undefined; | ||
| } | ||
| const repositoryMetadataProvided = | ||
| options.sourceRepositoryUrl !== undefined || | ||
| options.sourceRepositoryRef !== undefined || | ||
| options.sourceRepositoryCommit !== undefined || | ||
| options.sourceRepositoryPath !== undefined; | ||
| if (options.sourceDigest !== undefined && !repositoryMetadataProvided) { | ||
| throw new IntegrateInputError( | ||
| "integrate source digest requires repository-backed source metadata", | ||
| ); | ||
| } | ||
| if ( | ||
| !options.sourceRepositoryUrl || | ||
| !options.sourceRepositoryRef || | ||
| !options.sourceRepositoryPath | ||
| ) { | ||
| throw new IntegrateInputError( | ||
| "repository-backed integrate source bindings require --source-repository-url, --source-repository-ref, and --source-repository-path", | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| sourceRepositoryUrl: options.sourceRepositoryUrl, | ||
| sourceRepositoryRef: options.sourceRepositoryRef, | ||
| ...(options.sourceRepositoryCommit | ||
| ? { sourceRepositoryCommit: options.sourceRepositoryCommit } | ||
| : {}), | ||
| sourceRepositoryPath: options.sourceRepositoryPath, | ||
| ...(options.sourceDigest ? { sourceDigest: options.sourceDigest } : {}), | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Reject blank repository source-binding flags explicitly.
This parser accepts whitespace-only values as valid (" " is truthy), and can silently drop an explicitly provided blank --source-digest. Validate these flags with trimming before required-field checks so empty values fail fast at CLI parsing.
Proposed fix
function resolveIntegrateSourceBindingOptions(
options: {
sourceRepositoryUrl?: string;
sourceRepositoryRef?: string;
sourceRepositoryCommit?: string;
sourceRepositoryPath?: string;
sourceDigest?: string;
},
): LocalIntegrateSourceBindingRequest | undefined {
+ const sourceRepositoryUrl = options.sourceRepositoryUrl?.trim();
+ const sourceRepositoryRef = options.sourceRepositoryRef?.trim();
+ const sourceRepositoryCommit = options.sourceRepositoryCommit?.trim();
+ const sourceRepositoryPath = options.sourceRepositoryPath?.trim();
+ const sourceDigest = options.sourceDigest?.trim();
+
const provided = [
- options.sourceRepositoryUrl,
- options.sourceRepositoryRef,
- options.sourceRepositoryCommit,
- options.sourceRepositoryPath,
- options.sourceDigest,
- ].some((value) => value !== undefined);
+ sourceRepositoryUrl,
+ sourceRepositoryRef,
+ sourceRepositoryCommit,
+ sourceRepositoryPath,
+ sourceDigest,
+ ].some((value) => value !== undefined && value.length > 0);
if (!provided) {
return undefined;
}
const repositoryMetadataProvided =
- options.sourceRepositoryUrl !== undefined ||
- options.sourceRepositoryRef !== undefined ||
- options.sourceRepositoryCommit !== undefined ||
- options.sourceRepositoryPath !== undefined;
- if (options.sourceDigest !== undefined && !repositoryMetadataProvided) {
+ (sourceRepositoryUrl?.length ?? 0) > 0 ||
+ (sourceRepositoryRef?.length ?? 0) > 0 ||
+ (sourceRepositoryCommit?.length ?? 0) > 0 ||
+ (sourceRepositoryPath?.length ?? 0) > 0;
+ if ((sourceDigest?.length ?? 0) > 0 && !repositoryMetadataProvided) {
throw new IntegrateInputError(
"integrate source digest requires repository-backed source metadata",
);
}
if (
- !options.sourceRepositoryUrl ||
- !options.sourceRepositoryRef ||
- !options.sourceRepositoryPath
+ !sourceRepositoryUrl ||
+ !sourceRepositoryRef ||
+ !sourceRepositoryPath
) {
throw new IntegrateInputError(
"repository-backed integrate source bindings require --source-repository-url, --source-repository-ref, and --source-repository-path",
);
}
return {
- sourceRepositoryUrl: options.sourceRepositoryUrl,
- sourceRepositoryRef: options.sourceRepositoryRef,
- ...(options.sourceRepositoryCommit
- ? { sourceRepositoryCommit: options.sourceRepositoryCommit }
+ sourceRepositoryUrl,
+ sourceRepositoryRef,
+ ...(sourceRepositoryCommit
+ ? { sourceRepositoryCommit }
: {}),
- sourceRepositoryPath: options.sourceRepositoryPath,
- ...(options.sourceDigest ? { sourceDigest: options.sourceDigest } : {}),
+ sourceRepositoryPath,
+ ...(sourceDigest ? { sourceDigest } : {}),
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function resolveIntegrateSourceBindingOptions( | |
| options: { | |
| sourceRepositoryUrl?: string; | |
| sourceRepositoryRef?: string; | |
| sourceRepositoryCommit?: string; | |
| sourceRepositoryPath?: string; | |
| sourceDigest?: string; | |
| }, | |
| ): LocalIntegrateSourceBindingRequest | undefined { | |
| const provided = [ | |
| options.sourceRepositoryUrl, | |
| options.sourceRepositoryRef, | |
| options.sourceRepositoryCommit, | |
| options.sourceRepositoryPath, | |
| options.sourceDigest, | |
| ].some((value) => value !== undefined); | |
| if (!provided) { | |
| return undefined; | |
| } | |
| const repositoryMetadataProvided = | |
| options.sourceRepositoryUrl !== undefined || | |
| options.sourceRepositoryRef !== undefined || | |
| options.sourceRepositoryCommit !== undefined || | |
| options.sourceRepositoryPath !== undefined; | |
| if (options.sourceDigest !== undefined && !repositoryMetadataProvided) { | |
| throw new IntegrateInputError( | |
| "integrate source digest requires repository-backed source metadata", | |
| ); | |
| } | |
| if ( | |
| !options.sourceRepositoryUrl || | |
| !options.sourceRepositoryRef || | |
| !options.sourceRepositoryPath | |
| ) { | |
| throw new IntegrateInputError( | |
| "repository-backed integrate source bindings require --source-repository-url, --source-repository-ref, and --source-repository-path", | |
| ); | |
| } | |
| return { | |
| sourceRepositoryUrl: options.sourceRepositoryUrl, | |
| sourceRepositoryRef: options.sourceRepositoryRef, | |
| ...(options.sourceRepositoryCommit | |
| ? { sourceRepositoryCommit: options.sourceRepositoryCommit } | |
| : {}), | |
| sourceRepositoryPath: options.sourceRepositoryPath, | |
| ...(options.sourceDigest ? { sourceDigest: options.sourceDigest } : {}), | |
| }; | |
| function resolveIntegrateSourceBindingOptions( | |
| options: { | |
| sourceRepositoryUrl?: string; | |
| sourceRepositoryRef?: string; | |
| sourceRepositoryCommit?: string; | |
| sourceRepositoryPath?: string; | |
| sourceDigest?: string; | |
| }, | |
| ): LocalIntegrateSourceBindingRequest | undefined { | |
| const sourceRepositoryUrl = options.sourceRepositoryUrl?.trim(); | |
| const sourceRepositoryRef = options.sourceRepositoryRef?.trim(); | |
| const sourceRepositoryCommit = options.sourceRepositoryCommit?.trim(); | |
| const sourceRepositoryPath = options.sourceRepositoryPath?.trim(); | |
| const sourceDigest = options.sourceDigest?.trim(); | |
| const provided = [ | |
| sourceRepositoryUrl, | |
| sourceRepositoryRef, | |
| sourceRepositoryCommit, | |
| sourceRepositoryPath, | |
| sourceDigest, | |
| ].some((value) => value !== undefined && value.length > 0); | |
| if (!provided) { | |
| return undefined; | |
| } | |
| const repositoryMetadataProvided = | |
| (sourceRepositoryUrl?.length ?? 0) > 0 || | |
| (sourceRepositoryRef?.length ?? 0) > 0 || | |
| (sourceRepositoryCommit?.length ?? 0) > 0 || | |
| (sourceRepositoryPath?.length ?? 0) > 0; | |
| if ((sourceDigest?.length ?? 0) > 0 && !repositoryMetadataProvided) { | |
| throw new IntegrateInputError( | |
| "integrate source digest requires repository-backed source metadata", | |
| ); | |
| } | |
| if ( | |
| !sourceRepositoryUrl || | |
| !sourceRepositoryRef || | |
| !sourceRepositoryPath | |
| ) { | |
| throw new IntegrateInputError( | |
| "repository-backed integrate source bindings require --source-repository-url, --source-repository-ref, and --source-repository-path", | |
| ); | |
| } | |
| return { | |
| sourceRepositoryUrl, | |
| sourceRepositoryRef, | |
| ...(sourceRepositoryCommit | |
| ? { sourceRepositoryCommit } | |
| : {}), | |
| sourceRepositoryPath, | |
| ...(sourceDigest ? { sourceDigest } : {}), | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/run.ts` around lines 1356 - 1403, The
resolveIntegrateSourceBindingOptions function currently treats whitespace-only
flag values as provided; trim and normalize the incoming string fields
(sourceRepositoryUrl, sourceRepositoryRef, sourceRepositoryCommit,
sourceRepositoryPath, sourceDigest) before any presence checks so that values
like " " are treated as not provided, and explicitly detect an empty trimmed
sourceDigest to avoid silently dropping it; update the presence checks (the
provided and repositoryMetadataProvided booleans) and the required-field guard
(the check that throws IntegrateInputError about missing
--source-repository-url, --source-repository-ref, and --source-repository-path)
to use trimmed values, and return the trimmed strings (or omit fields) in the
returned LocalIntegrateSourceBindingRequest so downstream logic no longer
receives whitespace-only values.
Comment on lines
+446
to
+463
| if (childSegment === undefined || childSegment.includes("/")) { | ||
| return false; | ||
| } | ||
|
|
||
| let decodedChildSegment: string; | ||
| try { | ||
| decodedChildSegment = decodeURIComponent(childSegment); | ||
| } catch { | ||
| return false; | ||
| } | ||
|
|
||
| if ( | ||
| decodedChildSegment.length === 0 || | ||
| decodedChildSegment === "." || | ||
| decodedChildSegment === ".." | ||
| ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Reject encoded slash after decoding in direct-child validation.
Line 452 decodes the segment, but Line 457-461 does not reject decoded /. A value like %2Fnested (or a%2Fb) passes as a direct child even though it represents a deeper path.
🔧 Proposed fix
if (
decodedChildSegment.length === 0 ||
+ decodedChildSegment.includes("/") ||
decodedChildSegment === "." ||
decodedChildSegment === ".."
) {
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (childSegment === undefined || childSegment.includes("/")) { | |
| return false; | |
| } | |
| let decodedChildSegment: string; | |
| try { | |
| decodedChildSegment = decodeURIComponent(childSegment); | |
| } catch { | |
| return false; | |
| } | |
| if ( | |
| decodedChildSegment.length === 0 || | |
| decodedChildSegment === "." || | |
| decodedChildSegment === ".." | |
| ) { | |
| return false; | |
| } | |
| if (childSegment === undefined || childSegment.includes("/")) { | |
| return false; | |
| } | |
| let decodedChildSegment: string; | |
| try { | |
| decodedChildSegment = decodeURIComponent(childSegment); | |
| } catch { | |
| return false; | |
| } | |
| if ( | |
| decodedChildSegment.length === 0 || | |
| decodedChildSegment.includes("/") || | |
| decodedChildSegment === "." || | |
| decodedChildSegment === ".." | |
| ) { | |
| return false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/payload/version_intent.ts` around lines 446 - 463, The direct-child
validation decodes childSegment into decodedChildSegment but doesn't reject
cases where decoding yields a slash (e.g., "%2Fnested" or "a%2Fb"); update the
validation in the same block to return false if
decodedChildSegment.includes("/") (i.e., add a check after decodeURIComponent
and before the "."/".." checks), so decodedChildSegment that represents deeper
paths is rejected; reference variables: childSegment and decodedChildSegment in
the existing validation code.
Comment on lines
+77
to
92
| Deno.test("branch Fantasy Rules manifests use working-only source bindings for branch-published sources", async () => { | ||
| for (const binding of branchSourceBindings) { | ||
| const registryPath = `${binding.designatorPath}/_knop/_sources`; | ||
| const sourcesFilePath = `${registryPath}/sources.ttl`; | ||
| const inventory = await readMeshBranchFantasyRulesBranchFile( | ||
| "15-extracted-term-references-woven", | ||
| `${binding.designatorPath}/_knop/_inventory/inventory.ttl`, | ||
| ); | ||
| const sources = await readMeshBranchFantasyRulesBranchFile( | ||
| "15-extracted-term-references-woven", | ||
| sourcesFilePath, | ||
| const manifestPath = resolveMeshBranchFantasyRulesConformanceManifestPath( | ||
| binding.manifestName, | ||
| ); | ||
| const manifestText = await Deno.readTextFile(manifestPath); | ||
| const transitionCase = await readSingleTransitionCase(manifestPath); | ||
| const invocations = transitionCase.hasReplayProfile?.hasCommandSequence ?? | ||
| []; | ||
| const integrateInvocation = invocations[0]; | ||
|
|
||
| assertStringIncludes( | ||
| inventory, | ||
| `sflo:hasKnopSourceRegistry <${registryPath}>`, | ||
| assertEquals( | ||
| integrateInvocation?.argv?.includes("--source-binding-id"), | ||
| false, | ||
| ); |
There was a problem hiding this comment.
Fix the failing replay-invocation assertion before merge.
CI is currently failing at Line 89 in this test. The assertion contract (--source-binding-id must be absent) does not match the current manifest/replay data, so this needs to be reconciled (either regenerate manifests to the new contract or adjust the assertion to the actual intended behavior).
🧰 Tools
🪛 GitHub Actions: ci / 0_ci.txt
[error] 89-89: AssertionError in test 'branch Fantasy Rules manifests use working-only source bindings for branch-published sources': expected condition true but was false.
🪛 GitHub Actions: ci / ci
[error] 89-89: AssertionError: 'branch Fantasy Rules manifests use working-only source bindings for branch-published sources' expected true but got false.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/branch_fantasy_rules_fixture_test.ts` around lines 77 - 92,
The test "branch Fantasy Rules manifests use working-only source bindings for
branch-published sources" is failing because the assertion on
integrateInvocation (the first invocation from readSingleTransitionCase for each
entry in branchSourceBindings) expects "--source-binding-id" to be absent but
current manifests/replay data include it; update the assertion or manifests to
match the intended contract: either regenerate the conformance manifests so
integrateInvocation.argv no longer contains "--source-binding-id", or change the
assertion in the test (around
assertEquals(integrateInvocation?.argv?.includes("--source-binding-id"), false))
to the correct expectation (true or to validate presence plus correct value),
ensuring you update the test logic that iterates branchSourceBindings and checks
integrateInvocation accordingly.
- Check out semantic-flow/sflo in CI so ontology guardrail tests can read active ontology files. - Strip ANSI escapes from the CLI version assertion because CI may color Cliffy version output. - Sort publication validation file traversal for deterministic host-local leakage findings.
- collect raw coverage data during deno test so Deno does not emit its automatic coverage report before repo filters apply - exclude .test-tmp coverage sources during explicit LCOV generation - keep deno task ci passing with 452 tests and clean coverage output
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 33-37: The checkout steps using "uses: actions/checkout@v6" (e.g.,
the step with "name: Check out Semantic Flow Ontologies") are missing
persist-credentials: false; add persist-credentials: false under the with: block
for every secondary repository checkout (all steps that use actions/checkout@v6
except the primary repo) so credentials are not persisted across later artifact
uploads, ensuring each checkout's with: block includes persist-credentials:
false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3881f598-2393-41ff-bd6c-c2f196640c87
📒 Files selected for processing (4)
.github/workflows/ci.ymldeno.jsonsrc/runtime/publication/presets.tstests/e2e/weave_cli_test.ts
✅ Files skipped from review due to trivial changes (1)
- deno.json
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/weave_cli_test.ts
- src/runtime/publication/presets.ts
Comment on lines
+33
to
+37
| - name: Check out Semantic Flow Ontologies | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: semantic-flow/sflo | ||
| path: dependencies/github.com/semantic-flow/sflo |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify checkout steps missing persist-credentials hardening
rg -n 'uses:\s*actions/checkout@' .github/workflows/ci.yml -A6 | rg -n 'uses:|persist-credentials|name:'Repository: semantic-flow/weave
Length of output: 687
🏁 Script executed:
cat -n .github/workflows/ci.yml | head -70Repository: semantic-flow/weave
Length of output: 2772
🏁 Script executed:
cat -n .github/workflows/ci.yml | tail -50Repository: semantic-flow/weave
Length of output: 1985
🌐 Web query:
actions/checkout v6 persist-credentials default behavior
💡 Result:
Default behavior (actions/checkout v6): persist-credentials is effectively enabled (default true), meaning the checkout action persists the authentication token it used into the local git configuration so later steps can run authenticated git commands. The token is removed during post-job cleanup. To opt out, set persist-credentials: false. [1][2] Notes on “where” it’s persisted: recent versions improved credential security by storing the credentials under $RUNNER_TEMP (instead of directly writing into.git/config). [1][2] What you should conclude for workflows: if you don’t specify persist-credentials, actions/checkout@v6 will keep credentials available to subsequent steps in the job (until cleanup). [1][2]
Citations:
- 1: https://github.com/actions/checkout/tree/v6.0.0
- 2: https://github.com/actions/checkout/blob/main/README.md
Add persist-credentials: false to all secondary repository checkouts.
All non-primary checkout steps are missing persist-credentials: false. Since credentials default to persisted by actions/checkout@v6 and artifacts are uploaded later in the same job, disabling credential persistence reduces token exposure risk. Apply this setting to checkouts at lines 22–25, 28–31, 33–37, 40–44, 46–51, and 53–58.
Suggested hardening diff
- name: Check out Accord
uses: actions/checkout@v6
with:
repository: spectacular-voyage/accord
path: dependencies/github.com/spectacular-voyage/accord
+ persist-credentials: false
- name: Check out Semantic Flow Framework
uses: actions/checkout@v6
with:
repository: semantic-flow/semantic-flow-framework
path: dependencies/github.com/semantic-flow/semantic-flow-framework
+ persist-credentials: false
- name: Check out Semantic Flow Ontologies
uses: actions/checkout@v6
with:
repository: semantic-flow/sflo
path: dependencies/github.com/semantic-flow/sflo
+ persist-credentials: false
- name: Check out Mesh Alice Bio
uses: actions/checkout@v6
with:
repository: semantic-flow/mesh-alice-bio
path: dependencies/github.com/semantic-flow/mesh-alice-bio
fetch-depth: 0
+ persist-credentials: false
- name: Check out Mesh Sidecar Fantasy Rules
uses: actions/checkout@v6
with:
repository: semantic-flow/mesh-sidecar-fantasy-rules
path: dependencies/github.com/semantic-flow/mesh-sidecar-fantasy-rules
fetch-depth: 0
+ persist-credentials: false
- name: Check out Mesh Branch Fantasy Rules
uses: actions/checkout@v6
with:
repository: semantic-flow/mesh-branch-fantasy-rules
path: dependencies/github.com/semantic-flow/mesh-branch-fantasy-rules
fetch-depth: 0
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Check out Semantic Flow Ontologies | |
| uses: actions/checkout@v6 | |
| with: | |
| repository: semantic-flow/sflo | |
| path: dependencies/github.com/semantic-flow/sflo | |
| - name: Check out Semantic Flow Ontologies | |
| uses: actions/checkout@v6 | |
| with: | |
| repository: semantic-flow/sflo | |
| path: dependencies/github.com/semantic-flow/sflo | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 33-37: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 34-34: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 33 - 37, The checkout steps using
"uses: actions/checkout@v6" (e.g., the step with "name: Check out Semantic Flow
Ontologies") are missing persist-credentials: false; add persist-credentials:
false under the with: block for every secondary repository checkout (all steps
that use actions/checkout@v6 except the primary repo) so credentials are not
persisted across later artifact uploads, ensuring each checkout's with: block
includes persist-credentials: false.
- allocate createTestTmpDir roots through Deno's system temp directory instead of repo-local .test-tmp - add a guard test proving helper-created temp directories stay outside the repository - exclude transient Deno cache paths from LCOV so fake-HOME subprocess caches do not produce source lookup noise
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.
Summary by CodeRabbit
New Features
Documentation
Breaking Changes