diff --git a/.github/workflows/shared-dependency-impact.yml b/.github/workflows/shared-dependency-impact.yml new file mode 100644 index 0000000..6081303 --- /dev/null +++ b/.github/workflows/shared-dependency-impact.yml @@ -0,0 +1,38 @@ +name: Shared dependency impact (advisory) + +on: + pull_request: + paths: + - pyproject.toml + - uv.lock + - scripts/review_shared_dependency_delta.py + - tests/test_review_shared_dependency_delta.py + - .github/workflows/shared-dependency-impact.yml + +permissions: + contents: read + +jobs: + report: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + persist-credentials: false + - uses: actions/setup-python@v6 + with: + python-version: '3.12' + - name: Test report with synthetic offline fixtures + run: python -m unittest discover -s tests -p test_review_shared_dependency_delta.py -v + - name: Report exact base/head dependency changes + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + GIT_TERMINAL_PROMPT: '0' + run: | + python scripts/review_shared_dependency_delta.py \ + --base-ref "$BASE_SHA" --head-ref "$HEAD_SHA" \ + --output "$RUNNER_TEMP/shared-dependency-impact.md" + tee -a "$GITHUB_STEP_SUMMARY" < "$RUNNER_TEMP/shared-dependency-impact.md" diff --git a/docs/shared-dependency-impact-report.md b/docs/shared-dependency-impact-report.md new file mode 100644 index 0000000..448cf3d --- /dev/null +++ b/docs/shared-dependency-impact-report.md @@ -0,0 +1,57 @@ +# Advisory shared-dependency impact report + +The `Shared dependency impact (advisory)` PR workflow complements existing exact-pin, +checkout SHA and lock-consistency checks. It compares the PR event's exact base and +head commits, resolves each direct shared dependency from `pyproject.toml` and +`uv.lock`, and reports every changed upstream production/other path separately from +documentation, tests and CI/configuration. Supported repositories are explicitly +allowlisted: QuantPlatformKit, UsEquityStrategies and HkEquityStrategies. + +The report uses a full Git tree diff, not a paginated GitHub compare response. +Renames are represented as old and new paths so neither side disappears. Fixed +path hints highlight authentication/token refresh, possible cloud/secret writes, +execution, risk and scheduler/automation changes. They are review prompts, not +semantic analysis, a generic dependency graph, a risk score or an allowlist. +Unmatched paths and dynamic imports remain unresolved, never presumed safe. + +## Safety and limitations + +- Report success is **not** an upgrade-safety verdict or merge approval. This adds + no auto-block/auto-approve policy and does not change existing CI gates. +- Nothing installs/imports upstream packages, invokes token refresh, reads token + contents, calls broker/cloud APIs, changes deployment, or writes secrets. +- The workflow uses `pull_request` with `contents: read`, no secrets/OIDC, no PR + comment writes, and checkout credentials are not persisted. Git fetch targets + fixed repository URLs with validated full SHAs, no shell interpolation, depth + one and per-command timeouts. No extra PAT/app grant is requested for forks or + private dependencies. Unavailable upstream access is explicitly INCOMPLETE. +- Missing pins, mismatches, failed comparisons, invalid/truncated output and report + size limits produce INCOMPLETE rather than a false clean report. The report is + advisory, so it returns zero after successfully writing that warning. Test or + infrastructure failures still fail the workflow; inspect the actual summary. +- Normal local startup wires `main.build_composer` through + `application/runtime_bootstrap_adapters.py`'s + `LongBridgeRuntimeBootstrap.build_contexts` to the injected upstream refresh + helper. Account-snapshot contexts bypass refresh. These are explicit local + review notes to revalidate when wiring changes, not generated reachability proof. +- The synthetic `350dd38ece0952beef096893a7dcf8ae3871609a` to + `155d74a615ccdebf7223787939c2dc7151d75efd` fixture includes + `src/quant_platform_kit/longbridge/auth.py` along with presentation changes so an + authentication change cannot be hidden by describing the upgrade as a facade. + +## Local use + +Python 3.11+ and Git suffice; no runtime environment or credentials are needed for +synthetic tests: + +```sh +python -m unittest discover -s tests -p test_review_shared_dependency_delta.py -v +python scripts/review_shared_dependency_delta.py \ + --base-ref FULL_BASE_COMMIT_SHA --head-ref FULL_HEAD_COMMIT_SHA \ + --output /tmp/shared-dependency-impact.md +``` + +The real comparison needs read access to those GitHub upstream trees. No-change +pins do not fetch upstream. Output contains paths and commit identifiers only, +not source patches or credentials. Fetch and diff output is bounded to 2 MB and +the rendered summary to 900 KB; exceeding either makes the report INCOMPLETE. diff --git a/scripts/review_shared_dependency_delta.py b/scripts/review_shared_dependency_delta.py new file mode 100644 index 0000000..15edc3b --- /dev/null +++ b/scripts/review_shared_dependency_delta.py @@ -0,0 +1,195 @@ +#!/usr/bin/env python3 +"""Read-only, advisory shared-dependency delta report. Never imports runtime code.""" +from __future__ import annotations + +import argparse +import re +import subprocess +import tempfile +import tomllib +from pathlib import Path + +REPOSITORIES = { + "quant-platform-kit": "QuantPlatformKit", + "us-equity-strategies": "UsEquityStrategies", + "hk-equity-strategies": "HkEquityStrategies", +} +SHA = re.compile(r"[0-9a-f]{40}") +MAX_OUTPUT = 2_000_000 +# Deliberately conservative path hints, not a call graph or a safety verdict. +FLAGS = { + "auth / token refresh": ("auth", "token", "credential"), + "cloud / possible secret writes": ("secret", "cloud", "auth", "token"), + "execution": ("execution", "order", "broker"), + "risk": ("risk", "guard", "stop_loss", "stoploss"), + "scheduler / automation": ("scheduler", "schedule", "watch/", "automation"), +} +CALL_NOTES = ( + "Local review note (not automatically proven): main.build_composer injects " + "refresh_token_if_needed into application/runtime_bootstrap_adapters.py " + "LongBridgeRuntimeBootstrap.build_contexts. That normal startup path can reach upstream " + "longbridge/auth.py and token/Secret Manager writes. The separate read-only " + "account-snapshot context path bypasses refresh; that does not make normal startup read-only." +) + + +class Incomplete(Exception): + """Evidence is missing, invalid, failed, or exceeds the bounded report limit.""" + + +def exact_sha(value: str) -> str: + if not SHA.fullmatch(value): + raise Incomplete("Missing or invalid full commit SHA") + return value + + +def run_git(repo: Path, *args: str, timeout: int = 60) -> bytes: + # No shell, no diff helpers, no credential contents in output/errors. + with tempfile.TemporaryFile() as output: + try: + subprocess.run( + ["git", "-C", str(repo), *args], stdout=output, + stderr=subprocess.DEVNULL, check=True, timeout=timeout, + ) + except (OSError, subprocess.SubprocessError) as exc: + raise Incomplete("Git comparison/read failed or timed out; access may be unavailable") from exc + if output.tell() > MAX_OUTPUT: + raise Incomplete("Git output exceeds report limit; comparison incomplete (not truncated silently)") + output.seek(0) + return output.read() + + +def resolve_pins(project_text: str, lock_text: str) -> dict[str, str]: + try: + project, lock = tomllib.loads(project_text), tomllib.loads(lock_text) + dependencies = project["project"]["dependencies"] + overrides = project.get("tool", {}).get("uv", {}).get("override-dependencies", []) + result = {} + for name, repository in REPOSITORIES.items(): + prefix = f"{name} @ git+https://github.com/QuantStrategyLab/{repository}.git@" + def matches_name(dependency: str) -> bool: + match = re.match(r"^\s*([A-Za-z0-9_.-]+)", dependency) + return bool(match and re.sub(r"[-_.]+", "-", match[1]).lower() == name) + + declarations = [d for d in dependencies if matches_name(d)] + refs = [d[len(prefix):] for d in declarations if d.startswith(prefix)] + if len(declarations) != 1 or len(refs) != 1: + raise Incomplete(f"{name}: missing or ambiguous direct pin") + pin = exact_sha(refs[0]) + for dependency in overrides: + if matches_name(dependency) and dependency != prefix + pin: + raise Incomplete(f"{name}: override mismatch") + packages = [p for p in lock["package"] if p["name"] == name] + expected = f"https://github.com/QuantStrategyLab/{repository}.git?rev={pin}#{pin}" + if len(packages) != 1 or packages[0].get("source", {}).get("git") != expected: + raise Incomplete(f"{name}: pyproject/lock SHA mismatch or missing resolved SHA") + result[name] = pin + return result + except (AttributeError, KeyError, TypeError, ValueError) as exc: + raise Incomplete("Missing or malformed dependency manifests") from exc + + +def pins_at(repo: Path, ref: str) -> dict[str, str]: + exact_sha(ref) + return resolve_pins( + run_git(repo, "show", f"{ref}:pyproject.toml").decode(), + run_git(repo, "show", f"{ref}:uv.lock").decode(), + ) + + +def changed_paths(repo: Path, base: str, head: str) -> list[str]: + exact_sha(base) + exact_sha(head) + raw = run_git(repo, "diff", "--no-ext-diff", "--no-textconv", "--no-renames", + "--name-only", "-z", base, head, "--") + if raw and not raw.endswith(b"\0"): + raise Incomplete("Incomplete/truncated path list") + try: + return sorted(set(p.decode("utf-8") for p in raw.split(b"\0") if p)) + except UnicodeError as exc: + raise Incomplete("Non-UTF-8 path cannot be represented losslessly") from exc + + +def upstream_delta(repository: str, base: str, head: str) -> list[str]: + if repository not in REPOSITORIES.values(): + raise Incomplete("Unrecognized upstream repository") + exact_sha(base) + exact_sha(head) + with tempfile.TemporaryDirectory(prefix="dependency-review-") as directory: + repo = Path(directory) + run_git(repo, "init", "--bare") + # Fixed GitHub URLs, exact SHAs, depth one. No installation/import or hooks. + url = f"https://github.com/QuantStrategyLab/{repository}.git" + for ref in dict.fromkeys((base, head)): + run_git(repo, "-c", "credential.helper=", "-c", "core.askPass=", + "fetch", "--no-tags", "--depth=1", url, ref, timeout=120) + return changed_paths(repo, base, head) + + +def category(path: str) -> str: + lower = path.lower() + if lower.startswith("docs/") or lower.endswith((".md", ".rst")): + return "Documentation" + if lower.startswith(("tests/", "test/")): + return "Tests" + if lower.startswith(".github/"): + return "CI/configuration" + return "Production / other (review required)" + + +def quoted(value: str) -> str: + # Escape paths for GitHub Markdown/HTML and make control characters visible. + visible = value.encode("unicode_escape").decode("ascii") + return "" + "".join(c if c.isalnum() else f"&#{ord(c)};" for c in visible) + "" + + +def report(base: dict[str, str], head: dict[str, str], compare=upstream_delta) -> str: + lines = ["# Shared dependency change impact (advisory)", "", + "Report generation success is NOT an upgrade-safety verdict or merge approval.", + "Dynamic imports, runtime reachability and transitive behavior remain unresolved; " + "an unflagged path is not proven safe.", "", CALL_NOTES, ""] + for name, repository in REPOSITORIES.items(): + before, after = base[name], head[name] + lines += [f"## {name}", f"Base: {quoted(before)}; head: {quoted(after)}"] + if before == after: + lines += ["No pin change; upstream comparison not needed.", ""] + continue + try: + paths = compare(repository, before, after) + except Incomplete as exc: + lines += [f"INCOMPLETE: {quoted(str(exc))}. No safety conclusion.", ""] + continue + flags = [label for label, hints in FLAGS.items() + if any(any(hint in path.lower() for hint in hints) + for path in paths if category(path).startswith("Production"))] + lines += ["Comparison: complete path list (full Git tree diff; no REST pagination).", + "High-impact review flags: " + (", ".join(flags) or "none matched; not proof of safety")] + if paths and all(category(p) == "Documentation" for p in paths): + lines.append("Documentation-only delta (path classification only).") + for group in ("Production / other (review required)", "Documentation", "Tests", "CI/configuration"): + selected = [p for p in paths if category(p) == group] + lines += [f"### {group} ({len(selected)})"] + lines.extend(f"- {quoted(path)}" for path in selected) + lines.append("") + return "\n".join(lines) + "\n" + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", type=Path, default=Path(".")) + parser.add_argument("--base-ref", required=True) + parser.add_argument("--head-ref", required=True) + parser.add_argument("--output", type=Path, required=True) + args = parser.parse_args() + try: + content = report(pins_at(args.repo, args.base_ref), pins_at(args.repo, args.head_ref)) + except (Incomplete, UnicodeError) as exc: + content = "# Shared dependency change impact\n\nINCOMPLETE: " + quoted(str(exc)) + "\n\nNo upgrade-safety conclusion.\n" + if len(content.encode("utf-8")) > 900_000: + content = "# Shared dependency change impact\n\nINCOMPLETE: rendered report exceeds summary limit. No safety conclusion.\n" + args.output.write_text(content, encoding="utf-8") + return 0 # Advisory only: existing pin/lock and test gates remain authoritative. + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_review_shared_dependency_delta.py b/tests/test_review_shared_dependency_delta.py new file mode 100644 index 0000000..7f90b16 --- /dev/null +++ b/tests/test_review_shared_dependency_delta.py @@ -0,0 +1,138 @@ +"""Synthetic offline fixtures; never import broker/runtime dependencies.""" +import subprocess +import tempfile +import sys +import html +import unittest +from pathlib import Path +from unittest.mock import patch + +from scripts import review_shared_dependency_delta as delta + +BASE = "350dd38ece0952beef096893a7dcf8ae3871609a" +HEAD = "155d74a615ccdebf7223787939c2dc7151d75efd" + + +def manifests(pin=BASE): + dependencies = [] + packages = [] + for name, repository in delta.REPOSITORIES.items(): + url = f"https://github.com/QuantStrategyLab/{repository}.git" + dependencies.append(f'"{name} @ git+{url}@{pin}"') + packages.append(f'[[package]]\nname = "{name}"\nsource = {{ git = "{url}?rev={pin}#{pin}" }}') + return '[project]\ndependencies = [' + ','.join(dependencies) + ']\n', '\n'.join(packages) + + +class DependencyDeltaTests(unittest.TestCase): + def test_exact_manifest_pins(self): + self.assertEqual(set(delta.resolve_pins(*manifests()).values()), {BASE}) + + def test_missing_sha_and_lock_mismatch(self): + project, lock = manifests() + for bad_project, bad_lock in ((project.replace(BASE, 'main'), lock), + (project, lock.replace(BASE, HEAD)), + (project, lock.replace('#' + BASE, ''))): + with self.subTest(lock=bad_lock): + with self.assertRaises(delta.Incomplete): + delta.resolve_pins(bad_project, bad_lock) + + def test_no_change_never_fetches(self): + pins = delta.resolve_pins(*manifests()) + def forbidden(*args): + self.fail('No-change comparison attempted network') + self.assertIn('No pin change', delta.report(pins, pins, forbidden)) + + def test_350dd38_to_155d74a_auth_and_facade_fixture(self): + base = delta.resolve_pins(*manifests()) + head = dict(base, **{'quant-platform-kit': HEAD}) + paths = ['src/quant_platform_kit/longbridge/auth.py', + 'src/quant_platform_kit/common/presentation/value_target_plan.py', + 'src/quant_platform_kit/execution_kernel/guards.py', + 'src/quant_platform_kit/strategy_lifecycle/watch/runner.py', + 'docs/facade.md', 'tests/test_longbridge_auth.py'] + result = delta.report(base, head, lambda *args: paths) + for expected in paths + list(delta.FLAGS) + [BASE, HEAD, 'unresolved']: + self.assertIn(expected, html.unescape(result)) + + def test_docs_only_and_unknown_production_not_safe(self): + base, head = delta.resolve_pins(*manifests()), delta.resolve_pins(*manifests(HEAD)) + docs = delta.report(base, head, lambda *args: ['docs/change.md']) + self.assertIn('Documentation-only delta', docs) + unknown = delta.report(base, head, lambda *args: ['src/dynamic_plugin.py']) + self.assertIn('src/dynamic_plugin.py', html.unescape(unknown)) + self.assertIn('not proof of safety', unknown) + self.assertNotIn('Documentation-only delta', unknown) + + def test_failed_compare_is_incomplete(self): + def fail(*args): + raise delta.Incomplete('comparison failed') + result = delta.report(delta.resolve_pins(*manifests()), + delta.resolve_pins(*manifests(HEAD)), fail) + self.assertIn('INCOMPLETE', result) + self.assertNotIn('Comparison: complete', result) + + def test_non_utf8_path_is_incomplete_not_collapsed(self): + with patch.object(delta, 'run_git', return_value=b'src/\xff.py\0src/\\xff.py\0'): + with self.assertRaises(delta.Incomplete): + delta.changed_paths(Path('.'), BASE, HEAD) + + def test_truncated_output_is_incomplete(self): + with patch.object(delta, 'run_git', return_value=b'path-without-nul'): + with self.assertRaises(delta.Incomplete): + delta.changed_paths(Path('.'), BASE, HEAD) + + def test_refs_and_markdown_paths_are_not_executable(self): + with self.assertRaises(delta.Incomplete): + delta.changed_paths(Path('.'), '--help', HEAD) + hostile = '\n`$(command)`[SAFE](https://example.invalid)![pic](img)*_' + rendered = delta.quoted(hostile) + self.assertEqual(html.unescape(rendered), '' + hostile.encode('unicode_escape').decode() + '') + for marker in '[]()!*_`': + self.assertNotIn(marker, rendered) + + def test_alternate_override_and_duplicate_direct_declarations(self): + project, lock = manifests() + for override in ('quant-platform-kit==999', + 'quant_platform_kit @ git+https://github.com/QuantStrategyLab/QuantPlatformKit.git@' + HEAD): + with self.subTest(override=override), self.assertRaises(delta.Incomplete): + delta.resolve_pins(project + '[tool.uv]\noverride-dependencies = ["' + override + '"]', lock) + duplicate = project.replace('dependencies = [', 'dependencies = ["quant_platform_kit==999",') + with self.assertRaises(delta.Incomplete): + delta.resolve_pins(duplicate, lock) + + + def test_rendered_summary_overflow_is_incomplete(self): + with tempfile.TemporaryDirectory() as directory: + output = Path(directory) / 'report.md' + args = ['report', '--base-ref', BASE, '--head-ref', HEAD, '--output', str(output)] + with patch.object(sys, 'argv', args), patch.object(delta, 'pins_at', return_value={}), \ + patch.object(delta, 'report', return_value='界' * 400_000): + self.assertEqual(delta.main(), 0) + result = output.read_text() + self.assertIn('INCOMPLETE', result) + self.assertLess(len(result.encode()), 900_000) + + def test_real_git_deleted_renamed_and_unusual_paths(self): + with tempfile.TemporaryDirectory() as directory: + repo = Path(directory) + def git(*args): + return subprocess.check_output(['git', '-C', directory, *args], stderr=subprocess.DEVNULL).decode().strip() + git('init') + git('config', 'user.email', 'fixture@example.invalid') + git('config', 'user.name', 'Offline fixture') + (repo / 'old.py').write_text('fixture') + git('add', '.') + git('commit', '-m', 'base') + base = git('rev-parse', 'HEAD') + (repo / 'old.py').rename(repo / 'new\npath.py') + git('add', '.') + git('commit', '-m', 'head') + self.assertEqual(delta.changed_paths(repo, base, git('rev-parse', 'HEAD')), + ['new\npath.py', 'old.py']) + with patch.object(delta, 'MAX_OUTPUT', 1): + with self.assertRaises(delta.Incomplete): + delta.changed_paths(repo, base, git('rev-parse', 'HEAD')) + + +if __name__ == '__main__': + unittest.main()