59 lines
4.0 KiB
Markdown
59 lines
4.0 KiB
Markdown
# Issue #812 — durable Gitea PR review comments
|
|
|
|
- **Lane:** ms-812
|
|
- **Branch:** `fix/812-pr-review-comment`
|
|
- **Issue:** mosaicstack/stack#812
|
|
- **Budget:** 15K working estimate; single focused shell-wrapper/test/docs change.
|
|
|
|
## Objective
|
|
|
|
Make the Gitea `comment` action in `packages/mosaic/framework/tools/git/pr-review.sh` use the supported Gitea comments REST API and report success only after provider read-back verifies the created comment against the intended repository, PR, and exact body.
|
|
|
|
## Plan
|
|
|
|
1. Add and commit a failing shell regression harness before production changes.
|
|
2. Verify RED against the nonexistent `tea pr comment` fallback false-positive.
|
|
3. Implement the minimal supported write plus ID-based provider read-back.
|
|
4. Document that wrapper write output is not durable provenance until read-back succeeds.
|
|
5. Run focused regression tests, touched-package tests, and repository quality gates.
|
|
6. Remediate review findings, queue-guard, and push for coordinator-owned independent review. Do not open or merge a PR.
|
|
|
|
## Progress checkpoints
|
|
|
|
- [x] RED regression committed and reported to mosaic-100 (rebased commit `770e3f57`)
|
|
- [x] Initial minimal fix implemented (rebased commit `ea7f8c57`)
|
|
- [x] Rebased cleanly onto main `627cf2bb387f7c84a532d88819903a7679ce0d72`
|
|
- [x] Codex blocker remediated by replacing unsupported `tea api` with authenticated REST write/read-back
|
|
- [x] Focused, package, and repository gates green
|
|
- [ ] Coordinator-owned independent review pending after push
|
|
- [x] No PR opened; no self-review or self-merge
|
|
|
|
## Tests run
|
|
|
|
- RED after rebase: the regression harness failed against `origin/main` with status 1 after reproducing the old `tea pr comment` zero-exit fallback and false success echo.
|
|
- GREEN at resumed head: the same harness passed with REST POST 201 plus GET 200 read-back.
|
|
- All `packages/mosaic/framework/tools/git/test-*.sh` harnesses passed.
|
|
- `shellcheck -x` passed for the changed scripts; `bash -n` passed.
|
|
- Manifest resolver returned `framework` for `tools/git/test-pr-review-gitea-comment.sh`.
|
|
- `pnpm test` passed (43/43 Turbo tasks; Mosaic 75 files/1434 tests; Gateway 56 files/628 tests plus documented skips).
|
|
- `pnpm typecheck` passed (42/42 tasks), `pnpm lint` passed (23/23), and `pnpm format:check` passed.
|
|
- Firewall checks found no user-home paths or operator identities in changed shipped files; no token value is logged or echoed.
|
|
|
|
## Risks / blockers
|
|
|
|
- No active implementation blocker. #789 reached terminal merged state and the coordination hold was lifted.
|
|
- Review round 1 found one portability blocker: the API base reconstructed `https://$host` and discarded configured schemes/path prefixes.
|
|
- Review round 2 found a second subpath portability blocker: clone-derived `get_repo_slug` retained the deployment prefix, duplicating it under `/api/v1/repos/`.
|
|
- Round 3 resolves owner/repo relative to the configured Gitea base path for HTTP(S) clones while preserving root-mounted and SSH clone forms. Host matching now compares non-default ports consistently.
|
|
- REST transport failures, non-201 writes, malformed/missing created IDs, non-200 read-backs, and read-back mismatches all fail closed.
|
|
- Existing approve/request-changes behavior remains covered.
|
|
- Independent exact-head re-review remains coordinator-owned.
|
|
|
|
## Final verification evidence
|
|
|
|
- URL-portability regression was RED before remediation at the new `http://git.mosaicstack.dev` case and GREEN afterward.
|
|
- Round-3 genuine subpath regression was RED against round-2 head `1b190201` and GREEN after the fix: `https://git.example/gitea/owner/repo.git` maps to API repository `owner/repo` under configured base `/gitea`.
|
|
- Regression coverage verifies POST and read-back GET for root-mounted HTTP(S), path-prefixed HTTP(S), non-default HTTP port, scp-style SSH, and `ssh://` clone forms.
|
|
- Focused shell checks, all git-wrapper harnesses, and full repository test/typecheck/lint/format gates passed after remediation.
|
|
- Branch will be force-pushed with lease for coordinator re-verification; no PR opened.
|