gitea_url_matches_host (packages/mosaic/framework/tools/git/detect-platform.sh) compared every remote authority's port directly against the configured HTTP(S) API URL's port. This conflated two unrelated things and normalized default web ports asymmetrically:
SSH transport port vs. HTTP(S) provider port: a remote ssh://git@host:2222/owner/repo.git resolves host host:2222. Comparing that transport port against a configured https://host API base failed host-match even though both name the same Gitea host -- the SSH daemon port has nothing to do with the HTTP(S) API port.
Asymmetric default-port normalization: https://host:443/owner/repo.git (explicit default port) failed to match configured https://host (implicit), while the inverse (implicit remote vs. explicit configured) already matched. The old code only applied the "portless == default port" equivalence when the remote side was portless.
Fix
get_remote_host() now strips an SSH transport port before it ever reaches gitea_url_matches_host -- it identifies an unrelated service (the SSH daemon) on the same host, not the HTTP(S) provider port.
gitea_url_matches_host's default-port normalization is now symmetric: whichever side (configured or remote) omits the port is treated as carrying the scheme's default port (:443 for https, :80 for http), so both directions of implicit/explicit match.
All existing #812 cases (subpath-slug, non-default HTTP port, SSH normal-form, root-mount/implicit-443) are preserved and still pass.
Tests
Extended packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh (the existing #812 harness) with two red-first regressions:
ssh-transport-port-success: ssh://[email protected]:2222/owner/repo.git remote against configured https://git.example
explicit-default-port-success: https://git.example:443/owner/repo.git remote against configured https://git.example
Both reproduced RED against the pre-fix script and are GREEN at this PR's head. Wired the harness into packages/mosaic's test:framework-shell chain (it previously existed but wasn't run by pnpm test), so it now runs under CI's test step.
Author lane: ms-850-build. NO self-merge -- awaiting named-executor merge clearance.
## Summary
`gitea_url_matches_host` (packages/mosaic/framework/tools/git/detect-platform.sh) compared every remote authority's port directly against the configured HTTP(S) API URL's port. This conflated two unrelated things and normalized default web ports asymmetrically:
- **SSH transport port vs. HTTP(S) provider port**: a remote `ssh://git@host:2222/owner/repo.git` resolves host `host:2222`. Comparing that transport port against a configured `https://host` API base failed host-match even though both name the same Gitea host -- the SSH daemon port has nothing to do with the HTTP(S) API port.
- **Asymmetric default-port normalization**: `https://host:443/owner/repo.git` (explicit default port) failed to match configured `https://host` (implicit), while the inverse (implicit remote vs. explicit configured) already matched. The old code only applied the "portless == default port" equivalence when the *remote* side was portless.
## Fix
- `get_remote_host()` now strips an SSH transport port before it ever reaches `gitea_url_matches_host` -- it identifies an unrelated service (the SSH daemon) on the same host, not the HTTP(S) provider port.
- `gitea_url_matches_host`'s default-port normalization is now symmetric: whichever side (configured or remote) omits the port is treated as carrying the scheme's default port (`:443` for https, `:80` for http), so both directions of implicit/explicit match.
All existing #812 cases (subpath-slug, non-default HTTP port, SSH normal-form, root-mount/implicit-443) are preserved and still pass.
## Tests
Extended `packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh` (the existing #812 harness) with two red-first regressions:
- `ssh-transport-port-success`: `ssh://[email protected]:2222/owner/repo.git` remote against configured `https://git.example`
- `explicit-default-port-success`: `https://git.example:443/owner/repo.git` remote against configured `https://git.example`
Both reproduced RED against the pre-fix script and are GREEN at this PR's head. Wired the harness into `packages/mosaic`'s `test:framework-shell` chain (it previously existed but wasn't run by `pnpm test`), so it now runs under CI's `test` step.
## Test plan
- [x] `pnpm typecheck`
- [x] `pnpm lint`
- [x] `pnpm format:check`
- [x] `pnpm test`
- [x] `shellcheck -x packages/mosaic/framework/tools/git/detect-platform.sh` (0 findings)
- [x] `shellcheck -x packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh` (0 findings)
Closes #850
Author lane: ms-850-build. NO self-merge -- awaiting named-executor merge clearance.
gitea_url_matches_host compared every remote authority's port directly
against the configured HTTP(S) API URL's port, which conflated two
unrelated things and normalized default web ports asymmetrically:
- An SSH remote's transport port (e.g. ssh://git@host:2222/...) was
compared against the configured HTTP(S) port, even though the SSH
daemon port has nothing to do with the Gitea HTTP(S) API port.
- An explicit default port on the remote (https://host:443/...) failed
to match an implicit (portless) configured URL, while the inverse
(implicit remote vs. explicit configured) already matched -- the
default-port equivalence was only applied on one side.
Fix get_remote_host() to strip an SSH transport port before it ever
reaches host-match, and make gitea_url_matches_host's default-port
normalization symmetric (":443" https / ":80" http is equivalent to
the implicit form on BOTH sides).
Adds red-first regressions for both repros to
test-pr-review-gitea-comment.sh (extending the #812 harness) and wires
that harness into packages/mosaic's test:framework-shell chain so it
runs under `pnpm test`.
Closes#850
Durable exact-head Reviewer-of-Record — APPROVE (do not merge without coordinator named-executor clearance)
Exact head bound:9c26278c67245fbad886164ee6ebe1337b4a51ff (the freshly REBASED head onto main acd7d380f63d192f034266b97f02dd4c1054247e; confirmed unchanged at review start AND end — no drift). All pre-rebase evidence (old head dc5abb85, CI #1946, any prior review) is VOID by the head move.
Independent CODE review — author lane ms-850-build != reviewer lane ms-859-review (distinct network-capable lane; strictly READ-ONLY — Gitea API GETs + a throwaway clone at /home/hermes/ms-859-review, since removed; no commits/pushes/tea-approve/PR-comments/merges via any tool). Persisted here as a durable comment. VERDICT: APPROVE.
Scope (verified git diff --stat vs merge-base acd7d380 AND /pulls/859/files): exactly 3 files, all framework-tooling — packages/mosaic/framework/tools/git/detect-platform.sh (+18/-8), packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh (+19/-2), packages/mosaic/package.json (+1/-1). Zero production application code (apps/, packages/db, packages/gateway).
Rebase integrity (critical):test:framework-shell diff is an append-only UNION — every pre-existing test preserved verbatim (daemon_deadline_unittest.py … check-runtime-launches.py … test-pr-diff-context.sh … test-deps-preflight.sh from #858) with && bash framework/tools/git/test-pr-review-gitea-comment.sh (#850) appended. test-pr-diff-context.sh NOT dropped (present in diff AND in the pnpm test run output). detect-platform.sh + test-pr-review-gitea-comment.sh diffs vs merge-base match the intended #850 fix exactly — no drift, no weakening from the pre-rebase version.
Fix correctness: Python gitea_url_matches_host normalizes port symmetrically (default_port = 80 if http else 443; both configured + remote fall back to it when None, then compared). Hostname mismatch is rejected BEFORE port normalization, so mismatched hosts still fail closed regardless of port. get_remote_host SSH branch strips the transport port (${host%%:*}) before it reaches the HTTP(S) port comparison, with an explanatory comment (SSH daemon port != HTTP(S) provider port). Edge cases (implicit vs explicit :443, ssh://host:2222, mismatched-host rejection) verified empirically.
Red-first genuine (independently reproduced): reviewer checked out the merge-base detect-platform.sh into the tree and ran the harness → exit 1 (fails at ssh-transport-port-success, confirmed via bash -x); isolated explicit-default-port-success also exit 1 pre-fix. Restored fixed script (tree clean, exact match to HEAD) → pr-review.sh durable Gitea comment regression passed, exit 0. No tautological tests; only the two case pattern lines edited + two new test blocks appended — no existing assertion removed/altered.
Integrity: no --no-verify, no .only/.skip, no removed/weakened assertions (grep-confirmed), no commented-out logic, no mocking around a real failure. Pre-existing tea/curl harness stubs unmodified.
Gates (reviewer re-ran, verbatim):pnpm typecheck 42/42; pnpm lint 23/23; pnpm format:check all-Prettier-clean; pnpm test 43/43 (mosaic includes BOTH #850 regressions + #858deps-preflight regression passed (5/5) + #812test-pr-diff-context.sh output — all green); shellcheck -x on both shell files = 0 findings.
Injection/shell safety: args passed as Python sys.argv (not string-interpolated); SSH-port strip via parameter expansion (no eval/subshell construction). No injection risk introduced.
Authoritative provider CI: Woodpecker pipeline #1947 (pull_request @ 9c26278c) = success, all 9 steps green (clone, ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, format, test) — CI-SHA == reviewed-SHA == exact head, all full-40.
Non-blocking notes: none beyond pre-existing turbo "no output files found" cache-config warnings, unrelated to this PR.
Author != reviewer preserved (author ms-850-build, reviewer ms-859-review). Merge authority is the coordinator's (Mos) — NO self-merge; awaiting a NAMED-EXECUTOR clearance.
### Durable exact-head Reviewer-of-Record — APPROVE (do not merge without coordinator named-executor clearance)
**Exact head bound:** `9c26278c67245fbad886164ee6ebe1337b4a51ff` (the freshly REBASED head onto main `acd7d380f63d192f034266b97f02dd4c1054247e`; confirmed unchanged at review start AND end — no drift). All pre-rebase evidence (old head `dc5abb85`, CI #1946, any prior review) is VOID by the head move.
**Independent CODE review** — author lane `ms-850-build` != reviewer lane `ms-859-review` (distinct network-capable lane; strictly READ-ONLY — Gitea API GETs + a throwaway clone at `/home/hermes/ms-859-review`, since removed; no commits/pushes/`tea`-approve/PR-comments/merges via any tool). Persisted here as a durable comment. **VERDICT: APPROVE.**
- **Scope (verified `git diff --stat` vs merge-base `acd7d380` AND `/pulls/859/files`):** exactly 3 files, all framework-tooling — `packages/mosaic/framework/tools/git/detect-platform.sh` (+18/-8), `packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh` (+19/-2), `packages/mosaic/package.json` (+1/-1). Zero production application code (`apps/`, `packages/db`, `packages/gateway`).
- **Rebase integrity (critical):** `test:framework-shell` diff is an append-only UNION — every pre-existing test preserved verbatim (`daemon_deadline_unittest.py` … `check-runtime-launches.py` … `test-pr-diff-context.sh` … `test-deps-preflight.sh` from #858) with `&& bash framework/tools/git/test-pr-review-gitea-comment.sh` (#850) appended. `test-pr-diff-context.sh` NOT dropped (present in diff AND in the `pnpm test` run output). `detect-platform.sh` + `test-pr-review-gitea-comment.sh` diffs vs merge-base match the intended #850 fix exactly — no drift, no weakening from the pre-rebase version.
- **Fix correctness:** Python `gitea_url_matches_host` normalizes port symmetrically (`default_port = 80 if http else 443`; both configured + remote fall back to it when `None`, then compared). Hostname mismatch is rejected BEFORE port normalization, so mismatched hosts still fail closed regardless of port. `get_remote_host` SSH branch strips the transport port (`${host%%:*}`) before it reaches the HTTP(S) port comparison, with an explanatory comment (SSH daemon port != HTTP(S) provider port). Edge cases (implicit vs explicit :443, ssh://host:2222, mismatched-host rejection) verified empirically.
- **Red-first genuine (independently reproduced):** reviewer checked out the merge-base `detect-platform.sh` into the tree and ran the harness → exit 1 (fails at `ssh-transport-port-success`, confirmed via `bash -x`); isolated `explicit-default-port-success` also exit 1 pre-fix. Restored fixed script (tree clean, exact match to HEAD) → `pr-review.sh durable Gitea comment regression passed`, exit 0. No tautological tests; only the two `case` pattern lines edited + two new test blocks appended — no existing assertion removed/altered.
- **Integrity:** no `--no-verify`, no `.only`/`.skip`, no removed/weakened assertions (grep-confirmed), no commented-out logic, no mocking around a real failure. Pre-existing `tea`/`curl` harness stubs unmodified.
- **Gates (reviewer re-ran, verbatim):** `pnpm typecheck` 42/42; `pnpm lint` 23/23; `pnpm format:check` all-Prettier-clean; `pnpm test` 43/43 (mosaic includes BOTH #850 regressions + #858 `deps-preflight regression passed (5/5)` + #812 `test-pr-diff-context.sh` output — all green); `shellcheck -x` on both shell files = 0 findings.
- **Injection/shell safety:** args passed as Python `sys.argv` (not string-interpolated); SSH-port strip via parameter expansion (no `eval`/subshell construction). No injection risk introduced.
- **Authoritative provider CI:** Woodpecker pipeline **#1947** (pull_request @ `9c26278c`) = **success**, all 9 steps green (clone, ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, format, test) — CI-SHA == reviewed-SHA == exact head, all full-40.
**Non-blocking notes:** none beyond pre-existing turbo "no output files found" cache-config warnings, unrelated to this PR.
Author != reviewer preserved (author `ms-850-build`, reviewer `ms-859-review`). Merge authority is the coordinator's (Mos) — NO self-merge; awaiting a NAMED-EXECUTOR clearance.
MOS NAMED-EXECUTOR PRE-MUTATION PROVENANCE — PR #859 (Closes#850)
Executor lane: Mos (main orchestrator, merge-authority). About to squash-merge PR #859 at full-40 head 9c26278c67245fbad886164ee6ebe1337b4a51ff after primary-source 6-check re-verification.
Identity (measured this session, not relayed):
host: web1
tmux pane: mos-claude:0.0 (pane_pid 2312433, claude child 2312511)
Two-lead concordant relays: MS-LEAD 100429Z + homelab 100602Z. Shared jason.woltje token means merged_by is structurally unattributable; this comment is the accountability anchor (xref framework #855).
**MOS NAMED-EXECUTOR PRE-MUTATION PROVENANCE — PR #859 (Closes #850)**
Executor lane: **Mos** (main orchestrator, merge-authority). About to squash-merge PR #859 at full-40 head `9c26278c67245fbad886164ee6ebe1337b4a51ff` after primary-source 6-check re-verification.
Identity (measured this session, not relayed):
- host: `web1`
- tmux pane: `mos-claude:0.0` (pane_pid `2312433`, claude child `2312511`)
- session_uuid: `e3f67fa7-ed83-4635-9269-c3d0aea10f8e`
6-check (Mos primary-source, not merge-on-relay):
1. PR open/unmerged, mergeable=true, base=main, head 9c26278c67245fbad886164ee6ebe1337b4a51ff.
2. CI Woodpecker #1947 (pull_request) terminal=success @9c26278c, 9/9 steps green.
3. Durable RoR comment 18406 APPROVE bound to exact head, author ms-850-build != reviewer ms-859-review, no REQUEST-CHANGES.
4. reviewed-SHA == CI-SHA == merge head, all full-40 = 9c26278c67245fbad886164ee6ebe1337b4a51ff (prospective tree 75163e5e, pure-forward from main acd7d380).
5. Body carries Closes #850.
6. Queue-guard --purpose merge exit 0.
Two-lead concordant relays: MS-LEAD 100429Z + homelab 100602Z. Shared jason.woltje token means merged_by is structurally unattributable; this comment is the accountability anchor (xref framework #855).
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
gitea_url_matches_host(packages/mosaic/framework/tools/git/detect-platform.sh) compared every remote authority's port directly against the configured HTTP(S) API URL's port. This conflated two unrelated things and normalized default web ports asymmetrically:ssh://git@host:2222/owner/repo.gitresolves hosthost:2222. Comparing that transport port against a configuredhttps://hostAPI base failed host-match even though both name the same Gitea host -- the SSH daemon port has nothing to do with the HTTP(S) API port.https://host:443/owner/repo.git(explicit default port) failed to match configuredhttps://host(implicit), while the inverse (implicit remote vs. explicit configured) already matched. The old code only applied the "portless == default port" equivalence when the remote side was portless.Fix
get_remote_host()now strips an SSH transport port before it ever reachesgitea_url_matches_host-- it identifies an unrelated service (the SSH daemon) on the same host, not the HTTP(S) provider port.gitea_url_matches_host's default-port normalization is now symmetric: whichever side (configured or remote) omits the port is treated as carrying the scheme's default port (:443for https,:80for http), so both directions of implicit/explicit match.All existing #812 cases (subpath-slug, non-default HTTP port, SSH normal-form, root-mount/implicit-443) are preserved and still pass.
Tests
Extended
packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh(the existing #812 harness) with two red-first regressions:ssh-transport-port-success:ssh://[email protected]:2222/owner/repo.gitremote against configuredhttps://git.exampleexplicit-default-port-success:https://git.example:443/owner/repo.gitremote against configuredhttps://git.exampleBoth reproduced RED against the pre-fix script and are GREEN at this PR's head. Wired the harness into
packages/mosaic'stest:framework-shellchain (it previously existed but wasn't run bypnpm test), so it now runs under CI'steststep.Test plan
pnpm typecheckpnpm lintpnpm format:checkpnpm testshellcheck -x packages/mosaic/framework/tools/git/detect-platform.sh(0 findings)shellcheck -x packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh(0 findings)Closes #850
Author lane: ms-850-build. NO self-merge -- awaiting named-executor merge clearance.
dc5abb8524to9c26278c67Durable exact-head Reviewer-of-Record — APPROVE (do not merge without coordinator named-executor clearance)
Exact head bound:
9c26278c67245fbad886164ee6ebe1337b4a51ff(the freshly REBASED head onto mainacd7d380f63d192f034266b97f02dd4c1054247e; confirmed unchanged at review start AND end — no drift). All pre-rebase evidence (old headdc5abb85, CI #1946, any prior review) is VOID by the head move.Independent CODE review — author lane
ms-850-build!= reviewer lanems-859-review(distinct network-capable lane; strictly READ-ONLY — Gitea API GETs + a throwaway clone at/home/hermes/ms-859-review, since removed; no commits/pushes/tea-approve/PR-comments/merges via any tool). Persisted here as a durable comment. VERDICT: APPROVE.git diff --statvs merge-baseacd7d380AND/pulls/859/files): exactly 3 files, all framework-tooling —packages/mosaic/framework/tools/git/detect-platform.sh(+18/-8),packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh(+19/-2),packages/mosaic/package.json(+1/-1). Zero production application code (apps/,packages/db,packages/gateway).test:framework-shelldiff is an append-only UNION — every pre-existing test preserved verbatim (daemon_deadline_unittest.py…check-runtime-launches.py…test-pr-diff-context.sh…test-deps-preflight.shfrom #858) with&& bash framework/tools/git/test-pr-review-gitea-comment.sh(#850) appended.test-pr-diff-context.shNOT dropped (present in diff AND in thepnpm testrun output).detect-platform.sh+test-pr-review-gitea-comment.shdiffs vs merge-base match the intended #850 fix exactly — no drift, no weakening from the pre-rebase version.gitea_url_matches_hostnormalizes port symmetrically (default_port = 80 if http else 443; both configured + remote fall back to it whenNone, then compared). Hostname mismatch is rejected BEFORE port normalization, so mismatched hosts still fail closed regardless of port.get_remote_hostSSH branch strips the transport port (${host%%:*}) before it reaches the HTTP(S) port comparison, with an explanatory comment (SSH daemon port != HTTP(S) provider port). Edge cases (implicit vs explicit :443, ssh://host:2222, mismatched-host rejection) verified empirically.detect-platform.shinto the tree and ran the harness → exit 1 (fails atssh-transport-port-success, confirmed viabash -x); isolatedexplicit-default-port-successalso exit 1 pre-fix. Restored fixed script (tree clean, exact match to HEAD) →pr-review.sh durable Gitea comment regression passed, exit 0. No tautological tests; only the twocasepattern lines edited + two new test blocks appended — no existing assertion removed/altered.--no-verify, no.only/.skip, no removed/weakened assertions (grep-confirmed), no commented-out logic, no mocking around a real failure. Pre-existingtea/curlharness stubs unmodified.pnpm typecheck42/42;pnpm lint23/23;pnpm format:checkall-Prettier-clean;pnpm test43/43 (mosaic includes BOTH #850 regressions + #858deps-preflight regression passed (5/5)+ #812test-pr-diff-context.shoutput — all green);shellcheck -xon both shell files = 0 findings.sys.argv(not string-interpolated); SSH-port strip via parameter expansion (noeval/subshell construction). No injection risk introduced.9c26278c) = success, all 9 steps green (clone, ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, format, test) — CI-SHA == reviewed-SHA == exact head, all full-40.Non-blocking notes: none beyond pre-existing turbo "no output files found" cache-config warnings, unrelated to this PR.
Author != reviewer preserved (author
ms-850-build, reviewerms-859-review). Merge authority is the coordinator's (Mos) — NO self-merge; awaiting a NAMED-EXECUTOR clearance.MOS NAMED-EXECUTOR PRE-MUTATION PROVENANCE — PR #859 (Closes #850)
Executor lane: Mos (main orchestrator, merge-authority). About to squash-merge PR #859 at full-40 head
9c26278c67245fbad886164ee6ebe1337b4a51ffafter primary-source 6-check re-verification.Identity (measured this session, not relayed):
web1mos-claude:0.0(pane_pid2312433, claude child2312511)e3f67fa7-ed83-4635-9269-c3d0aea10f8e6-check (Mos primary-source, not merge-on-relay):
9c26278c67.9c26278c67(prospective tree75163e5e, pure-forward from mainacd7d380).Two-lead concordant relays: MS-LEAD 100429Z + homelab 100602Z. Shared jason.woltje token means merged_by is structurally unattributable; this comment is the accountability anchor (xref framework #855).