fix(test): handle COPY --chmod in platform-agent drift gate (CI fix) #2990
@@ -13,87 +13,115 @@ name: template-delivery-e2e
|
||||
# /configs/plugins/seo-all/. The e2e asserts the skill arrives via the
|
||||
# PLUGIN channel and NOT the asset channel (negative control).
|
||||
#
|
||||
# STAGED ROLLOUT (do NOT make required until green):
|
||||
# Phase 1 (now): advisory — runs on the relevant paths + main + dispatch.
|
||||
# Asserts the new two-channel contract.
|
||||
# Phase 2 (after this goes green twice on the new contract): remove
|
||||
# continue-on-error and add this check to branch protection
|
||||
# required_status_checks so a future delivery regression is
|
||||
# merge-blocking.
|
||||
# STAGED ROLLOUT (now COMPLETE — this gate is merge-blocking):
|
||||
# Phase 1 (done): advisory — asserted the new two-channel contract.
|
||||
# Phase 2a (done, f6155d68): HARDENED the asset-channel assertions (C
|
||||
# config.yaml, D prompts) to poll within E2E_ASSET_SETTLE_SECS,
|
||||
# killing the false stub from a transient /configs read
|
||||
# (`curl: (28) ... 0 bytes`). Banked a green main run, which
|
||||
# lint-pre-flip-continue-on-error requires before the flip.
|
||||
# Phase 2b (THIS change, mc#2996): FLIP to merge-blocking —
|
||||
# • continue-on-error removed → a real delivery regression fails;
|
||||
# • `on: paths:` removed (required workflows must not be
|
||||
# path-filtered); path-scoping moved to the detect-changes job
|
||||
# (profile `template-delivery`) and applied per-step;
|
||||
# • the emitted context is added to .gitea/required-contexts.txt
|
||||
# and to branch-protection required_status_checks (as
|
||||
# "... (pull_request)") so a delivery PR cannot merge unless a
|
||||
# fresh seo-agent provisions and BOTH channels verify.
|
||||
#
|
||||
# Cost: provisions ONE throwaway tenant + ONE seo-agent (real EC2), teardown
|
||||
# trap deletes the org even on failure. Path-filtered so it only runs when the
|
||||
# delivery code actually changes.
|
||||
# trap deletes the org even on failure. The workflow has NO `on: paths:` filter:
|
||||
# a REQUIRED-check workflow must not carry one (lint-required-no-paths.py /
|
||||
# feedback_path_filtered_workflow_cant_be_required — a docs-only PR would never
|
||||
# emit the context, Gitea reports it `pending`, and the PR wedges forever).
|
||||
# Instead the detect-changes job applies the same path-scoping at RUNTIME, and
|
||||
# the delivery job's real steps are gated on its output: a non-delivery PR emits
|
||||
# SUCCESS cheaply (no provision), while a delivery PR runs the full e2e and
|
||||
# BLOCKS on failure. Mirrors the e2e-api / peer-visibility required-gate shape.
|
||||
|
||||
on:
|
||||
workflow_dispatch: {}
|
||||
push:
|
||||
branches: [main]
|
||||
paths:
|
||||
- 'workspace-server/internal/provisioner/template_assets.go'
|
||||
- 'workspace-server/internal/provisioner/gitea_template_assets.go'
|
||||
- 'workspace-server/internal/provisioner/cp_provisioner.go'
|
||||
- 'workspace-server/internal/handlers/platform_agent.go'
|
||||
- 'workspace-server/cmd/server/main.go'
|
||||
- 'workspace-server/internal/handlers/org_import.go'
|
||||
- 'workspace-server/internal/handlers/workspace.go'
|
||||
- 'workspace-server/internal/handlers/template_plugins.go'
|
||||
- 'workspace-server/internal/handlers/plugins_reconcile.go'
|
||||
- 'workspace-server/internal/handlers/registry.go'
|
||||
- 'workspace-server/internal/handlers/plugins_install_pipeline.go'
|
||||
- 'workspace-server/internal/handlers/plugins_tracking.go'
|
||||
- 'workspace-server/internal/plugins/source.go'
|
||||
- 'manifest.json'
|
||||
- 'tests/e2e/test_template_delivery_e2e.sh'
|
||||
- '.gitea/workflows/template-delivery-e2e.yml'
|
||||
pull_request:
|
||||
paths:
|
||||
- 'workspace-server/internal/provisioner/template_assets.go'
|
||||
- 'workspace-server/internal/provisioner/gitea_template_assets.go'
|
||||
- 'workspace-server/internal/provisioner/cp_provisioner.go'
|
||||
- 'workspace-server/internal/handlers/platform_agent.go'
|
||||
- 'workspace-server/cmd/server/main.go'
|
||||
- 'workspace-server/internal/handlers/org_import.go'
|
||||
- 'workspace-server/internal/handlers/workspace.go'
|
||||
- 'workspace-server/internal/handlers/template_plugins.go'
|
||||
- 'workspace-server/internal/handlers/plugins_reconcile.go'
|
||||
- 'workspace-server/internal/handlers/registry.go'
|
||||
- 'workspace-server/internal/handlers/plugins_install_pipeline.go'
|
||||
- 'workspace-server/internal/handlers/plugins_tracking.go'
|
||||
- 'workspace-server/internal/plugins/source.go'
|
||||
- 'manifest.json'
|
||||
- 'tests/e2e/test_template_delivery_e2e.sh'
|
||||
- '.gitea/workflows/template-delivery-e2e.yml'
|
||||
branches: [main]
|
||||
|
||||
concurrency:
|
||||
group: template-delivery-e2e-${{ github.ref }}
|
||||
cancel-in-progress: true
|
||||
group: template-delivery-e2e-${{ github.event.pull_request.head.sha || github.sha }}
|
||||
cancel-in-progress: false
|
||||
|
||||
jobs:
|
||||
# Job renamed for the RFC#2843 #32 two-channel contract (config+prompts via
|
||||
# the asset channel; seo-all installs via the post-online plugin reconcile,
|
||||
# not at boot). Renaming the job changes the emitted status context.
|
||||
# bp-exempt: advisory Phase-1 gate (continue-on-error, mc#2996) — informational, not a required BP context.
|
||||
# Runtime path-scoping (replaces the removed `on: paths:`). Mirrors the
|
||||
# e2e-api / peer-visibility detect-changes shape. Outputs `delivery=true`
|
||||
# when the diff touches the delivery surface; the gate job runs the real e2e
|
||||
# only then. bp-exempt: helper job; the REQUIRED context is the `delivery`
|
||||
# job below, not this one.
|
||||
detect-changes:
|
||||
runs-on: ubuntu-latest
|
||||
continue-on-error: false
|
||||
outputs:
|
||||
delivery: ${{ steps.decide.outputs.delivery }}
|
||||
debug: ${{ steps.decide.outputs.debug }}
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 0
|
||||
- id: decide
|
||||
env:
|
||||
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
|
||||
PUSH_BEFORE: ${{ github.event.before }}
|
||||
run: |
|
||||
python3 .gitea/scripts/detect-changes.py \
|
||||
--profile template-delivery \
|
||||
--event-name "${{ github.event_name }}" \
|
||||
--pr-base-sha "$PR_BASE_SHA" \
|
||||
--base-ref "$PR_BASE_REF" \
|
||||
--push-before "$PUSH_BEFORE" || {
|
||||
# Script crash → fail OPEN so the gate runs rather than silently
|
||||
# no-oping a potentially-breaking delivery PR.
|
||||
echo "delivery=true" >> "$GITHUB_OUTPUT"
|
||||
echo "debug=detect-script-error event=${{ github.event_name }}" >> "$GITHUB_OUTPUT"
|
||||
exit 0
|
||||
}
|
||||
echo "debug=profile=template-delivery event=${{ github.event_name }}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
# ONE job (no job-level `if:`) that ALWAYS runs and reports under the
|
||||
# required-check name. Real work is gated per-step on
|
||||
# `needs.detect-changes.outputs.delivery` — a non-delivery PR runs only the
|
||||
# no-op step and emits SUCCESS (branch-protection-clean; a job-level `if:`
|
||||
# would emit a SKIPPED check run that fails the required-check eval — see
|
||||
# e2e-api.yml's PR#2264 note). A delivery PR runs the full e2e and, with no
|
||||
# continue-on-error, BLOCKS the merge on failure.
|
||||
# bp-required: yes — mc#2996 / RFC#2843 #37: this context is merge-blocking;
|
||||
# branch protection lists "<this> (pull_request)".
|
||||
delivery:
|
||||
needs: detect-changes
|
||||
# No colon in the name — lint-required-context's PyYAML AST parse rejects an
|
||||
# unquoted scalar containing a colon.
|
||||
name: Template-asset delivery (fresh seo-agent — config+prompts via asset channel, seo-all via plugin reconcile)
|
||||
runs-on: ubuntu-latest
|
||||
# Phase 1: advisory. Remove this line in Phase 2 to make it merge-blocking.
|
||||
# mc#2996 — Phase 2 promotion tracker (remove continue-on-error; forced 14d renewal cadence).
|
||||
continue-on-error: true # mc#2996
|
||||
timeout-minutes: 30
|
||||
env:
|
||||
MOLECULE_CP_URL: ${{ vars.CP_URL || 'https://staging-api.moleculesai.app' }}
|
||||
MOLECULE_ADMIN_TOKEN: ${{ secrets.CP_ADMIN_API_TOKEN }}
|
||||
E2E_EXPECTED_MODEL: moonshot/kimi-k2.6
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- name: No-op pass (delivery surface unchanged in this diff)
|
||||
if: needs.detect-changes.outputs.delivery != 'true'
|
||||
run: |
|
||||
echo "No delivery-surface changes — gate satisfied without provisioning."
|
||||
echo "::notice::template-delivery-e2e no-op pass (detect-changes: ${{ needs.detect-changes.outputs.debug }})."
|
||||
- if: needs.detect-changes.outputs.delivery == 'true'
|
||||
uses: actions/checkout@v4
|
||||
- name: Verify required secret present
|
||||
if: needs.detect-changes.outputs.delivery == 'true'
|
||||
run: |
|
||||
if [ -z "${MOLECULE_ADMIN_TOKEN:-}" ]; then
|
||||
echo "::error::CP_ADMIN_API_TOKEN secret not set — cannot run delivery e2e"
|
||||
exit 2
|
||||
fi
|
||||
- name: Run template-asset delivery e2e
|
||||
if: needs.detect-changes.outputs.delivery == 'true'
|
||||
run: bash tests/e2e/test_template_delivery_e2e.sh
|
||||
|
||||
@@ -119,6 +119,107 @@ func isConciergeIdentityPath(rel string) bool {
|
||||
strings.HasPrefix(rel, "prompts/")
|
||||
}
|
||||
|
||||
// hasDockerfileCopyForRel reports whether Dockerfile.platform-agent contains
|
||||
// a COPY instruction for the expected IMAGE-BAKED file `rel` (relative to the
|
||||
// platform-agent template SSOT root). The Dockerfile uses two patterns:
|
||||
//
|
||||
// - COPY ${PLATFORM_AGENT_TEMPLATE_DIR}/<rel> ... for top-level files
|
||||
// (config.yaml, mcp_servers.yaml, identity-fallback.sh).
|
||||
// - COPY ${PLATFORM_AGENT_TEMPLATE_DIR}/<dir>/ ... for directory-baked
|
||||
// content (prompts/concierge.md is shipped via the prompts/ dir copy).
|
||||
//
|
||||
// COPY instructions may also carry Dockerfile flags such as
|
||||
// `--chmod=0755` before the source path, so the matcher permits an
|
||||
// optional flag segment between `COPY` and the source path.
|
||||
//
|
||||
// This helper centralises the pattern matching so the test body stays readable
|
||||
// and the two valid COPY shapes are documented in one place.
|
||||
func hasDockerfileCopyForRel(dockerfileStr, rel string) bool {
|
||||
rel = filepath.ToSlash(filepath.Clean(rel))
|
||||
relRe := regexp.QuoteMeta(rel)
|
||||
dirRe := regexp.QuoteMeta(filepath.Dir(rel) + "/")
|
||||
|
||||
// Match: COPY [flags] ${PLATFORM_AGENT_TEMPLATE_DIR}/<rel> ...
|
||||
// or: COPY [flags] ${PLATFORM_AGENT_TEMPLATE_DIR}/<dir>/ ...
|
||||
// Flags are zero or more `--flag[=value]` tokens (e.g. --chmod=0755,
|
||||
// --chown=app:app, --chown=1000:1000) before the source path.
|
||||
pattern := `(?m)^COPY(?:\s+--\S+)*\s+\$\{PLATFORM_AGENT_TEMPLATE_DIR\}/(?:` + relRe + `|` + dirRe + `)\s`
|
||||
matched, err := regexp.MatchString(pattern, dockerfileStr)
|
||||
if err != nil {
|
||||
// regexp.QuoteMeta only produces safe patterns; a compile error
|
||||
// here is a test-authoring bug, not a product failure.
|
||||
panic("invalid hasDockerfileCopyForRel pattern: " + err.Error())
|
||||
}
|
||||
return matched
|
||||
}
|
||||
|
||||
func TestHasDockerfileCopyForRel(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
dockerfile string
|
||||
rel string
|
||||
wantMatched bool
|
||||
}{
|
||||
{
|
||||
name: "top-level file COPY",
|
||||
dockerfile: "COPY ${PLATFORM_AGENT_TEMPLATE_DIR}/config.yaml /opt/molecule-platform-agent-template/config.yaml\n",
|
||||
rel: "config.yaml",
|
||||
wantMatched: true,
|
||||
},
|
||||
{
|
||||
name: "top-level file COPY with --chmod",
|
||||
dockerfile: "COPY --chmod=0755 ${PLATFORM_AGENT_TEMPLATE_DIR}/identity-fallback.sh /opt/molecule-platform-agent-template/identity-fallback.sh\n",
|
||||
rel: "identity-fallback.sh",
|
||||
wantMatched: true,
|
||||
},
|
||||
{
|
||||
name: "top-level file COPY with --chown",
|
||||
dockerfile: "COPY --chown=1000:1000 ${PLATFORM_AGENT_TEMPLATE_DIR}/identity-fallback.sh /opt/molecule-platform-agent-template/identity-fallback.sh\n",
|
||||
rel: "identity-fallback.sh",
|
||||
wantMatched: true,
|
||||
},
|
||||
{
|
||||
name: "top-level file COPY with multiple flags",
|
||||
dockerfile: "COPY --chmod=0755 --chown=node:node ${PLATFORM_AGENT_TEMPLATE_DIR}/identity-fallback.sh /opt/molecule-platform-agent-template/identity-fallback.sh\n",
|
||||
rel: "identity-fallback.sh",
|
||||
wantMatched: true,
|
||||
},
|
||||
{
|
||||
name: "directory COPY for nested file",
|
||||
dockerfile: "COPY ${PLATFORM_AGENT_TEMPLATE_DIR}/prompts/ /opt/molecule-platform-agent-template/prompts/\n",
|
||||
rel: "prompts/concierge.md",
|
||||
wantMatched: true,
|
||||
},
|
||||
{
|
||||
name: "missing COPY",
|
||||
dockerfile: "RUN echo no-copy\n",
|
||||
rel: "config.yaml",
|
||||
wantMatched: false,
|
||||
},
|
||||
{
|
||||
name: "wrong source variable",
|
||||
dockerfile: "COPY ${OTHER_DIR}/config.yaml /opt/molecule-platform-agent-template/config.yaml\n",
|
||||
rel: "config.yaml",
|
||||
wantMatched: false,
|
||||
},
|
||||
{
|
||||
name: "nested file missing directory COPY",
|
||||
dockerfile: "COPY ${PLATFORM_AGENT_TEMPLATE_DIR}/prompts/concierge.md /opt/molecule-platform-agent-template/prompts/concierge.md\n",
|
||||
rel: "prompts/concierge.md",
|
||||
wantMatched: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := hasDockerfileCopyForRel(tt.dockerfile, tt.rel)
|
||||
if got != tt.wantMatched {
|
||||
t.Errorf("hasDockerfileCopyForRel(%q, %q) = %v, want %v", tt.dockerfile, tt.rel, got, tt.wantMatched)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// canonicalPlatformAgentSSOTRelPath is the default SSOT path the
|
||||
// drift-gate reads from when PLATFORM_AGENT_TEMPLATE_REPO_PATH is
|
||||
// unset, RELATIVE TO THE REPO ROOT. It mirrors Dockerfile.platform-
|
||||
@@ -234,26 +335,7 @@ func TestPlatformAgentImageDriftGate(t *testing.T) {
|
||||
dockerfileStr := string(dockerfile)
|
||||
|
||||
for _, rel := range expectedImageBakedFiles {
|
||||
// The Dockerfile uses two patterns: COPY <rel> /opt/...
|
||||
// for the top-level files (config.yaml, mcp_servers.yaml,
|
||||
// identity-fallback.sh) and COPY <dir>/ /opt/.../ for the
|
||||
// prompts/ directory. We check that EITHER pattern appears
|
||||
// for the expected file.
|
||||
//
|
||||
// COPY may carry build-flags between the verb and the source
|
||||
// arg — e.g. `COPY --chmod=0755 ${PLATFORM_AGENT_TEMPLATE_DIR}/
|
||||
// identity-fallback.sh ...` (e4efc35d switched identity-
|
||||
// fallback.sh from `RUN chmod` to `COPY --chmod` because the
|
||||
// non-root tenant base can't `RUN chmod`). The matcher must
|
||||
// tolerate any such `--flag[=value]` tokens; a literal-substring
|
||||
// match on `COPY ${...}/` would false-fail the drift-gate the
|
||||
// moment a COPY grows a flag. Match `COPY` + optional flags +
|
||||
// the source path via regex (whitespace-flexible).
|
||||
quotedDir := regexp.QuoteMeta(`${PLATFORM_AGENT_TEMPLATE_DIR}/`)
|
||||
copyFlags := `(?:\s+--\S+)*` // zero or more `--flag[=val]` tokens
|
||||
topLevel := regexp.MustCompile(`COPY` + copyFlags + `\s+` + quotedDir + regexp.QuoteMeta(rel) + `\b`)
|
||||
dirPattern := regexp.MustCompile(`COPY` + copyFlags + `\s+` + quotedDir + regexp.QuoteMeta(filepath.Dir(rel)) + `/`)
|
||||
if !topLevel.MatchString(dockerfileStr) && !dirPattern.MatchString(dockerfileStr) {
|
||||
if !hasDockerfileCopyForRel(dockerfileStr, rel) {
|
||||
t.Errorf("Dockerfile COPY missing: %s — the IMAGE-BAKED impl must COPY %s from the platform-agent template SSOT; if a new identity file is added, update Dockerfile.platform-agent AND expectedImageBakedFiles", rel, rel)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user