policy-bound config and runtime resolution - #19
Conversation
- avoid loading working-source payload bytes when extract uses --source-state - add regression coverage for exact extraction with an unresolvable working source - update extract source provenance to emit ArtifactResolutionSpec / observedArtifactResolutionSpec RDF - update the URPX mesh runbook to extract from woven release-state snapshots - use the local source repository remote for URPX floating working-source integration
- avoid loading current working-source payload bytes when extract uses --source-state - resolve exact extract sources from woven HistoricalState snapshots instead - add regression coverage for exact all-terms extraction when the current working source is only a floating repository locator - update extract/source-reference expectations for targetArtifact and targetHistoricalState - emit extraction observations with observedArtifactResolutionSpec instead of mirrored observed-target predicates - update Weave runtime parsers, serializers, page-source resolution, and fixture normalization for ArtifactResolutionSpec vocabulary - clarify CLI docs that --source-state reads woven snapshots and does not require current working-source resolution - update URPX CLI examples to extract from woven release-state snapshots and use floating current repository source bindings
- drop the legacy fixture RDF normalization bridge now that live fixture branches carry the new vocabulary - read Alice, Sidecar, and Branch fixture refs directly from git - update branch-published fixture assertions for IntegrationSource and split negative ASK checks - refresh deno.lock from the verified Deno run Verified with deno task ci.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
💤 Files with no reviewable changes (4)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughReplaces embedded defaults with explicit policy bindings; renames SFLO target/state predicates and collapses observed target facts into an ArtifactResolutionSpec; adds user settings and host-local access profiles; threads resource-page generation/presentation through EffectiveConfig; moves CLI logs to per-mesh XDG paths; updates tests, fixtures, and CI coverage scripts. ChangesEnd-to-end policy and runtime refactor
Sequence Diagram(s) sequenceDiagram
participant CLI
participant OperationalPolicy
participant UserSettings
participant EffectiveConfig
participant WeaveCore
CLI->>OperationalPolicy: loadOperationalLocalPathPolicy(meshRoot)
OperationalPolicy->>UserSettings: resolveUserSettingsPaths(meshBase)
CLI->>UserSettings: resolveCliLogDirForMeshBase(meshBase)
CLI->>EffectiveConfig: loadWeaveEffectiveConfig(meshConfig,..)
EffectiveConfig->>WeaveCore: resource/page generation policies
WeaveCore->>WeaveCore: renderResourcePages (uses presentation profiles)
Estimated code review effort: Possibly related PRs:
Poem:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Rename the default RDF metadata file to stylesheet-metadata.ttl and expose the CSS artifact as the Weave defaults stylesheet. Update effective config defaults and guardrail coverage for the metadata/CSS separation.
- add the first Weave ontology artifact with user settings and host-local access terms - add a user settings resolver for WEAVE_SETTINGS, XDG defaults, mesh identifiers, logs, and caches - read and write host-local path grants from mesh-scoped access.ttl instead of ~/.sf-local-access.ttl - write settings.ttl metadata for mesh settings groups when creating local grants - update tests and user docs for the new settings access profile path
Route CLI runtime logs through the user settings resolver so default operational and audit logs live under the mesh-scoped XDG state directory instead of workspace .weave/logs. Preserve WEAVE_LOG_DIR as the explicit override. Isolate test HOME, WEAVE_SETTINGS, and XDG roots for each test, add shared default-log assertions, and update e2e coverage/docs for the new log path.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/core/weave/shape_assertions.ts (1)
58-60: ⚡ Quick winConsider renaming constants to match the updated predicate names.
The constant names
SFLO_HAS_REQUESTED_TARGET_STATE_IRIandSFLO_HAS_TARGET_ARTIFACT_IRIretain theHAS_prefix, but the underlying predicates have been updated totargetHistoricalStateandtargetArtifact(without the "has" prefix). For consistency and clarity, consider renaming these constants toSFLO_TARGET_HISTORICAL_STATE_IRIandSFLO_TARGET_ARTIFACT_IRI.This would align with other constants in the file like
SFLO_CURRENT_ARTIFACT_HISTORY_IRI(line 67-68) that already match their predicate names.♻️ Proposed refactor
-const SFLO_HAS_REQUESTED_TARGET_STATE_IRI = - `${SFLO_NAMESPACE}targetHistoricalState`; -const SFLO_HAS_TARGET_ARTIFACT_IRI = `${SFLO_NAMESPACE}targetArtifact`; +const SFLO_TARGET_HISTORICAL_STATE_IRI = + `${SFLO_NAMESPACE}targetHistoricalState`; +const SFLO_TARGET_ARTIFACT_IRI = `${SFLO_NAMESPACE}targetArtifact`;Then update all usages within this file (lines 119, 443, 456).
🤖 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/weave/shape_assertions.ts` around lines 58 - 60, Rename the constants SFLO_HAS_REQUESTED_TARGET_STATE_IRI -> SFLO_TARGET_HISTORICAL_STATE_IRI and SFLO_HAS_TARGET_ARTIFACT_IRI -> SFLO_TARGET_ARTIFACT_IRI to match the predicate names (targetHistoricalState, targetArtifact), and update every usage in this file that references SFLO_HAS_REQUESTED_TARGET_STATE_IRI or SFLO_HAS_TARGET_ARTIFACT_IRI to use the new names; ensure any exports or local references in functions that build or assert shapes (places that import or compare these IRIs) are updated accordingly so identifiers remain consistent across the file.src/core/weave/reference_catalog_links.ts (1)
18-20: ⚡ Quick winConsider renaming constants to match the updated predicate names.
Similar to
shape_assertions.ts, these constant names retain theHAS_prefix while the predicates have been updated totargetHistoricalStateandtargetArtifact. Renaming toSFLO_TARGET_HISTORICAL_STATE_IRIandSFLO_TARGET_ARTIFACT_IRIwould improve clarity and consistency.♻️ Proposed refactor
-const SFLO_HAS_REQUESTED_TARGET_STATE_IRI = - `${SFLO_NAMESPACE}targetHistoricalState`; -const SFLO_HAS_TARGET_ARTIFACT_IRI = `${SFLO_NAMESPACE}targetArtifact`; +const SFLO_TARGET_HISTORICAL_STATE_IRI = + `${SFLO_NAMESPACE}targetHistoricalState`; +const SFLO_TARGET_ARTIFACT_IRI = `${SFLO_NAMESPACE}targetArtifact`;Then update usages at lines 113 and 119.
🤖 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/weave/reference_catalog_links.ts` around lines 18 - 20, Rename the constants SFLO_HAS_REQUESTED_TARGET_STATE_IRI and SFLO_HAS_TARGET_ARTIFACT_IRI to SFLO_TARGET_HISTORICAL_STATE_IRI and SFLO_TARGET_ARTIFACT_IRI respectively to match the predicate names (targetHistoricalState, targetArtifact); update every reference in this module that uses the old constant names to the new ones (e.g., the places where those IRIs are imported/consumed later in this file) and run a quick search to replace any remaining usages to keep naming consistent with shape_assertions.ts.src/cli/run.ts (1)
1351-1365: 💤 Low valueDuplicate policy loading per command invocation.
Both
inferCliWorkspaceRoot(line 1348) andresolveCliLogDirForExistingMeshcallloadOperationalLocalPathPolicy(meshRoot), causing each command that uses both to load and parse the operational policy twice. Consider extracting to a single load or passing the policy through:♻️ Suggested optimization
+interface ResolvedMeshContext { + workspaceRoot: string; + logDir: string | undefined; +} + +async function resolveCliMeshContext(meshRoot: string): Promise<ResolvedMeshContext> { + const policy = await loadOperationalLocalPathPolicy(meshRoot); + const logDir = policy.meshBase + ? await resolveCliLogDirForMeshBase(policy.meshBase) + : resolveOptionalCliLogDir(); + return { workspaceRoot: policy.workspaceRoot, logDir }; +}Then in each command action replace the two separate calls with a single
resolveCliMeshContext(meshRoot)invocation.🤖 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 1351 - 1365, Extract the repeated load of the operational policy into a single helper (e.g., create resolveCliMeshContext(meshRoot) that calls loadOperationalLocalPathPolicy(meshRoot) once and returns the policy plus any derived helpers like workspace root and logDir), then update callers (notably resolveCliLogDirForExistingMesh and inferCliWorkspaceRoot) to accept or consume the policy/context instead of calling loadOperationalLocalPathPolicy again; ensure resolveCliLogDirForMeshBase is unchanged except called via the new context path so the policy is parsed only once per command invocation.src/runtime/config/effective_config.ts (1)
854-858: ⚡ Quick winError message lacks context for debugging conflicts.
When conflicting policy bindings are detected, the error doesn't indicate which bindings conflict or what values they hold. This makes troubleshooting configuration issues harder.
♻️ Include conflict details in error
if (values.size !== 1) { + const conflictDetails = priorityWinners.map((b) => + `${b.bindingTerm} (${policyValueForSlot(b.values, slot)})` + ).join(", "); throw new EffectiveConfigError( - `Conflicting ${slot} policy bindings at the same layer, specificity, and priority`, + `Conflicting ${slot} policy bindings at the same layer, specificity, and priority: ${conflictDetails}`, ); }🤖 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/runtime/config/effective_config.ts` around lines 854 - 858, The EffectiveConfigError thrown when values.size !== 1 should include detailed context so you can identify which bindings conflict: update the throw in the block that checks values (the variables slot and values in effective_config.ts) to include the slot name and a serialized list of the conflicting values (e.g., Array.from(values) or equivalent) and any relevant layer/specificity/priority identifiers available in that scope, so the error message shows the slot and the actual conflicting binding values.
🤖 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/runtime/operational/local_path_policy_test.ts`:
- Around line 180-181: The test currently only sets HOME via withEnv before
calling loadOperationalLocalPathPolicy, allowing external WEAVE_SETTINGS to leak
in; update the environment isolation by including WEAVE_SETTINGS (set to empty
string or undefined) in the withEnv call so that withEnv({ HOME: homeRoot,
WEAVE_SETTINGS: "" }, async () => { ... }) is used when invoking
loadOperationalLocalPathPolicy(meshRoot), ensuring the policy load is hermetic
and not influenced by ambient WEAVE_SETTINGS.
In `@tests/support/cli_logs.ts`:
- Line 5: The type alias CliLogFileName is currently internal but is used by
exported functions; make it public by adding the export keyword to the
declaration for CliLogFileName so external modules can reference it (update the
type declaration "type CliLogFileName = ..." to "export type CliLogFileName =
..."). Ensure any imports/exports that rely on this type continue to work with
the newly exported symbol.
---
Nitpick comments:
In `@src/cli/run.ts`:
- Around line 1351-1365: Extract the repeated load of the operational policy
into a single helper (e.g., create resolveCliMeshContext(meshRoot) that calls
loadOperationalLocalPathPolicy(meshRoot) once and returns the policy plus any
derived helpers like workspace root and logDir), then update callers (notably
resolveCliLogDirForExistingMesh and inferCliWorkspaceRoot) to accept or consume
the policy/context instead of calling loadOperationalLocalPathPolicy again;
ensure resolveCliLogDirForMeshBase is unchanged except called via the new
context path so the policy is parsed only once per command invocation.
In `@src/core/weave/reference_catalog_links.ts`:
- Around line 18-20: Rename the constants SFLO_HAS_REQUESTED_TARGET_STATE_IRI
and SFLO_HAS_TARGET_ARTIFACT_IRI to SFLO_TARGET_HISTORICAL_STATE_IRI and
SFLO_TARGET_ARTIFACT_IRI respectively to match the predicate names
(targetHistoricalState, targetArtifact); update every reference in this module
that uses the old constant names to the new ones (e.g., the places where those
IRIs are imported/consumed later in this file) and run a quick search to replace
any remaining usages to keep naming consistent with shape_assertions.ts.
In `@src/core/weave/shape_assertions.ts`:
- Around line 58-60: Rename the constants SFLO_HAS_REQUESTED_TARGET_STATE_IRI ->
SFLO_TARGET_HISTORICAL_STATE_IRI and SFLO_HAS_TARGET_ARTIFACT_IRI ->
SFLO_TARGET_ARTIFACT_IRI to match the predicate names (targetHistoricalState,
targetArtifact), and update every usage in this file that references
SFLO_HAS_REQUESTED_TARGET_STATE_IRI or SFLO_HAS_TARGET_ARTIFACT_IRI to use the
new names; ensure any exports or local references in functions that build or
assert shapes (places that import or compare these IRIs) are updated accordingly
so identifiers remain consistent across the file.
In `@src/runtime/config/effective_config.ts`:
- Around line 854-858: The EffectiveConfigError thrown when values.size !== 1
should include detailed context so you can identify which bindings conflict:
update the throw in the block that checks values (the variables slot and values
in effective_config.ts) to include the slot name and a serialized list of the
conflicting values (e.g., Array.from(values) or equivalent) and any relevant
layer/specificity/priority identifiers available in that scope, so the error
message shows the slot and the actual conflicting binding values.
🪄 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: 45880cf8-35eb-413b-b233-db209ac24086
⛔ Files ignored due to path filters (1)
deno.lockis excluded by!**/*.lock
📒 Files selected for processing (75)
defaults/application.ttldefaults/config-resolution.ttldefaults/stylesheet-metadata.ttldocumentation/notes/wd.runtime.mddocumentation/notes/wu.cli-reference.examples.sflo.mddocumentation/notes/wu.cli-reference.examples.urpx.mddocumentation/notes/wu.cli-reference.extract.mddocumentation/notes/wu.cli-reference.mddocumentation/notes/wu.cli-reference.set.extraction-source.mddocumentation/notes/wu.environment-variables.mddocumentation/notes/wu.logging.mddocumentation/notes/wu.resource-pages.mdontology/weave-ontology.ttlsrc/cli/run.tssrc/core/extract/extract.tssrc/core/extract/extract_test.tssrc/core/import/import.tssrc/core/import/import_test.tssrc/core/integrate/integrate.tssrc/core/integrate/integrate_test.tssrc/core/knop/add_reference.tssrc/core/knop/add_reference_test.tssrc/core/rdf/namespaces.tssrc/core/weave/extraction_source_blocks.tssrc/core/weave/extraction_source_blocks_test.tssrc/core/weave/mesh_support_pages.tssrc/core/weave/planning_models.tssrc/core/weave/reference_catalog_links.tssrc/core/weave/reference_catalog_links_test.tssrc/core/weave/resource_page_policy.tssrc/core/weave/resource_page_reference_links.tssrc/core/weave/resource_page_reference_links_test.tssrc/core/weave/shape_assertions.tssrc/core/weave/weave.tssrc/core/weave/weave_test.tssrc/runtime/config/effective_config.tssrc/runtime/config/effective_config_test.tssrc/runtime/extract/extract.tssrc/runtime/import/import.tssrc/runtime/mesh/inventory.tssrc/runtime/mesh/inventory_test.tssrc/runtime/operational/local_path_policy.tssrc/runtime/operational/local_path_policy_test.tssrc/runtime/settings/user_settings.tssrc/runtime/settings/user_settings_test.tssrc/runtime/weave/execution_config.tssrc/runtime/weave/page_definition.tssrc/runtime/weave/page_definition_test.tssrc/runtime/weave/page_generation.tssrc/runtime/weave/page_model_assembly.tssrc/runtime/weave/pages.tssrc/runtime/weave/pages_test.tssrc/runtime/weave/resource_page_policy_test.tssrc/runtime/weave/version_execution.tstests/e2e/extract_cli_test.tstests/e2e/import_cli_test.tstests/e2e/integrate_cli_test.tstests/e2e/knop_add_reference_cli_test.tstests/e2e/knop_create_cli_test.tstests/e2e/mesh_create_cli_test.tstests/e2e/payload_update_cli_test.tstests/e2e/weave_cli_test.tstests/integration/branch_fantasy_rules_fixture_test.tstests/integration/defaults_guardrails_test.tstests/integration/extract_test.tstests/integration/import_test.tstests/integration/integrate_test.tstests/integration/knop_add_reference_test.tstests/integration/weave_test.tstests/support/cli_logs.tstests/support/fixture_normalization.tstests/support/mesh_alice_bio_fixture.tstests/support/mesh_branch_fantasy_rules_fixture.tstests/support/mesh_sidecar_fantasy_rules_fixture.tstests/support/test_tmp.ts
💤 Files with no reviewable changes (2)
- tests/support/fixture_normalization.ts
- defaults/config-resolution.ttl
Make the legacy local-path policy test independent of ambient WEAVE_SETTINGS, export the shared CLI log filename type, and resolve existing-mesh CLI context once per command. Also align reference-target predicate constant names with SFLO predicates and include conflicting values plus binding context in effective config errors.
- add shared coverage path constants and a prepare task for /tmp/semantic-flow-coverage - write Deno raw coverage, JUnit XML, and LCOV outside the repository - update Codecov CI upload paths for test analytics and LCOV coverage - keep local test analytics explicit while preserving plain test runs - document the temp coverage directory behavior and token expectations
- Keep portable path grants scoped to sfcfg:MeshConfig and stop treating generic sfcfg:OperationalConfig subjects as path-policy carriers. - Rename host-local runtime surfaces from local-config wording to host-local access profile wording for user settings access.ttl. - Preserve integrate grant reporting through a neutral policyPath while routing detached source grants to the settings access profile. - Update focused path-policy tests and integrate docs for the MeshConfig versus user-settings access split.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/operational/local_path_policy.ts (1)
546-584:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when mesh config subjects are missing/ambiguous before reading preserved fields.
collectWorkspaceRootValuesandcollectPublicationProfileValuesnow read onlysfcfg:MeshConfigsubjects, but they don't fail when none (or multiple) exist. That allows downstream rewrite paths to continue with incomplete context and risks clobbering existing config content.Suggested fix
function collectWorkspaceRootValues( quads: readonly Quad[], meshConfigPath: string, ): readonly string[] { const configSubjects = new Set(collectMeshConfigSubjects(quads)); + if (configSubjects.size !== 1) { + throw new OperationalConfigError( + `Expected exactly one ${MESH_CONFIG_IRI} subject in ${meshConfigPath}`, + ); + } const values = quads .filter((quad) => configSubjects.has(toTermKey(quad.subject)) && quad.predicate.value === WORKSPACE_ROOT_RELATIVE_TO_MESH_ROOT_IRI && quad.object.termType === "Literal" @@ function collectPublicationProfileValues( quads: readonly Quad[], meshConfigPath: string, ): readonly string[] { const configSubjects = new Set(collectMeshConfigSubjects(quads)); + if (configSubjects.size !== 1) { + throw new OperationalConfigError( + `Expected exactly one ${MESH_CONFIG_IRI} subject in ${meshConfigPath}`, + ); + } const values = quads .filter((quad) => configSubjects.has(toTermKey(quad.subject)) && quad.predicate.value === HAS_PUBLICATION_PROFILE_IRI && quad.object.termType === "NamedNode"🤖 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/runtime/operational/local_path_policy.ts` around lines 546 - 584, Both collectWorkspaceRootValues and collectPublicationProfileValues currently proceed even when collectMeshConfigSubjects(quads) yields zero or multiple subjects; add a fast-fail check right after creating configSubjects (using collectMeshConfigSubjects) to throw an OperationalConfigError when configSubjects.size === 0 or configSubjects.size > 1 (include meshConfigPath and the relevant IRI constant in the message) so preserved-field reads only occur when there is exactly one sfcfg:MeshConfig subject.
🤖 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:
- Line 79: Replace the tag-pinned GitHub Actions with SHA-pinned refs to harden
CI supply-chain: for each occurrence of actions/checkout@v6,
denoland/setup-deno@v2.0.4, codecov/codecov-action@v6, and
actions/upload-artifact@v7 in the workflow, look up the corresponding action
repository's commit SHA for the desired release/tag and update the `uses:` value
to the full ref form (e.g., owner/repo@<commit-sha>); ensure you update all
instances of codecov/codecov-action and actions/upload-artifact present in the
file so every `uses:` entry is pinned to an immutable commit SHA.
In `@scripts/upload-codecov-test-results.ts`:
- Around line 46-47: In scripts/upload-codecov-test-results.ts the case "--" in
the option-parsing switch currently just breaks the switch so parsing continues;
change it to act as an end-of-options terminator by stopping option parsing and
treating all remaining entries as positional arguments (for example, set an
endOfOptions flag or advance the parser index to the end and push the remaining
process.argv/args entries into the positionalArgs array) instead of just
breaking the switch.
---
Outside diff comments:
In `@src/runtime/operational/local_path_policy.ts`:
- Around line 546-584: Both collectWorkspaceRootValues and
collectPublicationProfileValues currently proceed even when
collectMeshConfigSubjects(quads) yields zero or multiple subjects; add a
fast-fail check right after creating configSubjects (using
collectMeshConfigSubjects) to throw an OperationalConfigError when
configSubjects.size === 0 or configSubjects.size > 1 (include meshConfigPath and
the relevant IRI constant in the message) so preserved-field reads only occur
when there is exactly one sfcfg:MeshConfig subject.
🪄 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: 920aeade-7698-466f-ac0f-8795181a53e9
📒 Files selected for processing (23)
.github/workflows/ci.yml.vscode/settings.jsonAGENTS.mddeno.jsondocumentation/notes/wd.testing.mddocumentation/notes/wu.cli-reference.integrate.mdscripts/coverage-paths.tsscripts/prepare-coverage-dir.tsscripts/run-codecov-test-analytics.tsscripts/upload-codecov-test-results.tssrc/cli/run.tssrc/core/weave/reference_catalog_links.tssrc/core/weave/resource_page_reference_links.tssrc/core/weave/shape_assertions.tssrc/runtime/config/effective_config.tssrc/runtime/config/effective_config_test.tssrc/runtime/integrate/integrate.tssrc/runtime/operational/local_path_policy.tssrc/runtime/operational/local_path_policy_test.tstests/scripts/prepare_coverage_dir_test.tstests/scripts/run_codecov_test_analytics_test.tstests/scripts/upload_codecov_test_results_test.tstests/support/cli_logs.ts
✅ Files skipped from review due to trivial changes (5)
- .vscode/settings.json
- tests/scripts/prepare_coverage_dir_test.ts
- scripts/coverage-paths.ts
- scripts/prepare-coverage-dir.ts
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src/core/weave/resource_page_reference_links.ts
- tests/support/cli_logs.ts
- src/cli/run.ts
- src/runtime/config/effective_config_test.ts
- src/runtime/operational/local_path_policy_test.ts
|
|
||
| - name: Upload test results to Codecov | ||
| if: ${{ !cancelled() }} | ||
| uses: codecov/codecov-action@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify tag-pinned actions in this workflow.
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.yml
# Expected after remediation:
# - No matches in this workflow (or only SHA-pinned references like @<40-hex>).Repository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.ymlRepository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.yml
# Expected after remediation: no matches (only SHA-pinned refs like @<40-hex>)Repository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.yml
# Expected after remediation: no matches (only SHA-pinned refs like @<40-hex>)Repository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.ymlRepository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.yml
# Expected after remediation: no matches (only SHA-pinned refs like @<40-hex>)Repository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.yml
# Expected after remediation: no matches (only SHA-pinned refs like @<40-hex>)Repository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.yml
# Expected after remediation: no matches (only SHA-pinned refs like @<40-hex>)Repository: semantic-flow/weave
Length of output: 505
🏁 Script executed:
rg -n '^\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/ci.yml
# Expected after remediation: no matches (only SHA-pinned refs like @<40-hex>)Repository: semantic-flow/weave
Length of output: 505
Pin CI workflow uses: actions to immutable commit SHAs
.github/workflows/ci.yml uses tag-pinned actions (e.g., @v6, @v7, @v2.0.4) instead of SHA-pinned refs, weakening CI supply-chain integrity:
actions/checkout@v6(lines 19, 22, 28, 34, 41, 48)denoland/setup-deno@v2.0.4(line 61)codecov/codecov-action@v6(lines 79, 100)actions/upload-artifact@v7(lines 93, 108)
🧰 Tools
🪛 zizmor (1.25.2)
[error] 79-79: 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 at line 79, Replace the tag-pinned GitHub Actions
with SHA-pinned refs to harden CI supply-chain: for each occurrence of
actions/checkout@v6, denoland/setup-deno@v2.0.4, codecov/codecov-action@v6, and
actions/upload-artifact@v7 in the workflow, look up the corresponding action
repository's commit SHA for the desired release/tag and update the `uses:` value
to the full ref form (e.g., owner/repo@<commit-sha>); ensure you update all
instances of codecov/codecov-action and actions/upload-artifact present in the
file so every `uses:` entry is pinned to an immutable commit SHA.
Implement direct scoped setting support in EffectiveConfig for mesh publication profile and workspace-root relationship, and resolve naming defaults plus ResourcePage regeneration config as layered singleton scoped settings. Require application config to link its resolver config and reject portable mesh resolver declarations until trusted resolver narrowing is implemented. Add focused unit tests for scoped setting parsing, validation, mesh-local overrides, unsafe workspace-root values, unsupported publication profiles, and mesh resolver-config rejection. Add integration coverage showing mesh-local _config/config.ttl changes history/support behavior, ResourcePage generation, ResourcePage presentation, and the weave/version/generate command surfaces without repeated command flags.
- describe existing-mesh commands loading _mesh/_config/config.ttl above defaults and below command overrides - update SFLO and URPX replay recipes to persist history and ResourcePage presentation policy bindings - clarify ResourcePage metadata panel selection through presentation policy rather than repeated flags
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 `@scripts/run-coverage-tests.ts`:
- Around line 25-31: In the args parsing loop in scripts/run-coverage-tests.ts
(the for loop iterating over args with the switch on arg), change the case "--"
handler so it stops option parsing and treats the rest of args as positional
tail instead of a no-op; for example, on encountering case "--" set a flag or
advance the loop index to args.length (or break the outer loop) so subsequent
arguments are not processed by the switch and can be handled as positional
values by the rest of the script.
🪄 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: dc9f854d-58c6-40dd-9c83-165b67b34fb3
📒 Files selected for processing (22)
.github/workflows/ci.ymldeno.jsondocumentation/notes/wd.testing.mdscripts/coverage-paths.tsscripts/normalize-deno-junit.tsscripts/run-codecov-test-analytics.tsscripts/run-coverage-tests.tsscripts/upload-codecov-test-results.tssrc/runtime/config/effective_config.tssrc/runtime/config/effective_config_test.tssrc/runtime/operational/local_path_policy.tssrc/runtime/operational/local_path_policy_test.tstests/e2e/integrate_cli_test.tstests/e2e/mesh_create_cli_test.tstests/integration/defaults_guardrails_test.tstests/integration/integrate_test.tstests/integration/validate_version_generate_test.tstests/integration/weave_test.tstests/scripts/normalize_deno_junit_test.tstests/scripts/run_codecov_test_analytics_test.tstests/scripts/run_coverage_tests_test.tstests/scripts/upload_codecov_test_results_test.ts
✅ Files skipped from review due to trivial changes (2)
- tests/scripts/run_coverage_tests_test.ts
- documentation/notes/wd.testing.md
🚧 Files skipped from review as they are similar to previous changes (8)
- scripts/coverage-paths.ts
- deno.json
- tests/e2e/mesh_create_cli_test.ts
- tests/integration/defaults_guardrails_test.ts
- scripts/upload-codecov-test-results.ts
- tests/e2e/integrate_cli_test.ts
- src/runtime/operational/local_path_policy.ts
- src/runtime/config/effective_config.ts
- Drop configLayerRole_meshInheritable from Weave defaults and parser constants. - Simplify Knop inheritance projection to ancestor Knop offers only. - Update config runtime tests for the surviving layer order.
Summary by CodeRabbit
New Features
Configuration & Environment
Documentation
Tests & Tooling