Skip to content

fix: use stable PR route for webview links #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ivanmilevtues wants to merge 1 commit into
base: main
Choose a base branch
from
+31 −46
Open

fix: use stable PR route for webview links #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
8 changes: 3 additions & 5 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 }}
Comment on lines 1324 to 1325

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the run ID used by the comment footer

For every review-mode run, removing RUN_ID from this step's environment leaves the existing interpolation at action.yml:1413 unset. Because this script does not enable set -u, the step succeeds but every posted comment ends with codeboarding-action · run instead of the identifying GitHub run number; keep github.run_id available here even though the webview URL no longer needs it.

Useful? React with 👍 / 👎.

CHANGED: ${{ steps.diagram.outputs.changed }}
RENDERED: ${{ steps.diagram.outputs.rendered }}
Expand Down Expand Up @@ -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" \
Expand Down
25 changes: 10 additions & 15 deletions scripts/build_cta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/<pr>?run=<run_id>``) 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/<pr>``) 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
Expand Down Expand Up @@ -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:
Expand All @@ -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.

Expand All @@ -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/<pr>?run=<run_id>``); 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/<pr>``); the webview resolves the latest
artifact for the repository and PR.
"""
parts: list[str] = []
if issues > 0:
Expand All @@ -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]
Expand All @@ -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",
Expand All @@ -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
Expand Down
42 changes: 17 additions & 25 deletions tests/test_build_cta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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):
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down
Loading