fix(#812 follow-up): normalize detect-platform.sh host-match port comparison by scheme #859

Merged

jason.woltje merged 1 commits from fix/850-detect-platform-port-normalization into main 2026-07-20 10:13:30 +00:00

Conversation 2 Commits 1 Files Changed 3 +38 -11

jason.woltje commented 2026-07-20 09:43:59 +00:00

Owner

Copy Link

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://git@git.example: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

• pnpm typecheck
• pnpm lint
• pnpm format:check
• pnpm test
• shellcheck -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.

## 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://git@git.example: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.

jason.woltje added 1 commit 2026-07-20 09:44:00 +00:00

fix(#812 follow-up): normalize detect-platform.sh host-match port comparison by scheme ... dc5abb8524

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

jason.woltje force-pushed fix/850-detect-platform-port-normalization from dc5abb8524 to 9c26278c67 2026-07-20 09:52:31 +00:00 Compare

jason.woltje commented 2026-07-20 10:03:44 +00:00

Author

Owner

Copy Link

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.

### 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.

jason.woltje commented 2026-07-20 10:13:23 +00:00

Author

Owner

Copy Link

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 9c26278c67.
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 = 9c26278c67 (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).

**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).

jason.woltje merged commit 344d86a635 into main 2026-07-20 10:13:30 +00:00

jason.woltje referenced this issue from a commit 2026-07-20 10:13:31 +00:00

fix(#812 follow-up): normalize detect-platform.sh host-match port comparison by scheme (#859)

jason.woltje referenced this pull request 2026-07-20 10:29:35 +00:00

fix(mosaic): de-flake mutator-class lease gate TTL-expiry test #861

jason.woltje referenced this pull request 2026-07-20 10:32:40 +00:00

fix(mosaic): de-flake mutator-class lease gate TTL-expiry test #861

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

fleet-enhancement

Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

jason.woltje

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#859