chore(skills): complete skill-lib b4234ca propagation - #14
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1a168032b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for relation in report["cross_repository_relations"]: | ||
| relations.append({"from": repository, "to": relation["repository"], **{k: v for k, v in relation.items() if k != "repository"}}) |
There was a problem hiding this comment.
Because cross_repository_relations explicitly permits additional properties, a schema-valid relation containing "from" or "to" overwrites the endpoints derived here. For example, adding "from": "Other/repo" to PTCNA's UCNS relation causes the output to attribute that relation to Other/repo, corrupting the authority graph and its digest; exclude reserved endpoint keys or apply the derived fields after copying extension metadata.
Useful? React with 👍 / 👎.
| # auth_boundary: none | ||
| # storage_boundary: none | ||
| # network_boundary: none | ||
| # user_data_boundary: none | ||
| # admin_only: false |
There was a problem hiding this comment.
When invoked as documented, this module reads every supplied report and, with --output, creates directories and writes a file, so declaring storage_boundary: none misrepresents its runtime permissions; the module also lacks the required actual-runtime BOUNDARIES block. Declare the read/write filesystem effects in both the build metadata and a BOUNDARIES declaration.
AGENTS.md reference: AGENTS.md:L45-L49
Useful? React with 👍 / 👎.
| # network_boundary: none | ||
| # user_data_boundary: none | ||
| # admin_only: false | ||
| # tests: tests/test_interdependent_work_graph_portfolio_plan.py |
There was a problem hiding this comment.
In the reviewed PTCNA tree, a repo-wide rg finds no tests/test_interdependent_work_graph_portfolio_plan.py file—only this metadata reference—and the new module declares neither CONTRACTS nor corresponding test-owned CHECKS. Consequently scripts/check_contracts.py reports a closed graph while none of this aggregator's validation and determinism promises have executable evidence; propagate the declared tests and their contract/check metadata with the module.
AGENTS.md reference: AGENTS.md:L45-L49
Useful? React with 👍 / 👎.
| repository = report.get("repository") | ||
| _require(isinstance(repository, str) and repository.count("/") == 1, f"{source_path}: invalid repository") |
There was a problem hiding this comment.
For an input such as "repository": "/ptcna" or "ptcna/", the slash-count check passes even though the frozen report schema requires non-empty owner and repository components. The aggregator then emits a portfolio that violates its own ^[^/]+/[^/]+$ output schema, so validate both components rather than only counting separators.
Useful? React with 👍 / 👎.
| if args.output: | ||
| args.output.parent.mkdir(parents=True, exist_ok=True) | ||
| args.output.write_text(rendered, encoding="utf-8") |
There was a problem hiding this comment.
When --output resolves to one of the supplied report paths, the program loads that authoritative report and then silently replaces it with the derived portfolio document. For example, passing the same file as the sole report and output returns success but destroys its repository-plan contents; reject output paths that alias any input before writing.
Useful? React with 👍 / 👎.
| Usage guidance: | ||
| python interdependent-work-graph/portfolio_plan.py \ | ||
| ../a0/docs/work-graphs/repository-plan-report.json \ |
There was a problem hiding this comment.
When this documented command is run from the PTCNA repository root, it fails immediately because the propagated script lives at .agents/skills/interdependent-work-graph/portfolio_plan.py, not interdependent-work-graph/portfolio_plan.py. Update the repo-local invocation so users can run the newly installed tool from the workspace described by the documentation.
Useful? React with 👍 / 👎.
| }) | ||
| repository_views.append({ | ||
| "repository": repository, | ||
| "authority": report["authority"], |
There was a problem hiding this comment.
For library callers of the declared public build_portfolio surface, this stores the input report's mutable authority object directly in the returned portfolio. Mutating that report after the call consequently changes the supposedly content-addressed portfolio while portfolio_plan_sha256 retains its old value; copy projected containers before calculating and returning the digest.
Useful? React with 👍 / 👎.
1 participant
Completes PTCNA's already-pinned skill-lib@b4234ca materialization by adding the omitted interdependent-work-graph portfolio files and refreshing the generated msdmd collection. Strict drift, 168 tests, contract audit, 228 ratios, collection replay, build, and Twine checks pass. No runtime or evaluation outcomes change.