feat(deploy): B13 deploy tags with the F-17 snapshot terminator fix and B14 closure record - #473
Merged
Merged
Conversation
Owner directive (2026-08-05): deployments to the canonical target must be tagged, name format 日期+timer ticker+序號. - Get-DeployTagName: pure builder, deploy-<yyyyMMdd>-<UtcTicks>-<NNN>, sequence 1..999 enforced. - New-RemoteDeployTag: annotated tag on the commit the TARGET actually checked out (parsed from the remote transcript's "HEAD is now at", resolved to the full sha operator-side), sequence from the day's existing tags after a tags fetch, collision retries with the next number, and a failed push is a hard error - a tag origin never saw would silently lie. Tag name and message carry no host/account/network detail (policy A). Git is driven only through an injectable runner. - Invoke-RemoteTestDeployRebuild tags on EXIT=0 non-dry-run only, returns DeployTag; an unresolvable deployed sha downgrades to a warning rather than failing a deployment that succeeded. - plan B13 recorded; tests cover the exact name shape, sequence bounds, collision retry, push discipline, and sha validation. The existing dispatch fixture already exercises the no-sha downgrade path. Verified: test-remote-deploy-transport passes (including the pre-existing assignments); lib parses clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GV5yjuGdAgJ9PYPRam6biV
Supersedes the suites-only closure attempt (#471, closed): the ledger entry stays open until an actual remote rebuild through the private-inventory path plus the ordered 12-command contract all pass. Remote target.local.json is owner-provisioned out-of-band only - transport never uploads private topology. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GV5yjuGdAgJ9PYPRam6biV
The compressed snapshot JSON is written without a trailing newline, so the remote template's cat glued the end marker onto the JSON line; the operator-side parser never matched and every successful real rebuild was rejected as "no effective env snapshot section". The template now emits a bare echo after cat, the parse moves into ConvertFrom-DeployEnvSnapshotTranscript (single implementation), and regression tests pin the real byte shape the fake-ssh transcript cannot reproduce. Also restores the backtick-escaped newline split in New-RemoteDeployTag's sequence counting: the committed literal-newline regex never split the tag list, so the B13 collision-retry test failed at baseline. Plan records F-17 (new 4.4.2), checks off B14 with the #472 fixpoint closure, and marks #467/#472 done in the sequence table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6
Contributor
📝 WalkthroughWalkthroughThe remote deployment flow now separates the effective-environment snapshot from its end marker, parses the transcript through a dedicated function, and creates annotated deployment tags for successful non-dry-run deployments. Tests cover parsing, naming, collision handling, push failures, and SHA validation. ChangesRemote deployment flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/lib/remote-deploy-transport.ps1`:
- Around line 515-529: Update scripts/lib/remote-deploy-transport.ps1 lines
515-529 to consume a machine-readable deployed-SHA marker from the remote
script, fetch and resolve that commit before calling New-RemoteDeployTag, and
gate tag creation to the registry canonical target only; throw instead of
warning when canonical SHA resolution or tag creation fails. Update
docs/plans/remote-linux-test-deploy-target.plan.md line 288 so B13 is not marked
complete until this contract is implemented.
- Line 522: Replace the bare Write-Host deployment-tag output in
scripts/lib/remote-deploy-transport.ps1 at lines 522-522 with the structured
logging mechanism from scripts/lib/StructLog.psm1. In
scripts/tests/test-remote-deploy-transport.ps1 at lines 254-254 and 299-299,
remove the F-17 and B13 status writes or route them through the same structured
logger; do not add bare Write-Host output.
- Around line 408-410: The failed-push path in New-RemoteDeployTag must delete
the locally created tag before throwing; update
scripts/lib/remote-deploy-transport.ps1 lines 408-410 to run tag -d $tagName,
report any cleanup failure together with the push failure, and then throw.
Update scripts/tests/test-remote-deploy-transport.ps1 lines 286-294 to record
Git calls and assert that tag -d <tagName> runs after a failed push.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bd73dd16-c4ac-49e0-bad9-782beee75d39
📒 Files selected for processing (3)
docs/plans/remote-linux-test-deploy-target.plan.mdscripts/lib/remote-deploy-transport.ps1scripts/tests/test-remote-deploy-transport.ps1
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fd685c2c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
🔵 Human review recommended
This is a Lane G deploy-path change that mutates git state (creates/pushes tags) and its central tag-creation wiring in Invoke-RemoteTestDeployRebuild is only verified live on the next real rebuild, warranting owner review.
Pull request overview
This PR is the first mechanism fix after the deploy-migration debt closure (per owner ruling "option A"). It fixes F-17, a deterministic transcript-parse defect where the compressed effective-env snapshot JSON is written without a trailing newline, so the remote template's end marker was glued onto the JSON line and the operator-side anchored regex could never match — causing a successful real rebuild to be wrongly rejected as "no effective env snapshot section." It also implements B13 deploy tagging (annotated deploy-<yyyyMMdd>-<UtcTicks>-<NNN> tags pushed to origin for successful non-dry-run deploys) and records the B14 closure in the plan doc.
Changes:
- F-17: emit a bare
echobetweencat "$SNAPSHOT_TMP"and the end marker so the marker starts its own line; extract the transcript parser into a singleConvertFrom-DeployEnvSnapshotTranscriptused by both live dispatch and tests. - B13: add pure
Get-DeployTagName(validated 1..999 sequence) andNew-RemoteDeployTag(fetch-then-count sequencing, collision retry, hard-error on failed push, injectable-GitRunner); wire tag creation intoInvoke-RemoteTestDeployRebuildbound to the sha the target actually checked out. - Regression tests pinning the real byte shape (pre/post-fix transcripts, tag naming/sequencing/collision/push discipline) and plan-doc updates closing B13/B14 and recording F-17.
File summaries
| File | Description |
|---|---|
| scripts/lib/remote-deploy-transport.ps1 | Adds the bare-echo F-17 template fix, the single snapshot-transcript parser, and B13 tag helpers plus tag creation in the rebuild flow. |
| scripts/tests/test-remote-deploy-transport.ps1 | Adds regression coverage pinning the F-17 byte shape and B13 tag naming, sequencing, collision retry, and push discipline. |
| docs/plans/remote-linux-test-deploy-target.plan.md | Records F-17 in §4.4.2, closes B13/B14 checklist items, and updates the §7 sequence table for #467/#472. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Review round (CodeRabbit + Codex connector + Copilot) on the B13 tag block: - a failed tag push now deletes the local tag before throwing (B13: never leave a local-only tag; a stale local tag would poison later sequence counts) - bootstrap rebuilds are never tagged - their evidence is never deploy-target evidence - tagging is restricted to the canonical target (role canonical_test_deploy, taken from the in-hand target object) and, once eligible, mandatory: a missing or unresolvable deployed sha is a hard error after one fetch retry, not a silently skipped record - the successful tag path is covered end-to-end by shadowing git next to the fake ssh (asserts the returned DeployTag and exactly one push), and a bootstrap dispatch asserts DeployTag stays empty Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6
Owner
Author
|
Review-round disposition (commit b51dc0c):
All transport tests pass locally on this head ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b51dc0c0fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![codex-tri-adversarial-bot[bot]](https://avatars.githubusercontent.com/u/311287868?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Tri-Adversarial Bot
Automated tri-adversarial ship-gate (L0 terra triage / L1 tier-routed lens fanout / L2 refute-by-default / L3 sol apex — Codex models).
Mapped event: COMMENT
Codex Tri-Adversarial ship-gate — PR #473
- Repo head:
feat/deploy-tags-b13-f17@b51dc0c - Base:
main@902f3a4 - Files changed: 3
- Engine: four-model tri-adversarial gate on Codex — L0 triage
gpt-5.6-terra/low; L1 lens finders routedgpt-5.6-terra/low →gpt-5.6-luna/medium →gpt-5.5/xhigh (security floorgpt-5.5); L2 refute-by-defaultgpt-5.5/xhigh, top-tier findings refuted bygpt-5.6-sol/xhigh (every refutation cross-model); L3 apexgpt-5.6-sol/max. 誠實聲明：層級與 Claude 三層 gate 同構（terra≈haiku、luna≈sonnet、gpt-5.5≈opus、sol≈fable），但模型池是 Codex 的，非 Anthropic 的。
Verdict
SHIP
- 阻擋門檻 severity:
critical, high - mapped GitHub event:
COMMENT - ℹ️ 判定為 SHIP，但刻意不送 APPROVE：GitHub App 的 approving review 不計入
required_approving_review_count（2026-07-31 實測）。本報告是證據，approving 那一票請由真人帳號投。
Difficulty & routing
- overall:
high(source: terra-triage) - lens tiers: correctness→
gpt-5.5, security→gpt-5.5, simplification→gpt-5.6-luna, test-gap→gpt-5.5
Layer stats
- L1: raw=10 deduped=10 finder_failures=0
- L2: confirmed=5 refuted=3 unverified=0
- L3 final: 5
Findings (final, after apex)
[medium] Remote transcript text controls the commit that receives the authoritative deploy tag
- id:
SEC-003lens:securityfile:scripts/lib/remote-deploy-transport.ps1line:529-543 - provenance: finder=
gpt-5.5refuter=gpt-5.6-sol(cross-model-guard) L2=confirmed - evidence: The code applies an unanchored first-match regex to the combined SSH transcript and passes the resulting SHA to New-RemoteDeployTag after only checking that it resolves locally. A targeted reproduction confirmed that a matching line placed before the genuine checkout line wins; a later forged line does not.
- why: A remote actor able to influence pre-checkout output can make the operator's local Git credentials push an authoritative governance tag for any locally resolvable commit. Hex validation and rev-parse prevent command injection but do not establish that the selected line came from the intended checkout operation.
- proposed fix: Emit one uniquely prefixed, exact-line checkout result immediately after checkout, reject missing or multiple markers, and verify the resulting full SHA against the expected fetched origin ref before tagging. Do not parse generic Git prose from the combined transcript.
[medium] Pushed deploy tag message includes unsanitized target and snapshot metadata
- id:
SEC-001lens:securityfile:scripts/lib/remote-deploy-transport.ps1line:400-407 - provenance: finder=
gpt-5.5refuter=gpt-5.6-sol(cross-model-guard) L2=confirmed - evidence: TargetId and SnapshotName are interpolated directly into an annotated tag message that is pushed to origin. Neither New-RemoteDeployTag nor the role-based eligibility predicate restricts Target.id; a targeted reproduction confirmed that CR/LF in TargetId creates additional annotation lines.
- why: Passing the message as one argv value prevents shell injection, but it does not protect the durable shared-origin record. A private-inventory target ID can accidentally disclose topology-derived data or inject misleading annotation fields, contradicting the stated metadata policy.
- proposed fix: Require a canonical public target identifier, reject control characters in every annotation field, and generate or validate the snapshot label from a restricted basename allowlist. Omit fields that cannot be proven public.
[medium] Mandatory tag failure paths are untested for eligible canonical deployments
- id:
TG-001lens:test-gapfile:scripts/tests/test-remote-deploy-transport.ps1 - provenance: finder=
gpt-5.5refuter=gpt-5.6-sol(cross-model-guard) L2=confirmed - evidence: The added live-dispatch test covers an eligible canonical success where rev-parse succeeds. No added test supplies a successful eligible transcript without HEAD is now at, or makes rev-parse fail both before and after the fetch retry.
- why: These throws enforce B13's mandatory-record contract. Replacing either with a warning or empty DeployTag would leave the shown happy-path and bootstrap tests passing, allowing successful canonical deployments to lose their required tag silently.
- proposed fix: Add eligible canonical live-dispatch tests for a missing checkout marker and for rev-parse failing before and after fetch; assert a hard failure and no tag creation or push.
[low] Canonical-target gating lacks a negative test for non-canonical successful deploys
- id:
TG-002lens:test-gapfile:scripts/tests/test-remote-deploy-transport.ps1 - provenance: finder=
gpt-5.5refuter=gpt-5.6-sol(cross-model-guard) L2=confirmed - evidence: The tests exercise a canonical role and a bootstrap exclusion, but no successful non-bootstrap target has a missing or non-canonical role. Removing the role predicate would therefore leave the shown tests passing.
- why: The role check is the boundary preventing ordinary successful targets from producing canonical governance tags. A focused negative test would pin that boundary without broadening the suite.
- proposed fix: Run a successful non-bootstrap dispatch with a missing or non-canonical role and assert an empty DeployTag and no tag push.
[low] Push-failure cleanup test cannot fail if cleanup itself fails
- id:
TG-004lens:test-gapfile:scripts/tests/test-remote-deploy-transport.ps1 - provenance: finder=
gpt-5.5refuter=gpt-5.6-sol(cross-model-guard) L2=confirmed - evidence: The push-failure fake returns ExitCode 1 only for push and success for tag -d, then asserts merely that deletion was attempted. It never reaches the cleanup-failure diagnostic branch.
- why: If deletion fails, the diagnostic is the only indication that a stale local tag remains and may affect later sequence counts. This is low severity because the core hard failure and cleanup attempt are already covered; only the recovery diagnostic branch is missing.
- proposed fix: Make tag -d fail in a second push-failure case and assert that the thrown message reports the cleanup failure while the overall tag operation still fails.
Killed (did not survive L2/L3)
L1-001[medium] Deploy tag message violates the documented no-target-detail contract — The documented contract forbids host/account/network details, not all target identifiers. The diff only shows$TargetId, with the concrete examplecanonical-linux, which is a logical label and does not itself disclose any prohibited detail. The finding relies on the unsupported conditional thatSEC-002[medium] Canonical deploy tag authorization trusts only a mutable target role field — The diff does not establish a security boundary around$Target, nor that a lower-trust actor can control itsrole. Mutability alone is insufficient: checking another identifier from the same object would be equally forgeable unless an independent trusted source is demonstrated. More importantly,SEC-004[low] Raw git failure output is surfaced in deploy-tag exceptions — The diff shows raw git diagnostics included in a locally thrown exception, but not a security-boundary violation. There is no evidence that the exception is exposed to an untrusted audience, persisted in public logs/PR evidence, or contains secrets.originis operator-configured, Git normally obta
Summary
All five survivors remain supported; TG-004 is downgraded from medium to low because the core push-failure and cleanup-attempt behavior is already tested. Before merge, bind the authoritative tag to an unambiguous checkout result, constrain shared annotation fields, and add negative coverage for the mandatory SHA and canonical-role gates. The cleanup-failure diagnostic test is worthwhile but non-blocking.
Agent calls
- 14/14 ok, engine wall-clock 706.1s
VERDICT
SHIP
VERDICT: SHIP
monkey1sai-blip
approved these changes
Aug 5, 2026
monkey1sai-blip
left a comment
monkey1sai-blip
left a comment
Collaborator
There was a problem hiding this comment.
Approved by monkey1sai-blip (the reviewer account pinned by the repo's merge governance).
Submitted through scripts/blip_review.py — a scripted approval carrying the operator's authority, pinned to head b51dc0c0fe026c408dbb6427f40c4e5b51281d32. This is the mechanism the GitHub App cannot satisfy: an App's approving review does not count toward required_approving_review_count.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-Compress＋WriteAllText寫出 effective-env snapshot JSON、無結尾換行，cat後 end-marker 與 JSON 併在同一行；operator 端 regex 要求\r?\n== effective env snapshot end ==，永不命中，成功的真實 rebuild 反而被誤判「no effective env snapshot section」拋錯。修法：模板cat後補 bareecho、parser 抽成ConvertFrom-DeployEnvSnapshotTranscript單一實作、regression 測試釘住真實 byte 形狀（既有 fake-ssh 測試以 PS 物件輸出、Out-String自動補換行，重現不了此形狀）。deploy-<yyyyMMdd>-<UtcTicks>-<NNN>並推送 origin；push 失敗為硬錯誤；DryRun 與失敗部署不打 tag。另修 tag 序號計數的行切割 bug：`r?`n反引號退化成字串內真實換行，LF-only tag 清單永遠切不開、當日序號恆為 1，B13 collision-retry 測試在 baseline 即紅。902f3a4關帳、ledger 零 open）、§7 序列表更新（feat(deploy): migrate the persistent test deploy area to remote Linux (PR-B) #467/fix(governance): close the deploy-migration debt with its rebuild-backed fixpoint #472 完成）、F-17 記入 plan §4.4.2。AI Coding Governance
Deploy Path Verification
.\scripts\deploy.ps1 -DryRun.\scripts\verify-all.ps1scripts/tests/test-remote-deploy-transport.ps1all assertions passed；.\scripts\deploy.ps1 -DryRunEXIT=0（Windows operator run 2026-08-05）Windows On-Demand Verification
.\scripts\deploy.ps1 -DryRunEXIT=0 on the Windows 11 operator workstation (2026-08-05), evidence bound to head b51dc0c; machine Windows run for this head: agent-governance (windows-latest, runs scripts/tests/test-remote-deploy-transport.ps1) pass - https://github.com/monkey1sai/AI-BIM-governance/actions/runs/30998193233;pwsh -NoProfile -NonInteractive -File scripts/tests/test-remote-deploy-transport.ps1all assertions passed（含新 F-17 regression＋B13 collision-retry）Self-Referential Bootstrap
Validation
pwsh -NoProfile -NonInteractive -File scripts/tests/test-remote-deploy-transport.ps1— all assertions passed（baseline 在 B13 collision-retry 斷言為紅；split 修復後轉綠；新增 F-17 regression 區塊通過）.\scripts\deploy.ps1 -DryRun— EXIT=0（Windows tierdeploy_dryrun）Known Risks
New-RemoteDeployTag在 fetch 失敗時設計上降級為本機 tag 計數；真正離線的 dispatch 可能選到遠端已用的序號並消耗一次 retry。Summary by CodeRabbit
Bug Fixes
New Features
Tests