diff --git a/README.md b/README.md index f3d3c59..4b9fdaa 100644 --- a/README.md +++ b/README.md @@ -316,7 +316,7 @@ Review mode does not need `contents: write`: PR-specific generated files are sto | `comment_header` | review | `Architecture review` | Heading for the PR comment. | | `trigger_command` | review | `/codeboarding` | Slash command for trusted on-demand runs. | | `cta_base_url` | review | empty | Click-proxy base URL: deep-links the editor link into VS Code/Cursor and adds a "get the extension" link (tracks owner/repo/pr). Empty links to the extension listing instead (GitHub strips `vscode:`/`cursor:` from comments). | -| `webview_base_url` | review | `https://app.codeboarding.org` | Hosted webview base URL. The PR comment links to an artifact-backed head-vs-comparison-branch architecture diff. Set empty to disable the browser link. | +| `webview_base_url` | review | `https://app.codeboarding.org` | Hosted webview base URL. The PR comment uses the stable `/{owner}/{repo}/pull/{pr}` route for an artifact-backed head-vs-comparison-branch architecture diff. Set empty to disable the browser link. | | `output_dir` | sync | `.codeboarding` | Directory the rendered docs and analysis metadata are committed to. Owned by the action: pre-existing top-level `.md` files in it are deleted on every run. | | `output_format` | sync | `.md` | Output format. Only `.md` is supported. | | `target_branch` | sync | `${{ github.ref_name }}` | In `push` strategy, the branch the docs are pushed to. In `pull_request` strategy, the PR base branch. | diff --git a/action.yml b/action.yml index df3a7d5..b4ecc85 100644 --- a/action.yml +++ b/action.yml @@ -68,7 +68,7 @@ inputs: required: false default: '' webview_base_url: - description: 'Review mode: hosted webview base URL. The PR comment links to an artifact-backed head-vs-comparison-branch architecture diff; review mode does not commit generated files to PR branches. Set empty to disable the browser link.' + description: 'Review mode: hosted webview base URL. The PR comment uses the stable /{owner}/{repo}/pull/{pr} route for an artifact-backed head-vs-comparison-branch architecture diff; review mode does not commit generated files to PR branches. Set empty to disable the browser link.' required: false default: 'https://app.codeboarding.org' trigger_command: @@ -1322,7 +1322,6 @@ runs: DIAGRAM_MD: ${{ steps.diagram.outputs.diagram_md }} BASE_ANALYSIS: ${{ steps.analyze.outputs.base_analysis }} HEAD_ANALYSIS: ${{ steps.analyze.outputs.head_analysis }} - RUN_ID: ${{ github.run_id }} CHANGED_COUNT: ${{ steps.diagram.outputs.n_changed }} CHANGED: ${{ steps.diagram.outputs.changed }} RENDERED: ${{ steps.diagram.outputs.rendered }} @@ -1351,13 +1350,12 @@ runs: cta() { local extra=() # ARTIFACT_NAME presence is the readiness gate (an analysis artifact was - # uploaded for this run). The short webview link itself needs only the run id - # — the webview re-derives head/base/artifact from the run + its metadata. + # uploaded for this run). The stable webview link identifies the repository + # and PR; the webview resolves their latest artifact. if [ -n "$WEBVIEW_BASE" ] && [ -n "$ARTIFACT_NAME" ]; then extra+=( --webview-ready --webview-base "$WEBVIEW_BASE" - --run-id "$RUN_ID" ) fi python3 "$ACTION_PATH/scripts/build_cta.py" \ diff --git a/scripts/build_cta.py b/scripts/build_cta.py index 6f5b6ce..384aa00 100644 --- a/scripts/build_cta.py +++ b/scripts/build_cta.py @@ -4,8 +4,8 @@ VS Code" — that merges the hosted-webview link with the editor link(s), preceded by a warning banner when real health findings exist. The "browser" link (a no-install hosted webview) is included only when ``webview_ready``; it's a short GitHub-style -link (``/owner/repo/pull/?run=``) that the webview resolves to this PR's -uploaded analysis artifact for the run. With a click proxy (``cta_base``) the +link (``/owner/repo/pull/``) that the webview resolves to this PR's latest +uploaded analysis artifact. With a click proxy (``cta_base``) the editor link routes through it (owner/repo/pr tracked) and deep-links into the editor (the proxy redirects to a ``vscode:``/``cursor:`` URL), and a separate "install the extension" link is appended. Without a proxy GitHub's comment sanitizer strips custom @@ -66,19 +66,17 @@ def webview_url( repo: str, *, pr: str = "", - run_id: str = "", ) -> str | None: """Return the hosted-webview PR deep-link, or None. - A GitHub-style short link: ``{base}/{owner}/{repo}/pull/{pr}?run={run_id}``. The - webview re-derives the head SHA, base SHA, and artifact name from the workflow - run's uploaded artifact (+ its metadata.json), so the link carries only the PR - number and the run id — short, and stable across re-runs. + A stable GitHub-style short link: ``{base}/{owner}/{repo}/pull/{pr}``. The + webview resolves the latest uploaded analysis artifact for the repository and + PR, so the link remains unchanged across workflow runs. """ - if not (webview_base and owner and repo and pr and run_id): + if not (webview_base and owner and repo and pr): return None base = webview_base.rstrip("/") - return f"{base}/{owner}/{repo}/pull/{pr}?{urlencode({'run': run_id})}" + return f"{base}/{owner}/{repo}/pull/{pr}" def _join_or(items: list[str]) -> str: @@ -100,7 +98,6 @@ def build_cta( *, webview_base: str = "", webview_ready: bool = False, - run_id: str = "", ) -> str: """Return the markdown CTA footer: a health-warning banner plus an editor link. @@ -111,8 +108,8 @@ def build_cta( The ⚠️ banner shows whenever ``issues > 0``. When ``webview_ready`` an "explore in browser" line deep-links the hosted webview - to this PR's diff (``/owner/repo/pull/?run=``); the webview re-derives - the head/base/artifact from the run, so only the PR number and run id are needed. + to this PR's diff (``/owner/repo/pull/``); the webview resolves the latest + artifact for the repository and PR. """ parts: list[str] = [] if issues > 0: @@ -138,7 +135,7 @@ def link(path: str, **extra: str) -> str: # "in your browser or VS Code" / "in VS Code". targets: list[str] = [] if webview_ready: - wv = webview_url(webview_base, owner, repo, pr=pr, run_id=run_id) + wv = webview_url(webview_base, owner, repo, pr=pr) if wv: targets.append(f"your [**browser**]({wv})") targets += [f"[**{_EDITOR_LABEL[e]}**]({editor_href[e]})" for e in editors] @@ -161,7 +158,6 @@ def main() -> int: p.add_argument("--repo-path", required=True, type=Path, help="Path to the analyzed repo checkout") p.add_argument("--issues", default="0", help="Real architecture-issue count (0 -> no warning banner)") p.add_argument("--webview-base", default="", help="Hosted webview base URL (e.g. https://app.codeboarding.org)") - p.add_argument("--run-id", default="", help="GitHub Actions run id containing the PR analysis artifact") p.add_argument( "--webview-ready", action="store_true", @@ -183,7 +179,6 @@ def main() -> int: issues, webview_base=args.webview_base, webview_ready=args.webview_ready, - run_id=args.run_id, ) ) return 0 diff --git a/tests/test_build_cta.py b/tests/test_build_cta.py index e439d51..a8f4da0 100644 --- a/tests/test_build_cta.py +++ b/tests/test_build_cta.py @@ -80,28 +80,22 @@ class TestWebviewUrl(unittest.TestCase): WV = "https://app.codeboarding.org" def test_url_is_github_style_pr_path(self): - url = bc.webview_url(self.WV, "Org", "Repo", pr="9", run_id="123") - self.assertEqual(url, "https://app.codeboarding.org/Org/Repo/pull/9?run=123") - - def test_url_carries_only_pr_path_and_run(self): - # Head/base SHAs and the artifact name/url are re-derived by the webview, so - # none of them appear in the short link. - url = bc.webview_url(self.WV, "o", "r", pr="9", run_id="123") - self.assertIn("/o/r/pull/9", url) - self.assertIn("run=123", url) - self.assertNotIn("ref=", url) - self.assertNotIn("compare=", url) - self.assertNotIn("artifact", url) - self.assertNotIn("repo=o%2Fr", url) # not the old query-style link - - def test_url_none_without_pr_or_run(self): - self.assertIsNone(bc.webview_url(self.WV, "o", "r", pr="9")) # no run - self.assertIsNone(bc.webview_url(self.WV, "o", "r", run_id="123")) # no pr - self.assertIsNone(bc.webview_url("", "o", "r", pr="9", run_id="123")) # no base + url = bc.webview_url(self.WV, "Org", "Repo", pr="9") + self.assertEqual(url, "https://app.codeboarding.org/Org/Repo/pull/9") + + def test_url_carries_only_stable_pr_path(self): + url = bc.webview_url(self.WV, "o", "r", pr="9") + self.assertEqual(url, "https://app.codeboarding.org/o/r/pull/9") + self.assertNotIn("?", url) + self.assertNotIn("run", url) + + def test_url_none_without_pr_or_base(self): + self.assertIsNone(bc.webview_url(self.WV, "o", "r")) # no pr + self.assertIsNone(bc.webview_url("", "o", "r", pr="9")) # no base def test_trailing_slash_in_webview_base_is_normalized(self): - a = bc.webview_url("https://app.codeboarding.org/", "o", "r", pr="9", run_id="1") - b = bc.webview_url("https://app.codeboarding.org", "o", "r", pr="9", run_id="1") + a = bc.webview_url("https://app.codeboarding.org/", "o", "r", pr="9") + b = bc.webview_url("https://app.codeboarding.org", "o", "r", pr="9") self.assertEqual(a, b) self.assertNotIn(".org//", a) @@ -115,11 +109,11 @@ def test_cta_includes_browser_link_when_ready(self): issues=0, webview_base=self.WV, webview_ready=True, - run_id="123", ) self.assertIn("Explore this PR", out) self.assertIn("your [**browser**](", out) - self.assertIn("/Org/Repo/pull/9?run=123", out) + self.assertIn("https://app.codeboarding.org/Org/Repo/pull/9", out) + self.assertNotIn("?run=", out) self.assertIn("VS Code", out) # editor merged into the same line def test_cta_omits_browser_link_when_not_ready(self): @@ -133,7 +127,6 @@ def test_cta_omits_browser_link_when_not_ready(self): issues=0, webview_base=self.WV, webview_ready=False, - run_id="123", ) self.assertNotIn("/pull/", out) # no browser link self.assertNotIn("[**browser**]", out) @@ -150,7 +143,6 @@ def test_cta_omits_browser_link_when_ready_but_no_base_url(self): issues=0, webview_base="", webview_ready=True, - run_id="123", ) self.assertNotIn("[**browser**]", out) self.assertNotIn("/pull/", out) @@ -167,7 +159,7 @@ class TestMergedExploreLine(unittest.TestCase): WV = "https://app.codeboarding.org" def _ready(self, repo, cta=""): - return bc.build_cta(cta, "o", "r", "1", repo, webview_base=self.WV, webview_ready=True, run_id="123") + return bc.build_cta(cta, "o", "r", "1", repo, webview_base=self.WV, webview_ready=True) def test_browser_and_single_editor_joined_with_or(self): out = self._ready(repo_with()) # default VS Code