Compare commits
55 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 81630a36f8 | |||
| 173881e67a | |||
| bde5421766 | |||
| 2e068c7586 | |||
| a380218234 | |||
| 578b145312 | |||
| a77b6850e2 | |||
| 2f9b5b6704 | |||
| 86df02c38f | |||
| db39d519dc | |||
| 0b771d5770 | |||
| 8a63d16f8c | |||
| 63c25d4c3f | |||
| 116697c576 | |||
| d1c6fce937 | |||
| 0e87fde0a3 | |||
| d768d8667b | |||
| 99d4a44250 | |||
| 29d15cbe2c | |||
| b1475b1f71 | |||
| b2d5f88f98 | |||
| 31283a292a | |||
| 8ae3cb6917 | |||
| bc7c45f3d6 | |||
| 7b3fc0f2ef | |||
| e441def8b3 | |||
| 51f83260df | |||
| 2fa68b1f23 | |||
| 79be721591 | |||
| 1c07d65561 | |||
| c950dcbd6e | |||
| 79e34175c9 | |||
| e5daf96dab | |||
| 4b56cabe24 | |||
| b057994cac | |||
| be1f38b7b5 | |||
| d4be3e383a | |||
| 7fb66f473d | |||
| be387623c6 | |||
| 61d8fdc9ec | |||
| 032befab27 | |||
| 2b78e29138 | |||
| d49a31ff29 | |||
| 1963356317 | |||
| d61d9af761 | |||
| 74c1c4e7dd | |||
| 90852601cc | |||
| 2f53bbac6c | |||
| 2f5536fd48 | |||
| 9a965cfcea | |||
| be46aabf78 | |||
| 74a3299a53 | |||
| c351adc46d | |||
| bb82e42901 | |||
| a60033dc16 |
+739
-114
File diff suppressed because it is too large
Load Diff
@@ -546,16 +546,24 @@ def verify_flip(flip: dict, branch: str, n: int) -> dict:
|
||||
|
||||
shas = recent_commits_on_branch(branch, n)
|
||||
if not shas:
|
||||
result["warnings"].append(
|
||||
f"no recent commits on {branch} (cannot verify flip)"
|
||||
)
|
||||
result["masked_runs"].append({
|
||||
"sha": "",
|
||||
"status": "unverified",
|
||||
"target_url": "",
|
||||
"samples": [f"no recent commits on {branch} — cannot verify flip"],
|
||||
})
|
||||
return result
|
||||
|
||||
for sha in shas:
|
||||
try:
|
||||
status_doc = combined_status(sha)
|
||||
except ApiError as e:
|
||||
result["warnings"].append(f"combined-status for {sha}: {e}")
|
||||
result["masked_runs"].append({
|
||||
"sha": sha,
|
||||
"status": "error",
|
||||
"target_url": "",
|
||||
"samples": [f"combined-status API error: {e}"],
|
||||
})
|
||||
continue
|
||||
statuses = status_doc.get("statuses") or []
|
||||
# First entry matching the context name. Newest SHAs come
|
||||
@@ -582,6 +590,17 @@ def verify_flip(flip: dict, branch: str, n: int) -> dict:
|
||||
"target_url": target_url,
|
||||
"samples": ["[log unavailable; status itself is " + state + "]"],
|
||||
})
|
||||
elif state == "success":
|
||||
# Fail-closed: unreadable log on a success status is a
|
||||
# potential Quirk #10 mask (continue-on-error hiding real
|
||||
# failures). We cannot verify it's clean, so treat as
|
||||
# masked rather than allowing the flip.
|
||||
result["masked_runs"].append({
|
||||
"sha": sha,
|
||||
"status": state,
|
||||
"target_url": target_url,
|
||||
"samples": ["[log unavailable; cannot verify status is genuine — treat as masked]"],
|
||||
})
|
||||
break
|
||||
samples = grep_fail_markers(log_text)
|
||||
if state in ("failure", "error"):
|
||||
@@ -605,10 +624,12 @@ def verify_flip(flip: dict, branch: str, n: int) -> dict:
|
||||
break
|
||||
|
||||
if result["checked_commits"] == 0:
|
||||
result["warnings"].append(
|
||||
f"no runs of {target_context!r} found in the last {n} commits on "
|
||||
f"{branch} — cannot verify; allowing flip with warning"
|
||||
)
|
||||
result["masked_runs"].append({
|
||||
"sha": "",
|
||||
"status": "unverified",
|
||||
"target_url": "",
|
||||
"samples": [f"no runs of {target_context!r} found in the last {n} commits on {branch} — cannot verify flip"],
|
||||
})
|
||||
return result
|
||||
|
||||
|
||||
|
||||
@@ -197,19 +197,15 @@ if [ "$HTTP_CODE" != "200" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Filter: state=APPROVED, not-dismissed, non-author. Optionally strict-mode
|
||||
# adds commit_id==head.sha (off by default; see header).
|
||||
# Filter: state=APPROVED, official=true, not-dismissed, non-author,
|
||||
# commit_id matches current PR head. All conditions are mandatory.
|
||||
JQ_FILTER='.[]
|
||||
| select(.state == "APPROVED")
|
||||
| select(.official == true)
|
||||
| select(.dismissed != true)
|
||||
| select(.official != false)
|
||||
| select(.user.login != $author)'
|
||||
if [ "${REVIEW_CHECK_STRICT:-}" = "1" ]; then
|
||||
JQ_FILTER="${JQ_FILTER}
|
||||
| select(.commit_id == \$head)"
|
||||
fi
|
||||
JQ_FILTER="${JQ_FILTER}
|
||||
| .user.login"
|
||||
| select(.user.login != $author)
|
||||
| select(.commit_id == $head)
|
||||
| .user.login'
|
||||
|
||||
REVIEW_CANDIDATES=$(jq -r --arg author "$PR_AUTHOR" --arg head "$PR_HEAD_SHA" "$JQ_FILTER" "$REVIEWS_JSON" | sort -u)
|
||||
debug "candidate non-author approvers: $(echo "$REVIEW_CANDIDATES" | tr '\n' ' ')"
|
||||
@@ -241,49 +237,14 @@ if [ -z "$REVIEW_CANDIDATES" ]; then
|
||||
|
||||
fi
|
||||
|
||||
# --- Fallback/extension (internal#348): check issue comments for agent-approval ---
|
||||
# core-qa-agent and core-security-agent can approve via issue comments. Always
|
||||
# include comment candidates, even if the reviews API returned approvals for a
|
||||
# different team; team membership below is the authoritative filter.
|
||||
COMMENT_CANDIDATES=""
|
||||
AGENT_PATTERN=""
|
||||
case "$TEAM" in
|
||||
qa) AGENT_PATTERN="\\[core-qa-agent\\]" ;;
|
||||
security) AGENT_PATTERN="\\[core-security-agent\\]" ;;
|
||||
esac
|
||||
HTTP_CODE=$(curl -sS -o "$COMMENTS_JSON" -w '%{http_code}' \
|
||||
-K "$CURL_AUTH_FILE" "${API}/repos/${OWNER}/${NAME}/issues/${PR_NUMBER}/comments")
|
||||
debug "GET /issues/${PR_NUMBER}/comments → HTTP ${HTTP_CODE}"
|
||||
if [ "$HTTP_CODE" = "200" ]; then
|
||||
# JQ expression: select non-author comments that match either the
|
||||
# agent-prefix pattern (case-insensitive) OR a generic approval keyword.
|
||||
JQ_APPROVALS='
|
||||
.[] |
|
||||
select(.user.login != $author) |
|
||||
. as $cmt |
|
||||
if ($agent_pattern | length) > 0 and ($cmt.body // "" | test($agent_pattern; "i")) then
|
||||
$cmt.user.login
|
||||
elif ($cmt.body // "" | test("\\b(APPROVED|LGTM|ACCEPTED)\\b"; "i")) then
|
||||
$cmt.user.login
|
||||
else
|
||||
empty
|
||||
end
|
||||
'
|
||||
COMMENT_CANDIDATES=$(jq -r \
|
||||
--arg author "$PR_AUTHOR" \
|
||||
--arg agent_pattern "$AGENT_PATTERN" \
|
||||
"$JQ_APPROVALS" \
|
||||
"$COMMENTS_JSON" 2>/dev/null | sort -u)
|
||||
debug "comment-based approval candidates: $(echo "$COMMENT_CANDIDATES" | tr '\n' ' ')"
|
||||
|
||||
if [ -n "$COMMENT_CANDIDATES" ]; then
|
||||
echo "::notice::${TEAM}-review: found $(echo "$COMMENT_CANDIDATES" | wc -w | xargs) comment-based approval candidate(s) — verifying team membership..."
|
||||
fi
|
||||
else
|
||||
debug "could not fetch issue comments (HTTP ${HTTP_CODE})"
|
||||
fi
|
||||
|
||||
CANDIDATES=$(printf '%s\n%s\n' "$REVIEW_CANDIDATES" "$COMMENT_CANDIDATES" | sed '/^$/d' | sort -u)
|
||||
# --- COMMENT APPROVAL REMOVED (security hardening) ---
|
||||
# Previous versions accepted issue comments containing generic approval
|
||||
# keywords (APPROVED/LGTM/ACCEPTED) or agent prefixes ([core-qa-agent],
|
||||
# [core-security-agent]) as satisfying the gate. Both paths are bypasses:
|
||||
# a comment lacks the audit trail, dismissal, stale-review invalidation,
|
||||
# and commit_id binding that an official Gitea review provides.
|
||||
# Only APPROVED reviews from the Gitea reviews API count.
|
||||
CANDIDATES="$REVIEW_CANDIDATES"
|
||||
|
||||
if [ -z "${CANDIDATES:-}" ]; then
|
||||
echo "::error::${TEAM}-review awaiting non-author APPROVE from ${TEAM} team (no candidates from reviews API or issue comments)"
|
||||
|
||||
@@ -174,6 +174,16 @@ def parse_directives(
|
||||
if not parts:
|
||||
continue
|
||||
first = parts[0]
|
||||
# Em-dash (U+2014) is a common visual separator in user-written
|
||||
# notes, e.g. /sop-ack Five-Axis — five-axis-review
|
||||
# If raw_slug contains an em-dash, split on the first one so
|
||||
# the part before becomes the slug and the rest becomes the note.
|
||||
note_from_slug = ""
|
||||
slug_source = raw_slug
|
||||
emdash_idx = raw_slug.find("—")
|
||||
if emdash_idx != -1:
|
||||
slug_source = raw_slug[:emdash_idx].strip()
|
||||
note_from_slug = raw_slug[emdash_idx + 1 :].strip()
|
||||
# If the slug-capture greedily matched multiple words (e.g.
|
||||
# "comprehensive testing"), preserve normalize behavior: join
|
||||
# the WHOLE first-word-token only; trailing words get appended to
|
||||
@@ -186,13 +196,19 @@ def parse_directives(
|
||||
# as slug and "testing extra-note" as note. We defer the
|
||||
# disambiguation to the caller via the returned canonical
|
||||
# slug. For simplicity: try the WHOLE captured string first.
|
||||
canonical = normalize_slug(raw_slug, numeric_aliases)
|
||||
canonical = normalize_slug(slug_source, numeric_aliases)
|
||||
else:
|
||||
canonical = normalize_slug(first, numeric_aliases)
|
||||
canonical = normalize_slug(slug_source, numeric_aliases)
|
||||
note_from_group = (m.group(3) or "").strip()
|
||||
# If we collapsed multi-word slug into kebab and there's a
|
||||
# trailing-text group too, append it.
|
||||
entry = (kind, canonical, note_from_group)
|
||||
# The em-dash (U+2014) is a visual separator; the regex puts it
|
||||
# in group(3) because it is outside the slug character class.
|
||||
# Strip it so "/sop-ack slug — note" yields just "note".
|
||||
if note_from_group.startswith("—"):
|
||||
note_from_group = note_from_group[1:].strip()
|
||||
# Combine note_from_slug (em-dash split) with note_from_group
|
||||
# (trailing text after the slug captured by the regex group).
|
||||
combined_note = (note_from_slug + " " + note_from_group).strip()
|
||||
entry = (kind, canonical, combined_note)
|
||||
if kind == "sop-n/a":
|
||||
na_directives.append(entry)
|
||||
else:
|
||||
|
||||
@@ -48,7 +48,6 @@ set -euo pipefail
|
||||
# workflow-level jq install can fail on runners with network restrictions
|
||||
# (GitHub releases not reachable from some runner networks — infra#241
|
||||
# follow-up). This fallback is idempotent — no-op when jq is already on PATH.
|
||||
# SOP_FAIL_OPEN=1 makes this always exit 0 so CI never blocks on jq absence.
|
||||
if ! command -v jq >/dev/null 2>&1; then
|
||||
echo "::notice::jq not found on PATH — attempting install..."
|
||||
_jq_installed="no"
|
||||
@@ -67,12 +66,6 @@ if ! command -v jq >/dev/null 2>&1; then
|
||||
if ! command -v jq >/dev/null 2>&1; then
|
||||
echo "::error::jq installation failed — apt-get and GitHub binary both failed."
|
||||
echo "::error::sop-tier-check requires jq for all JSON API parsing."
|
||||
# SOP_FAIL_OPEN=1 is set in the workflow step's env — makes script always
|
||||
# exit 0 so CI never blocks. The SOP-6 tier review gate remains enforced.
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
fi
|
||||
@@ -101,15 +94,10 @@ echo "::notice::tier-check start: repo=$OWNER/$NAME pr=$PR_NUMBER author=$PR_AUT
|
||||
# cause the script to exit prematurely when the token is empty/invalid — the
|
||||
# if check below handles that case gracefully. Without || true, a 401 from an
|
||||
# empty/invalid token causes jq to exit 1, triggering set -e and exiting the
|
||||
# entire script before SOP_FAIL_OPEN can be evaluated (the check is in the jq-
|
||||
# install block; if jq is already on PATH, that block is skipped entirely).
|
||||
# entire script before the error can be logged.
|
||||
WHOAMI=$(curl -sS -H "$AUTH" "${API}/user" | jq -r '.login // ""') || true
|
||||
if [ -z "$WHOAMI" ]; then
|
||||
echo "::error::GITEA_TOKEN cannot resolve a user via /api/v1/user — check the token scope and that the secret is wired correctly."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
echo "::notice::token resolves to user: $WHOAMI"
|
||||
@@ -119,10 +107,6 @@ echo "::notice::token resolves to user: $WHOAMI"
|
||||
HEAD_SHA=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}" | jq -r '.head.sha // ""') || true
|
||||
if [ -z "$HEAD_SHA" ]; then
|
||||
echo "::error::Failed to fetch PR head SHA — token may be invalid."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
debug "pr-head-sha=$HEAD_SHA"
|
||||
@@ -215,10 +199,6 @@ if [ "${SOP_DEBUG:-}" = "1" ]; then
|
||||
fi
|
||||
if [ "$_HTTP_EXIT" -ne 0 ] || [ "$HTTP_CODE" != "200" ]; then
|
||||
echo "::error::GET /orgs/${OWNER}/teams failed (curl exit=$_HTTP_EXIT HTTP=$HTTP_CODE) — token may lack read:org scope or be invalid."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -265,17 +245,13 @@ done
|
||||
|
||||
# 5. Read approving reviewers. set +e disables set -e temporarily so that curl
|
||||
# failures (e.g. empty/invalid token → HTTP 401) do not abort the script before
|
||||
# SOP_FAIL_OPEN is evaluated. set -e is restored immediately after.
|
||||
# set -e is restored immediately after.
|
||||
set +e
|
||||
REVIEWS=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}/reviews")
|
||||
_REVIEWS_EXIT=$?
|
||||
set -e
|
||||
if [ $_REVIEWS_EXIT -ne 0 ] || [ -z "$REVIEWS" ]; then
|
||||
echo "::error::Failed to fetch reviews (curl exit=$_REVIEWS_EXIT) — token may be invalid or unreachable."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
APPROVERS=$(echo "$REVIEWS" | jq -r --arg head_sha "$HEAD_SHA" '[.[] | select(.state=="APPROVED" and .commit_id == $head_sha) | .user.login] | unique | .[]') || true
|
||||
|
||||
@@ -689,8 +689,8 @@ def reap_branch(
|
||||
shas = list_recent_commit_shas(branch, limit)
|
||||
except ApiError as e:
|
||||
print(
|
||||
"::warning::status-reaper skipped this tick because the "
|
||||
f"commit list could not be read after retries: {e}"
|
||||
"::error::status-reaper cannot run: commit-list API failed "
|
||||
f"after retries: {e}"
|
||||
)
|
||||
return {
|
||||
"scanned_shas": 0,
|
||||
@@ -704,6 +704,7 @@ def reap_branch(
|
||||
"compensated_cancelled_push": 0,
|
||||
"preserved_pr_without_push_success": 0,
|
||||
"compensated_per_sha": {},
|
||||
"sha_api_errors": 0,
|
||||
"skipped": True,
|
||||
"skip_reason": "commit-list-api-error",
|
||||
}
|
||||
@@ -720,6 +721,7 @@ def reap_branch(
|
||||
"compensated_cancelled_push": 0,
|
||||
"preserved_pr_without_push_success": 0,
|
||||
"compensated_per_sha": {},
|
||||
"sha_api_errors": 0,
|
||||
}
|
||||
|
||||
for sha in shas:
|
||||
@@ -731,8 +733,9 @@ def reap_branch(
|
||||
try:
|
||||
combined = get_combined_status(sha)
|
||||
except ApiError as e:
|
||||
aggregate["sha_api_errors"] += 1
|
||||
print(
|
||||
f"::warning::get_combined_status({sha[:10]}) failed; "
|
||||
f"::error::get_combined_status({sha[:10]}) failed; "
|
||||
f"skipping this SHA: {e}"
|
||||
)
|
||||
continue
|
||||
@@ -819,6 +822,14 @@ def main() -> int:
|
||||
sort_keys=True,
|
||||
)
|
||||
)
|
||||
# Observability: infra-failure → red. If the commit list could not be
|
||||
# read or any per-SHA status fetch failed, the tick is incomplete and
|
||||
# must be observable as a failure (non-zero exit) so the cron bot or
|
||||
# runner surface alerts.
|
||||
if counters.get("skipped"):
|
||||
return 1
|
||||
if counters.get("sha_api_errors", 0) > 0:
|
||||
return 1
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
@@ -109,23 +109,34 @@ class Handler(http.server.BaseHTTPRequestHandler):
|
||||
return self._json(200, [{
|
||||
"state": "APPROVED",
|
||||
"dismissed": True,
|
||||
"official": True,
|
||||
"user": {"login": "core-devops"},
|
||||
"commit_id": "abc1234",
|
||||
"commit_id": "deadbeef0000111122223333444455556666",
|
||||
}])
|
||||
if sc == "T3_reviews_approved_non_author":
|
||||
return self._json(200, [
|
||||
{"state": "CHANGES_REQUESTED", "dismissed": False, "user": {"login": "bob"}, "commit_id": "abc1234"},
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "core-devops"}, "commit_id": "abc1234"},
|
||||
{"state": "CHANGES_REQUESTED", "dismissed": False, "official": True, "user": {"login": "bob"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "core-devops"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
if sc == "T19_ai_sop_ack_approved":
|
||||
# ai-sop-ack member submitted APPROVED review — must NOT count
|
||||
# toward qa-review (team_id=20) or security-review (team_id=21).
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "ai-reviewer"}, "commit_id": "abc1234"},
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "ai-reviewer"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
# Default: one non-author APPROVED
|
||||
if sc == "T21_stale_head_approved":
|
||||
# APPROVED review but on an old commit (stale head) → must be rejected
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "core-devops"}, "commit_id": "oldsha0000000000000000000000000000"},
|
||||
])
|
||||
if sc == "T22_missing_official":
|
||||
# APPROVED review with no official field → must be rejected
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "core-devops"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
# Default: one non-author APPROVED (current head, official)
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "core-devops"}, "commit_id": "abc1234"},
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "core-devops"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
|
||||
# GET /repos/{owner}/{name}/issues/{pr_number}/comments
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -320,10 +320,10 @@ class TestVerifyFlip(unittest.TestCase):
|
||||
self.assertEqual(len(verdict["fail_runs"]), 1)
|
||||
self.assertEqual(verdict["fail_runs"][0]["status"], "failure")
|
||||
|
||||
def test_unreadable_log_warns_not_blocks(self):
|
||||
# Acceptance test #5: log fetch 404 (None) → warn, not block.
|
||||
# Status is `success`, log is None — we can't tell, so we warn
|
||||
# and allow.
|
||||
def test_unreadable_log_on_success_blocks(self):
|
||||
# Fail-closed: log fetch 404 (None) on a success status is a
|
||||
# potential Quirk #10 mask — we cannot verify it's genuine, so
|
||||
# we block the flip rather than allowing it.
|
||||
with mock.patch.object(lpfc, "recent_commits_on_branch", return_value=["sha1"]):
|
||||
with mock.patch.object(
|
||||
lpfc, "combined_status",
|
||||
@@ -332,7 +332,8 @@ class TestVerifyFlip(unittest.TestCase):
|
||||
with mock.patch.object(lpfc, "fetch_log", return_value=None):
|
||||
verdict = lpfc.verify_flip(FLIP_FIXTURE, "main", 5)
|
||||
self.assertEqual(verdict["fail_runs"], [])
|
||||
self.assertEqual(verdict["masked_runs"], [])
|
||||
self.assertEqual(len(verdict["masked_runs"]), 1)
|
||||
self.assertIn("log unavailable", verdict["masked_runs"][0]["samples"][0])
|
||||
self.assertTrue(any("log unavailable" in w for w in verdict["warnings"]))
|
||||
|
||||
def test_unreadable_log_with_failure_status_still_blocks(self):
|
||||
@@ -349,9 +350,9 @@ class TestVerifyFlip(unittest.TestCase):
|
||||
self.assertEqual(len(verdict["fail_runs"]), 1)
|
||||
self.assertIn("log unavailable", verdict["fail_runs"][0]["samples"][0])
|
||||
|
||||
def test_zero_runs_history_warns_allows(self):
|
||||
# No commits with a matching context — newly added workflow.
|
||||
# Allow with warning.
|
||||
def test_zero_runs_history_blocks(self):
|
||||
# No commits with a matching context — cannot verify the flip.
|
||||
# Fail-closed: treat as masked rather than allowing.
|
||||
with mock.patch.object(lpfc, "recent_commits_on_branch", return_value=["sha1", "sha2"]):
|
||||
with mock.patch.object(
|
||||
lpfc, "combined_status",
|
||||
@@ -360,17 +361,32 @@ class TestVerifyFlip(unittest.TestCase):
|
||||
verdict = lpfc.verify_flip(FLIP_FIXTURE, "main", 5)
|
||||
self.assertEqual(verdict["checked_commits"], 0)
|
||||
self.assertEqual(verdict["fail_runs"], [])
|
||||
self.assertEqual(verdict["masked_runs"], [])
|
||||
self.assertTrue(any("no runs of" in w for w in verdict["warnings"]))
|
||||
self.assertEqual(len(verdict["masked_runs"]), 1)
|
||||
self.assertIn("cannot verify flip", verdict["masked_runs"][0]["samples"][0])
|
||||
|
||||
def test_zero_commits_warns_allows(self):
|
||||
# Empty branch (newly created repo, e.g.). Allow with warning.
|
||||
def test_zero_commits_blocks(self):
|
||||
# Empty branch (newly created repo, e.g.). Fail-closed: block.
|
||||
with mock.patch.object(lpfc, "recent_commits_on_branch", return_value=[]):
|
||||
verdict = lpfc.verify_flip(FLIP_FIXTURE, "main", 5)
|
||||
self.assertEqual(verdict["checked_commits"], 0)
|
||||
self.assertEqual(verdict["fail_runs"], [])
|
||||
self.assertEqual(verdict["masked_runs"], [])
|
||||
self.assertTrue(any("no recent commits" in w for w in verdict["warnings"]))
|
||||
self.assertEqual(len(verdict["masked_runs"]), 1)
|
||||
self.assertIn("cannot verify flip", verdict["masked_runs"][0]["samples"][0])
|
||||
|
||||
def test_combined_status_api_error_blocks(self):
|
||||
# Fail-closed: combined_status ApiError means the check history is
|
||||
# unreadable — we cannot verify the flip, so block as masked.
|
||||
with mock.patch.object(lpfc, "recent_commits_on_branch", return_value=["sha1"]):
|
||||
with mock.patch.object(
|
||||
lpfc, "combined_status",
|
||||
side_effect=lpfc.ApiError("GET /statuses/sha → HTTP 500"),
|
||||
):
|
||||
verdict = lpfc.verify_flip(FLIP_FIXTURE, "main", 5)
|
||||
self.assertEqual(verdict["checked_commits"], 0)
|
||||
self.assertEqual(verdict["fail_runs"], [])
|
||||
# One masked_run from the ApiError, one from zero checked_commits.
|
||||
self.assertEqual(len(verdict["masked_runs"]), 2)
|
||||
self.assertIn("API error", verdict["masked_runs"][0]["samples"][0])
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
@@ -14,10 +14,17 @@
|
||||
# T9 — team membership probe → 403 (token not in team) → script exits 1 (fail closed)
|
||||
# T10 — CURL_AUTH_FILE created with mode 600 and correct header content
|
||||
# T11 — bash syntax check (bash -n passes)
|
||||
# T12 — jq filter: non-author APPROVED → in candidate list; dismissed → excluded
|
||||
# T12 — jq filter: non-author APPROVED official current-head → in candidate list; dismissed → excluded
|
||||
# T13 — missing required env GITEA_TOKEN → exits 1 with error
|
||||
# T14 — non-default-base PR exits 0 without requiring review
|
||||
# T18 — wrong-team review candidate does not block right-team comment approval
|
||||
# T15 — comment agent-prefix approval → exit 1
|
||||
# T16 — comment generic keyword approval → exit 1
|
||||
# T17 — comments with no approval keywords → exit 1
|
||||
# T18 — wrong-team review + right-team comment → exit 1
|
||||
# T19 — ai-sop-ack APPROVED review excluded from qa-review gate
|
||||
# T20 — ai-sop-ack APPROVED review excluded from security-review gate
|
||||
# T21 — stale-head APPROVED review → exit 1 (commit_id mismatch)
|
||||
# T22 — missing/non-official APPROVED review → exit 1 (official != true)
|
||||
#
|
||||
# Hostile-self-review (per feedback_assert_exact_not_substring):
|
||||
# this test MUST FAIL if the script is absent. Verified by running
|
||||
@@ -319,41 +326,50 @@ assert_file_contains "T10b printf header format (CURL_AUTH_FILE content)" "$T10_
|
||||
assert_file_contains "T10c 'header =' curl-config syntax" "$T10_AUTHFILE" 'header = "Authorization: token '
|
||||
rm -f "$T10_AUTHFILE"
|
||||
|
||||
# T12 — jq filter: non-author APPROVED included, dismissed excluded
|
||||
# T12 — jq filter: non-author APPROVED official current-head included; dismissed/stale/missing-official excluded
|
||||
echo
|
||||
echo "== T12 jq filter =="
|
||||
# These are tested indirectly via T3 and T6 above, but let's also test
|
||||
# the jq expression directly.
|
||||
JQ_FILTER='.[]
|
||||
| select(.state == "APPROVED")
|
||||
| select(.official == true)
|
||||
| select(.dismissed != true)
|
||||
| select(.user.login != "alice")
|
||||
| select(.commit_id == $head)
|
||||
| .user.login'
|
||||
|
||||
T12_INPUT='[{"state":"APPROVED","dismissed":false,"user":{"login":"core-devops"}},{"state":"CHANGES_REQUESTED","dismissed":false,"user":{"login":"bob"}},{"state":"APPROVED","dismissed":false,"user":{"login":"alice"}},{"state":"APPROVED","dismissed":true,"user":{"login":"carol"}}]'
|
||||
T12_INPUT='[{"state":"APPROVED","official":true,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"core-devops"}},{"state":"CHANGES_REQUESTED","official":true,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"bob"}},{"state":"APPROVED","official":true,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"alice"}},{"state":"APPROVED","official":true,"dismissed":true,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"carol"}},{"state":"APPROVED","official":false,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"dave"}},{"state":"APPROVED","official":true,"dismissed":false,"commit_id":"oldsha0000000000000000000000000000","user":{"login":"eve"}}]'
|
||||
|
||||
JQ_CMD=$(command -v jq 2>/dev/null || echo /tmp/jq)
|
||||
T12_CANDIDATES=$(echo "$T12_INPUT" | "$JQ_CMD" -r "$JQ_FILTER" 2>/dev/null | sort -u)
|
||||
assert_contains "T12 jq: core-devops (non-author APPROVED) in candidates" "core-devops" "$T12_CANDIDATES"
|
||||
T12_CANDIDATES=$(echo "$T12_INPUT" | "$JQ_CMD" -r --arg head "deadbeef0000111122223333444455556666" "$JQ_FILTER" 2>/dev/null | sort -u)
|
||||
assert_contains "T12 jq: core-devops (non-author APPROVED official current-head) in candidates" "core-devops" "$T12_CANDIDATES"
|
||||
assert_eq "T12 jq: alice (author) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^alice$' || true)"
|
||||
assert_eq "T12 jq: carol (dismissed) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^carol$' || true)"
|
||||
assert_eq "T12 jq: dave (official=false) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^dave$' || true)"
|
||||
assert_eq "T12 jq: eve (stale head) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^eve$' || true)"
|
||||
|
||||
# T15 — comment-based approval via agent prefix pattern → exit 0
|
||||
# T15 — comment-based approval via agent prefix pattern → exit 1
|
||||
# SECURITY: agent-prefix comments are also removed. A text prefix in an
|
||||
# issue comment is spoofable (any team member can type "[core-qa-agent]")
|
||||
# and lacks the audit trail of an official Gitea review.
|
||||
echo
|
||||
echo "== T15 comment agent-prefix approval =="
|
||||
T15_OUT=$(run_review_check "T15_comments_agent_approval")
|
||||
T15_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T15 exit code 0 (agent-comment approval + team member)" "0" "$T15_RC"
|
||||
assert_contains "T15 comment fallback notice" "comment-based approval" "$T15_OUT"
|
||||
assert_contains "T15 core-qa-agent APPROVED" "APPROVED by core-qa-agent" "$T15_OUT"
|
||||
assert_eq "T15 exit code 1 (agent-prefix comment rejected — not an official review)" "1" "$T15_RC"
|
||||
assert_contains "T15 no candidates error" "no candidates from reviews API or issue comments" "$T15_OUT"
|
||||
|
||||
# T16 — comment-based approval via generic APPROVED keyword → exit 0
|
||||
# T16 — comment-based approval via generic APPROVED keyword → exit 1
|
||||
# SECURITY: generic keywords (APPROVED/LGTM/ACCEPTED) must NOT satisfy the
|
||||
# gate — only official Gitea reviews or agent-prefix comments count. A plain
|
||||
# comment from a team member is a bypass if it skips the review UI.
|
||||
echo
|
||||
echo "== T16 comment generic keyword approval =="
|
||||
T16_OUT=$(run_review_check "T16_comments_generic_approval")
|
||||
T16_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T16 exit code 0 (generic-approval comment + team member)" "0" "$T16_RC"
|
||||
assert_contains "T16 comment fallback notice" "comment-based approval" "$T16_OUT"
|
||||
assert_eq "T16 exit code 1 (generic-approval comment rejected — not an official review)" "1" "$T16_RC"
|
||||
assert_contains "T16 no candidates error" "no candidates from reviews API or issue comments" "$T16_OUT"
|
||||
|
||||
# T17 — no approval keywords in comments → exit 1
|
||||
echo
|
||||
@@ -363,16 +379,16 @@ T17_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T17 exit code 1 (no candidates from comments)" "1" "$T17_RC"
|
||||
assert_contains "T17 no candidates error" "no candidates from reviews API or issue comments" "$T17_OUT"
|
||||
|
||||
# T18 — a wrong-team PR review candidate must not suppress a right-team
|
||||
# comment approval. This matches PR #1790, where QA had an APPROVED review
|
||||
# and security approved via the agent comment convention.
|
||||
# T18 — wrong-team review + right-team comment → exit 1
|
||||
# SECURITY: with comment approval fully removed, a wrong-team review plus
|
||||
# a right-team comment yields NO valid candidates. Only official reviews
|
||||
# from the target team count.
|
||||
echo
|
||||
echo "== T18 review candidate wrong team, comment candidate right team =="
|
||||
T18_OUT=$(run_review_check "T18_review_wrong_team_comment_right_team")
|
||||
T18_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T18 exit code 0 (comment approval still considered)" "0" "$T18_RC"
|
||||
assert_contains "T18 comment candidate notice" "comment-based approval" "$T18_OUT"
|
||||
assert_contains "T18 comment approver accepted" "APPROVED by core-qa-agent" "$T18_OUT"
|
||||
assert_eq "T18 exit code 1 (comment approval removed — no valid candidates)" "1" "$T18_RC"
|
||||
assert_contains "T18 none are in team" "none are in team" "$T18_OUT"
|
||||
|
||||
# T19 — ai-sop-ack member APPROVED review must NOT count toward qa-review
|
||||
# or security-review (R1 hardening refinement, msg 1388c76f).
|
||||
@@ -393,6 +409,24 @@ assert_eq "T20 exit code 1 (ai-sop-ack not in security team)" "1" "$T20_RC"
|
||||
assert_contains "T20 ai-reviewer excluded from security" "candidates: ai-reviewer" "$T20_OUT"
|
||||
assert_contains "T20 none are in security team" "none are in team" "$T20_OUT"
|
||||
|
||||
# T21 — stale-head APPROVED review must be rejected (commit_id mismatch).
|
||||
# SECURITY: an approval on an old commit does not cover the current head.
|
||||
echo
|
||||
echo "== T21 stale-head APPROVED review rejected =="
|
||||
T21_OUT=$(run_review_check "T21_stale_head_approved")
|
||||
T21_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T21 exit code 1 (stale-head approval rejected)" "1" "$T21_RC"
|
||||
assert_contains "T21 no candidates error" "no candidates from reviews API or issue comments" "$T21_OUT"
|
||||
|
||||
# T22 — missing/non-official APPROVED review must be rejected.
|
||||
# SECURITY: only official Gitea reviews count; comments and non-official reviews lack audit trail.
|
||||
echo
|
||||
echo "== T22 missing official flag APPROVED review rejected =="
|
||||
T22_OUT=$(run_review_check "T22_missing_official")
|
||||
T22_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T22 exit code 1 (missing official rejected)" "1" "$T22_RC"
|
||||
assert_contains "T22 no candidates error" "no candidates from reviews API or issue comments" "$T22_OUT"
|
||||
|
||||
echo
|
||||
echo "------"
|
||||
echo "PASS=$PASS FAIL=$FAIL"
|
||||
|
||||
@@ -208,6 +208,22 @@ class TestParseDirectives(unittest.TestCase):
|
||||
d = self.parse_ack_revoke("/sop-ack Comprehensive_Testing")
|
||||
self.assertEqual(d[0][1], "comprehensive-testing")
|
||||
|
||||
def test_emdash_separator_parsed_correctly(self):
|
||||
# Em-dash (U+2014) between slug and note is common in practice.
|
||||
# /sop-ack Five-Axis — five-axis-review
|
||||
# → slug = five-axis, note = — five-axis-review
|
||||
d = self.parse_ack_revoke("/sop-ack Five-Axis — five-axis-review")
|
||||
self.assertEqual(len(d), 1)
|
||||
self.assertEqual(d[0][1], "five-axis")
|
||||
self.assertIn("five-axis-review", d[0][2])
|
||||
|
||||
def test_emdash_no_note(self):
|
||||
# Em-dash at end of slug: only slug, no note content
|
||||
d = self.parse_ack_revoke("/sop-ack Five-Axis —")
|
||||
self.assertEqual(len(d), 1)
|
||||
self.assertEqual(d[0][1], "five-axis")
|
||||
self.assertEqual(d[0][2], "") # em-dash is separator-only → empty note
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# section_marker_present
|
||||
|
||||
@@ -205,5 +205,5 @@ n/a_gates:
|
||||
required_teams: [security, managers, ceo]
|
||||
description: >-
|
||||
Security review N/A when this change has no security surface
|
||||
(docs-only, pure-frontend, dependency-only). A security/owners
|
||||
(docs-only, pure-frontend, dependency-only). A security/managers/ceo
|
||||
member must post /sop-n/a security-review to activate.
|
||||
|
||||
@@ -34,11 +34,6 @@ jobs:
|
||||
check:
|
||||
name: Block forbidden paths
|
||||
runs-on: ubuntu-latest
|
||||
# Phase 3 (RFC #219 §1): surface broken workflows without blocking
|
||||
# the PR. Follow-up PR flips this off after surfaced defects are
|
||||
# triaged.
|
||||
# mc#1982: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
steps:
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
|
||||
@@ -7,10 +7,13 @@ name: gitea-merge-queue
|
||||
# the user-space queue bot, one PR per tick, using the non-bypass merge actor.
|
||||
#
|
||||
# Queue contract:
|
||||
# - add label `merge-queue` to an open same-repo PR
|
||||
# - auto-discovery (default): any open same-repo PR is considered — no
|
||||
# `merge-queue` label required (the label is optional metadata now)
|
||||
# - bot updates stale PR heads with current main, then waits for CI
|
||||
# - bot merges only when current main is green and required PR contexts pass
|
||||
# - add `merge-queue-hold` to pause a queued PR without removing it
|
||||
# - bot merges only when current main is green, genuine approvals are present
|
||||
# on the current head, required PR contexts pass, and the PR is mergeable
|
||||
# - add `merge-queue-hold`, `do-not-auto-merge`, or `wip` to keep a PR OUT of
|
||||
# autonomous merging; draft PRs are also skipped
|
||||
|
||||
on:
|
||||
# Schedule moved to operator-config:
|
||||
@@ -48,10 +51,34 @@ jobs:
|
||||
WATCH_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
QUEUE_LABEL: merge-queue
|
||||
HOLD_LABEL: merge-queue-hold
|
||||
# Auto-discovery (opt-OUT). When on (default), the queue considers ALL
|
||||
# open same-repo PRs that meet the merge bar — it does NOT wait for a
|
||||
# human/agent to add `merge-queue`. Agent Gitea tokens lack
|
||||
# write:issue (labels are issue-scoped) and could never self-label,
|
||||
# which stalled the queue; the label is now OPTIONAL metadata. The
|
||||
# merge bar is UNCHANGED — only candidate selection widens. Set
|
||||
# AUTO_DISCOVER=0 to restore legacy opt-IN (require the merge-queue
|
||||
# label to be considered).
|
||||
AUTO_DISCOVER: "1"
|
||||
# Opt-OUT labels: any of these on a PR keeps it OUT of autonomous
|
||||
# merging (the human escape hatch). HOLD_LABEL is always also honoured.
|
||||
# A human who wants a PR held just adds one of these labels.
|
||||
OPT_OUT_LABELS: do-not-auto-merge,wip
|
||||
UPDATE_STYLE: merge
|
||||
REQUIRED_CONTEXTS: >-
|
||||
CI / all-required (pull_request),
|
||||
sop-checklist / all-items-acked (pull_request)
|
||||
# Recognised official-reviewer set. A merge needs >= required_approvals
|
||||
# DISTINCT genuine official approvals from these accounts on the
|
||||
# CURRENT head sha (not stale/dismissed). The required_approvals count
|
||||
# itself is read from branch protection at runtime.
|
||||
REVIEWER_SET: agent-reviewer,agent-researcher,agent-reviewer-cr2
|
||||
# NOTE: REQUIRED_CONTEXTS is no longer the authoritative PR gate. The
|
||||
# queue now reads the required status contexts from BRANCH PROTECTION
|
||||
# (status_check_contexts) so non-required governance reds (qa-review,
|
||||
# security-review, sop-tier, sop-checklist when not branch-required,
|
||||
# E2E Chat, Staging SaaS, ci-arm64-advisory) cannot block a merge.
|
||||
# If branch protection cannot be enumerated the queue HOLDS
|
||||
# (fail-closed). REQUIRED_APPROVALS below is only a fallback used when
|
||||
# branch protection does not specify required_approvals.
|
||||
REQUIRED_APPROVALS: "2"
|
||||
# Push-side required contexts. Checking CI / all-required (push)
|
||||
# explicitly instead of the combined state avoids false-pause when
|
||||
# non-blocking jobs (continue-on-error: true) have failed — those
|
||||
|
||||
@@ -61,11 +61,9 @@ name: Lint pre-flip continue-on-error
|
||||
# feedback_no_shared_persona_token_use.
|
||||
#
|
||||
# Phase contract (RFC internal#219 §1 ladder):
|
||||
# - This workflow lands at `continue-on-error: true` (Phase 3 —
|
||||
# surface defects without blocking). Follow-up PR flips it to
|
||||
# `false` ONLY after this workflow's own recent runs on `main`
|
||||
# are confirmed clean — exactly the discipline the workflow
|
||||
# itself enforces. Eat your own dogfood.
|
||||
# - Flipped to `continue-on-error: false` after Researcher live-verified
|
||||
# clean runs. The script's own 35 pytest tests pass and recent PR
|
||||
# history shows no masked regressions — the gate is now enforcing.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
@@ -97,10 +95,9 @@ jobs:
|
||||
name: Verify continue-on-error flips have run-log proof
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 8
|
||||
# Phase 3 (RFC internal#219 §1): surface broken flips without blocking
|
||||
# the PR yet. Follow-up flips this to `false` once the workflow itself
|
||||
# has clean recent runs on main. mc#1982 interim — remove when CoE→false.
|
||||
continue-on-error: true # mc#1982
|
||||
# Fail-closed: the lint script is verified clean (35/35 tests pass,
|
||||
# Researcher live-check confirmed). Masking removed per mc#1982 close-out.
|
||||
continue-on-error: false
|
||||
steps:
|
||||
- name: Check out PR head (full history for base-SHA access)
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
@@ -93,12 +93,12 @@ For "do we have any backend?", use `HasProvisioner()`, never bare `h.provisioner
|
||||
3. **Restart divergence on runtime changes.** Docker re-reads `/configs/config.yaml` from the container before stop, so a changed `runtime:` survives a restart even if the DB isn't synced. EC2 trusts the DB only. If you change the runtime via the Config tab and the handler races the restart, Docker will land on the new runtime, EC2 will land on the old one. **Fix path:** make the Config-tab save explicitly flush to DB before kicking off a restart, not deferred.
|
||||
4. **Console-output asymmetry.** Users debugging a stuck workspace on Docker see `docker logs`; on EC2 they see `GetConsoleOutput`. The two outputs look nothing alike. **Fix path:** expose a unified `GET /workspaces/:id/boot-log` that proxies to whichever backend serves the data. Already partly there via `cp_provisioner.Console`.
|
||||
5. **Template script drift.** `install.sh` and `start.sh` in each template repo do the same high-level work (install hermes-agent, write .env, write config.yaml, start gateway) but must be kept byte-level consistent on the provider-key forwarding block. Easy to forget. Enforced now by `tools/check-template-parity.sh` (see below) — run it in each template repo's CI.
|
||||
6. **Both backends panic when underlying client is nil.** Discovered by the contract-test scaffold landing in this PR: `Provisioner.{Stop,IsRunning}` nil-dereferences the Docker client, and `CPProvisioner.{Stop,IsRunning}` nil-dereferences `httpClient`. The real code always sets these, so this is theoretical in prod — but it means the contract runner can't execute scenarios against zero-value backends. **Fix path:** guard each method with `if p.docker == nil { return false, errNoBackend }` (and equivalent for CP), then flip the `t.Skip` in the contract tests to `t.Run`.
|
||||
6. **Both backends panic when underlying client is nil.** ✅ **Resolved** (`fix/provisioner-nil-guards-1813`). `Provisioner.{Stop,IsRunning}` and `CPProvisioner.{Stop,IsRunning}` now guard against nil clients with `ErrNoBackend`, so the contract-test runner executes scenarios against zero-valued backends without panic.
|
||||
|
||||
## Enforcement
|
||||
|
||||
- **`tools/check-template-parity.sh`** (this repo) — ensures `install.sh` and `start.sh` in a template repo forward identical sets of provider keys. Wire into each template repo's CI as `bash $MONOREPO/tools/check-template-parity.sh install.sh start.sh`.
|
||||
- **Contract tests** (stub) — `workspace-server/internal/provisioner/backend_contract_test.go` defines the behaviors every `provisioner.Provisioner` implementation must satisfy. Fails compile when a method drifts between `Docker` and `CPProvisioner`. Scenario-level runs are `t.Skip`'d today pending drift risk #6 (see above) — compile-time assertions still catch method drift.
|
||||
- **Contract tests** — `workspace-server/internal/provisioner/backend_contract_test.go` defines the behaviors every `provisioner.Provisioner` implementation must satisfy. Fails compile when a method drifts between `Docker` and `CPProvisioner`. Scenario-level runs execute against zero-valued backends since drift risk #6 was resolved (`fix/provisioner-nil-guards-1813`).
|
||||
- **Source-level dispatcher pins** — `workspace_provision_auto_test.go` enforces the SoT pattern documented above:
|
||||
- `TestNoCallSiteCallsDirectProvisionerExceptAuto` — no handler calls `.provisionWorkspace(` or `.provisionWorkspaceCP(` directly outside the dispatcher's allowlist.
|
||||
- `TestNoCallSiteCallsBareStop` — no handler calls `.provisioner.Stop(` or `.cpProv.Stop(` directly outside the dispatcher's allowlist (strips Go comments before substring match so archaeology in code comments doesn't trip the gate).
|
||||
|
||||
@@ -0,0 +1,293 @@
|
||||
# RFC: Org-level Platform Agent — a tenant-resident concierge
|
||||
|
||||
**Perspective:** CTO + Backend Engineer + DevOps
|
||||
**Status:** Draft — pre-implementation, **CTO sign-off required before any implementation PR**
|
||||
**Scope:** `molecule-core` (workspace-server), `molecule-controlplane`, workspace runtime, `molecule-app`
|
||||
**This document is the single source of truth (SSOT) for the feature.** Code, OpenAPI, the platform
|
||||
MCP, and end-user docs reconcile to this RFC — not to each other.
|
||||
|
||||
---
|
||||
|
||||
## 1. Summary
|
||||
|
||||
Today a Molecule tenant is a control/router box: one EC2 runs the `workspace-server`
|
||||
(`molecule-tenant` container) + Postgres + Redis, and **each workspace is its own separate EC2**
|
||||
running a runtime image that joins the tenant's A2A mesh. A2A has exactly two participant kinds:
|
||||
**workspaces** (agents) and the **user** (the canvas, modeled implicitly as `activity_logs.source_id
|
||||
IS NULL`). A user who wants to *do* anything must drive individual workspaces directly — create them,
|
||||
assign agents, wire channels/schedules/secrets — i.e. they must carry a lot of platform knowledge.
|
||||
|
||||
This RFC introduces a **platform agent**: an always-on org-level agent that
|
||||
|
||||
1. runs as a **container on the tenant EC2** itself (beside `molecule-tenant`),
|
||||
2. natively holds the **platform-management MCP** (the org-admin tool surface) so it can do anything
|
||||
in the org,
|
||||
3. joins A2A as a **first-class third participant** (`kind='platform'`) that sits at the org root, and
|
||||
4. becomes the **user's default chat target** — a concierge the user talks to like a chatbot, which
|
||||
then orchestrates the org on their behalf.
|
||||
|
||||
Destructive actions the concierge triggers are **human-approved** through the existing approvals
|
||||
subsystem.
|
||||
|
||||
## 2. Motivation
|
||||
|
||||
- **Lower the knowledge floor.** "Spin up an SEO team and have them publish weekly" should be a
|
||||
sentence, not a sequence of workspace/agent/schedule/secret operations.
|
||||
- **One front door.** A single conversational entry point that *is* the org, instead of N per-workspace
|
||||
chats the user has to coordinate.
|
||||
- **Reuse, don't rebuild.** The agent runtime, A2A mesh, the 87-tool platform MCP, and the approvals
|
||||
subsystem already exist. This feature is mostly *composition* plus one honest new participant kind.
|
||||
|
||||
## 3. Goals / Non-Goals
|
||||
|
||||
**Goals**
|
||||
- A per-tenant platform agent, provisioned automatically, that controls the org via the platform MCP.
|
||||
- A first-class `platform` participant in A2A with correct routing and tenant isolation.
|
||||
- Server-side approval gating for destructive org operations.
|
||||
- Parity with normal workspaces for runtime/model/provider/billing (no special-casing).
|
||||
|
||||
**Non-Goals (this RFC)**
|
||||
- Replacing the canvas. The canvas remains the advanced/power-user surface.
|
||||
- Multi-concierge / per-team concierges. Exactly **one** platform agent per org.
|
||||
- A new scoped-down token system for the MCP (tracked separately; see §10 Open Questions).
|
||||
|
||||
## 4. Current-state ground truth (verified, with references)
|
||||
|
||||
- **Topology.** Tenant EC2 runs `molecule-tenant` (workspace-server) + Postgres + Redis;
|
||||
`controlplane/internal/provisioner/ec2.go:buildTenantUserDataSM()` `docker run`s it with
|
||||
`--network host`, `PORT=8080`. Each **workspace is its own EC2** (`ec2.go:ProvisionWorkspace`).
|
||||
- **No `org_id` column.** An "org" is the `parent_id IS NULL` subtree root;
|
||||
`workspace-server/internal/handlers/org_scope.go` resolves it with a recursive CTE (`orgRootID`) and
|
||||
`sameOrg()` compares two workspaces' resolved roots for tenant isolation (#1953/OFFSEC-015).
|
||||
- **A2A authorization is hierarchy-based.** `workspace-server/internal/registry/access.go:CanCommunicate`
|
||||
permits self / siblings / ancestor↔descendant. Root-level rows are "siblings" but every routing path
|
||||
is additionally gated by `sameOrg()`.
|
||||
- **No participant-kind discriminator.** `workspaces.role` is a free-form string; the user is implicit
|
||||
(`activity_logs.source_id IS NULL`). `migrations/001_workspaces.sql`.
|
||||
- **Runtime injects MCP servers** in the claude-code executor's `mcp_servers` dict — today exactly one
|
||||
entry, `"a2a"` (`molecule-ai-workspace-template-claude-code/claude_sdk_executor.py`,
|
||||
`molecule_runtime/claude_sdk_executor.py`). The agent self-registers via `POST /registry/register`
|
||||
(`molecule_runtime/main.py`) and is identified by `WORKSPACE_ID` + `X-Molecule-Org-Id`.
|
||||
- **Platform MCP** (`molecule-mcp-server`, stdio Node) authenticates purely from env
|
||||
(`MOLECULE_API_KEY` = org-admin token, `MOLECULE_API_URL`, `MOLECULE_ORG_ID`; `src/api.ts`), is a
|
||||
thin proxy over the tenant REST/A2A API (`chat_with_agent` → `POST /workspaces/:id/a2a`,
|
||||
`async_delegate` → `/delegate`), and has **zero embeddability blockers**.
|
||||
- **Billing** is a per-workspace resolver — `ResolveLLMBillingModeDerived`
|
||||
(`workspace-server/internal/handlers/workspace_provision.go`, `llm_billing_mode.go`), defaulting
|
||||
closed to `platform_managed`; `byok` runs on the tenant's own provider key (see
|
||||
`docs/architecture/byok-fail-closed-billing.md`).
|
||||
- **Approvals** exist: `migrations/007_approvals.sql`, `internal/handlers/approvals.go`,
|
||||
`EventApprovalRequested`, decide route `POST /workspaces/:id/approvals/:approvalId/decide`.
|
||||
|
||||
## 5. Design
|
||||
|
||||
### 5.1 The platform agent IS the org root
|
||||
|
||||
Because `sameOrg()` resolves each workspace to its topmost `parent_id IS NULL` root, a platform agent
|
||||
added as a *second* root would resolve to a *different* root than the existing team and be **blocked**
|
||||
by `sameOrg`. Therefore the platform agent **becomes the single org root**, and the org's existing
|
||||
root is **re-parented under it**. Consequences:
|
||||
|
||||
- `orgRootID(any workspace) == platform-agent-id`; `sameOrg(platform, any in-org ws) == true`.
|
||||
- The platform agent reaches every workspace (and is reachable) via the **existing**
|
||||
ancestor↔descendant rules — **no `CanCommunicate` change**, and tenant isolation is unchanged.
|
||||
|
||||
This is the honest realization of "a third participant above workspace and user": the concierge is
|
||||
literally the org.
|
||||
|
||||
### 5.2 `kind` discriminator (the only new marker)
|
||||
|
||||
Add a single column `workspaces.kind TEXT NOT NULL DEFAULT 'workspace'`, constrained to
|
||||
`('workspace','platform')`. It is the **only** marker of the platform agent — we do **not** also
|
||||
encode identity in `role`/`tier` (those stay descriptive). The enum is defined once: the migration
|
||||
`CHECK` and the Go constants `KindWorkspace`/`KindPlatform` (+ one `IsValidKind`) are kept in lockstep.
|
||||
|
||||
Invariants (handler-enforced, since there is no `org_id` for a pure-SQL unique):
|
||||
- `kind='platform' ⇒ parent_id IS NULL`.
|
||||
- A row may be `kind='platform'` only if it is its own org root (`orgRootID(self) == self`), giving
|
||||
"exactly one platform agent per org". Guard the check+write in a tx with `FOR UPDATE` on the root.
|
||||
|
||||
### 5.3 Identity & registration
|
||||
|
||||
- **ID** = derived `uuidv5(org-namespace, "platform-agent")` — reproducible, no stored-vs-derived
|
||||
drift, lowercase so it satisfies the runtime's `WORKSPACE_ID` validator.
|
||||
- CP **pre-seeds** the `workspaces` row (`kind='platform'`, `parent_id=NULL`, `tier=0`) before the
|
||||
agent boots; the agent self-registers (`POST /registry/register`) into that row. `Register` accepts
|
||||
an optional `kind` and reconciles it, enforcing the §5.2 invariants.
|
||||
|
||||
### 5.4 Default-target resolver
|
||||
|
||||
New `GET /registry/platform-agent` (handler `internal/handlers/platform_agent.go`): resolve the
|
||||
caller's `orgRootID()` and return it iff `kind='platform'`. This is the server hook the dashboard
|
||||
targets by default; no change to `ProxyA2A`. **Authored in the OpenAPI SSOT first**; MCP/CLI/docs
|
||||
derive from it.
|
||||
|
||||
### 5.5 Runtime: two MCPs, config-driven
|
||||
|
||||
Make the runtime's `mcp_servers` **config-driven** rather than hardcoded:
|
||||
- `molecule_runtime/config.py`: add `extra_mcp_servers: list[dict]` to `WorkspaceConfig`, read
|
||||
`raw.get("mcp_servers", [])`.
|
||||
- Both executors merge `extra_mcp_servers` into the `mcp_servers` dict after the always-on `"a2a"`
|
||||
entry (the template `claude_sdk_executor.py` is the live one; the runtime-package copy is the
|
||||
fallback).
|
||||
|
||||
The platform agent's `config.yaml` then declares:
|
||||
|
||||
```yaml
|
||||
runtime: claude-code
|
||||
model: sonnet # default; user-switchable model AND provider via providers.yaml
|
||||
a2a:
|
||||
port: 8090 # avoid the workspace default 8000 under host networking
|
||||
mcp_servers:
|
||||
- name: platform
|
||||
command: node
|
||||
args: ["/opt/molecule-mcp-server/dist/index.js"]
|
||||
```
|
||||
|
||||
The `platform` MCP reads `MOLECULE_API_KEY`/`MOLECULE_API_URL`/`MOLECULE_ORG_ID` from the container
|
||||
env (passed through to the stdio child) — no per-server `env` block needed.
|
||||
|
||||
### 5.6 Hosting & provisioning (tenant EC2 container)
|
||||
|
||||
In `ec2.go:buildTenantUserDataSM()` add a `start_platform_agent` stage **after** `wait_platform_health`
|
||||
(the agent registers against `localhost:8080` on boot):
|
||||
|
||||
```bash
|
||||
docker run -d --restart=always --name molecule-platform-agent --network host \
|
||||
-v /data/platform-agent/configs:/configs \
|
||||
-e WORKSPACE_ID=<platform-uuid> -e WORKSPACE_CONFIG_PATH=/configs \
|
||||
-e PLATFORM_URL=http://localhost:8080 \
|
||||
-e MOLECULE_API_URL=http://localhost:8080 -e MOLECULE_API_KEY=$ADMIN_TOKEN -e MOLECULE_ORG_ID=<orgID> \
|
||||
-e ANTHROPIC_AUTH_TOKEN=$ADMIN_TOKEN -e MOLECULE_LLM_ANTHROPIC_BASE_URL=$MOLECULE_LLM_ANTHROPIC_BASE_URL \
|
||||
<platform-agent-image>
|
||||
```
|
||||
|
||||
- The org `admin_token` is already on the box (Secrets Manager `molecule/tenant/{orgID}`).
|
||||
- `--restart=always` provides Docker-level supervision (matches `molecule-tenant`).
|
||||
- Mirror the block into the redeploy path (`buildRedeployScript`) so existing tenants backfill it.
|
||||
|
||||
### 5.7 Image
|
||||
|
||||
A **dedicated `molecule-platform-agent` image**: `FROM workspace-template-claude-code`, `COPY` the
|
||||
prebuilt `molecule-mcp-server/dist` + `node_modules` into `/opt/molecule-mcp-server`, and **pin Node
|
||||
20** (the slim base ships Node 18; the MCP expects ≥20). A dedicated image keeps the org-admin MCP
|
||||
**out of** ordinary workspace images (security hygiene) and lets us set concierge defaults without
|
||||
touching the workspace template. `molecule-ci` publishes it.
|
||||
|
||||
### 5.8 Approval gate (server-side trust boundary)
|
||||
|
||||
The MCP is a *client* of the tenant handlers, so enforcement lives in the **handlers**, not the MCP.
|
||||
|
||||
- `internal/approvals/policy.go` (new): one auditable map of gated actions —
|
||||
`delete_workspace`, `deprovision`, `secret_write`, `org_token_mint`.
|
||||
- `requireApproval(ctx, workspaceID, action, contextHash)` reuses the existing approvals
|
||||
INSERT/broadcast/escalate. If an `approved`+unconsumed row matches → consume it → proceed. Else
|
||||
create a `pending` row, broadcast `EventApprovalRequested`, and return **HTTP 202
|
||||
`{approval_id, status:"pending"}`** instead of executing. The human decides via the existing decide
|
||||
route; the agent retries and the gate now passes.
|
||||
- Add `approval_requests.consumed_at` (single-use) and optional `request_hash` (dedupe identical
|
||||
pending requests).
|
||||
- **Escalation:** the platform agent's `parent_id` is NULL, so platform-originated approvals escalate
|
||||
to the **user** (canvas notify), not a parent.
|
||||
- The 202 response shape is authored in the **OpenAPI SSOT**.
|
||||
|
||||
### 5.9 Billing & model/provider parity
|
||||
|
||||
The platform agent is a `workspaces` row, so it inherits the one billing resolver and the
|
||||
`providers.yaml` runtime matrix unchanged:
|
||||
- **Default `platform_managed`** (metered CP proxy, billed to org credits) — the env wiring in §5.6.
|
||||
- **`byok`** = flip `/admin/workspaces/:id/llm-billing-mode` + supply the org's `ANTHROPIC_API_KEY`
|
||||
secret (workspace or global). Exposed as a provisioning flag so a tenant can choose at create time.
|
||||
- Model **and provider** are switchable (Claude, Kimi-for-coding, …) via the same dashboard
|
||||
model-switcher any workspace uses.
|
||||
|
||||
### 5.10 UX (summary; detailed in app RFC / Phase 5)
|
||||
|
||||
The **dashboard** (`molecule-app`) becomes the primary entry: a concierge chat (default-targeting the
|
||||
§5.4 resolver) plus a live org overview, with pending approvals surfaced inline. The **canvas** stays
|
||||
for advanced users. First UI version is produced in Claude Design and iterated before build.
|
||||
|
||||
## 6. SSOT mapping (derive, don't fork)
|
||||
|
||||
| Concern | Single source of truth | This RFC's rule |
|
||||
|---|---|---|
|
||||
| "The org" | `orgRootID()`/`sameOrg()` (`org_scope.go`) | platform agent *becomes* the root; no `org_id` column |
|
||||
| Platform marker | `workspaces.kind` | `kind` only; never also `role`/`tier` |
|
||||
| Model/provider | `providers.yaml` runtime matrix | concierge switches via the same registry |
|
||||
| LLM billing | `ResolveLLMBillingModeDerived` | inherits the one resolver; no new path |
|
||||
| Config/secrets delivery | tenant Secrets Manager bundle (`seedWorkspaceConfigSecret`) | no new S3 prefix / second store |
|
||||
| Management API | OpenAPI spec | new endpoints authored there first; MCP/CLI/docs derive |
|
||||
| Gated actions | `internal/approvals/policy.go` | one map |
|
||||
| Platform-agent id | `uuidv5(org, "platform-agent")` | derived, never stored separately |
|
||||
|
||||
## 7. Security & blast radius
|
||||
|
||||
The concierge holds the org **admin token** (full tenant-root, self-minting) and is driven by
|
||||
end-user chat. Mitigations:
|
||||
- **Approval gate (§5.8)** must ship *with* the agent going user-facing, not after. Until then the
|
||||
agent is operator-only.
|
||||
- **Tenant isolation** is unchanged — every reach path still passes `sameOrg()`.
|
||||
- **MCP not in workspace images** (dedicated image, §5.7); the admin token lives only in the
|
||||
platform-agent container env on the tenant box.
|
||||
- **Token rotation:** the MCP reads env once at spawn → rotation = `docker restart
|
||||
molecule-platform-agent` (runbook item).
|
||||
- Future: a scoped-down org token (no delete/billing/member) — see §10.
|
||||
|
||||
## 8. Migration & rollout
|
||||
|
||||
Phase ordering is the rollout contract:
|
||||
- **Phase 0** (schema) ships and bakes before anything writes `kind`. Backward-compatible: every
|
||||
existing row defaults to `kind='workspace'`; the `CHECK` is added `NOT VALID` then validated.
|
||||
- **Phase 1 re-parenting backfill** is the one real watch-item. **Before** running it, audit whether
|
||||
any org-scoped table keys off the *root workspace id* (e.g. `org_api_tokens`, `org_plugin_allowlist`)
|
||||
versus the CP org UUID. If they reference the root workspace id, re-parenting changes "the root" and
|
||||
those refs must migrate too. The backfill is per-org, idempotent, and reversible.
|
||||
- New orgs get the platform agent from first boot; existing orgs backfill via `/admin/tenants
|
||||
redeploy` + a one-time re-parent migration.
|
||||
|
||||
## 9. Implementation phases
|
||||
|
||||
0. **Schema + model** (`molecule-core`): `kind` column + `approval_requests.consumed_at`; model field +
|
||||
constants; `Register` accepts/validates `kind` with invariants.
|
||||
1. **Platform-as-root + resolver** (`molecule-core` + CP): CP pre-seeds the platform row and creates
|
||||
teams under it; per-org re-parent backfill (after the §8 audit); `GET /registry/platform-agent`.
|
||||
2. **Config-driven two-MCP runtime** (runtime + claude-code template).
|
||||
3. **Image + tenant provisioning** (CP + image + `molecule-ci`): dedicated image; `start_platform_agent`
|
||||
in user-data + redeploy; config via the tenant Secrets Manager bundle; billing knob.
|
||||
4. **Approval gate** (`molecule-core`): policy map + `requireApproval` at destructive handlers; OpenAPI
|
||||
202 shape.
|
||||
5. **Dashboard concierge UX** (`molecule-app`): design-first, then build against the resolver.
|
||||
6. **Cleanup**: exclude the platform agent from billable counts; canvas visibility; rotation runbook.
|
||||
|
||||
## 10. Open questions
|
||||
|
||||
- **Scoped-down token.** Should the concierge hold a reduced-scope token (no delete/billing/member)
|
||||
instead of full admin + an approval gate? The token-scope system does not exist yet (`orgtoken`
|
||||
TODO). Recommendation: ship admin-token + approval gate now; add scope-down as a follow-up.
|
||||
- **Re-parenting vs. wrapper.** If product later wants a platform agent that is *not* the topological
|
||||
root, a `CanCommunicateWithKind` wrapper (guarded by `sameOrg`) is the alternative. Deferred —
|
||||
platform-as-root is lower-risk and needs zero access-control change.
|
||||
- **Canvas visibility** of the root concierge node (hide vs. show as the org anchor).
|
||||
|
||||
## 11. Verification (end-to-end on a staging tenant)
|
||||
|
||||
1. **Schema:** Phase-0 migrations applied; existing workspaces report `kind='workspace'`; `go test
|
||||
./...` + `-tags=integration` green.
|
||||
2. **Provision:** redeploy a staging tenant; `docker ps` shows `molecule-platform-agent` healthy; its
|
||||
logs show a successful `/registry/register`.
|
||||
3. **Identity:** the platform row is `kind='platform'`, `parent_id IS NULL`; the former root now has
|
||||
`parent_id = <platform id>`; `GET /registry/platform-agent` returns it.
|
||||
4. **Reach:** chat the platform agent → it `list_workspaces` then `create_workspace` via the platform
|
||||
MCP and reports back via `send_message_to_user`.
|
||||
5. **Isolation:** it reaches every workspace in its org and **cannot** reach another tenant's
|
||||
workspace.
|
||||
6. **Approval gate:** `delete_workspace` → HTTP 202 pending + approval event; decide-approve →
|
||||
completes; a second delete with the same approval is rejected (consumed).
|
||||
7. Drive a real concierge flow ("spin up a PM + engineer to build X") and watch the delegation/activity
|
||||
ledger.
|
||||
|
||||
---
|
||||
|
||||
*Derived from a read-only multi-agent source audit of `molecule-core`, `molecule-controlplane`,
|
||||
`molecule-ai-workspace-runtime`, `molecule-ai-workspace-template-claude-code`, and
|
||||
`molecule-mcp-server`. No secret values recorded.*
|
||||
@@ -8,26 +8,39 @@ against the latest `main`.
|
||||
|
||||
## Queue Contract
|
||||
|
||||
Add the `merge-queue` label to an open PR when it is ready to merge.
|
||||
**Auto-discovery (opt-OUT, default).** You do NOT need to label a PR. The bot
|
||||
auto-discovers every open same-repo PR and merges any that meets the bar. The
|
||||
`merge-queue` label is now optional metadata, not a gate. This removed the
|
||||
historical autonomy gap: agent Gitea tokens lack `write:issue` (labels are
|
||||
issue-scoped), so agents could never self-label and ready PRs stalled.
|
||||
|
||||
To keep a PR OUT of autonomous merging, add an opt-OUT label:
|
||||
`merge-queue-hold`, `do-not-auto-merge`, or `wip`. Draft PRs are also skipped.
|
||||
|
||||
The bot processes one PR per tick:
|
||||
|
||||
1. Confirms `main` is green.
|
||||
2. Selects the oldest open PR carrying `merge-queue`.
|
||||
3. Skips PRs with `merge-queue-hold`.
|
||||
4. Rejects fork PRs because the queue may only update same-repo branches.
|
||||
5. If the PR head does not contain current `main`, calls Gitea's
|
||||
1. Confirms `main`'s branch-protection-required push contexts are green.
|
||||
2. Selects the oldest open same-repo PR that is NOT opt-out-labeled and NOT a
|
||||
draft (auto-discovery). With `AUTO_DISCOVER=0` it falls back to legacy
|
||||
opt-IN: only PRs carrying `merge-queue` are considered.
|
||||
3. Rejects fork PRs because the queue may only update same-repo branches.
|
||||
4. If the PR head does not contain current `main`, calls Gitea's
|
||||
`/pulls/{n}/update?style=merge` endpoint and waits for CI on the new head.
|
||||
6. Merges only after the current PR head has required contexts green:
|
||||
- `CI / all-required (pull_request)`
|
||||
- `sop-checklist / all-items-acked (pull_request)`
|
||||
5. Merges only when, on the PR's CURRENT head sha:
|
||||
- `>= required_approvals` distinct genuine official `APPROVED` reviews from
|
||||
the recognised reviewer set (read from branch protection; default 2),
|
||||
- no open official `REQUEST_CHANGES`,
|
||||
- every branch-protection-required status context is green, and
|
||||
- the PR is `mergeable` (Gitea returns `True`; `None`/`False` = wait).
|
||||
|
||||
The workflow is serialized with `concurrency`, so two queued PRs cannot be
|
||||
The merge bar is unchanged by auto-discovery — only WHICH PRs are considered
|
||||
changes. The workflow is serialized with `concurrency`, so two PRs cannot be
|
||||
merged against the same observed `main`.
|
||||
|
||||
## Operator Commands
|
||||
|
||||
Queue a PR:
|
||||
Queue a PR (optional — auto-discovery already considers every ready PR; the
|
||||
label is just visible metadata):
|
||||
|
||||
```bash
|
||||
curl -fsS -X POST \
|
||||
@@ -37,7 +50,8 @@ curl -fsS -X POST \
|
||||
-d '{"labels":["merge-queue"]}'
|
||||
```
|
||||
|
||||
Temporarily hold a queued PR:
|
||||
Keep a PR OUT of autonomous merging (opt-OUT — use `merge-queue-hold`,
|
||||
`do-not-auto-merge`, or `wip`):
|
||||
|
||||
```bash
|
||||
curl -fsS -X POST \
|
||||
@@ -56,9 +70,11 @@ REPO=molecule-ai/molecule-core \
|
||||
WATCH_BRANCH=main \
|
||||
QUEUE_LABEL=merge-queue \
|
||||
HOLD_LABEL=merge-queue-hold \
|
||||
AUTO_DISCOVER=1 \
|
||||
OPT_OUT_LABELS=do-not-auto-merge,wip \
|
||||
REVIEWER_SET=agent-reviewer,agent-researcher,agent-reviewer-cr2 \
|
||||
UPDATE_STYLE=merge \
|
||||
REQUIRED_CONTEXTS='CI / all-required (pull_request),sop-checklist / all-items-acked (pull_request)' \
|
||||
python3 .gitea/scripts/gitea-merge-queue.py
|
||||
python3 .gitea/scripts/gitea-merge-queue.py --dry-run
|
||||
```
|
||||
|
||||
Dry run:
|
||||
|
||||
@@ -1004,6 +1004,12 @@ for wid in "${WS_TO_CHECK[@]}"; do
|
||||
else
|
||||
DIAG_FAIL=$(echo "$DIAG_JSON" | python3 -c "import json,sys; d=json.load(sys.stdin); print(d.get('first_failure','unknown'))" 2>/dev/null || echo "unknown")
|
||||
DIAG_DETAIL=$(echo "$DIAG_JSON" | python3 -c "import json,sys; d=json.load(sys.stdin); s=[x for x in d.get('steps',[]) if not x.get('ok')]; step=s[0] if s else {}; print(' — '.join(x for x in [step.get('error',''), step.get('detail','')] if x))" 2>/dev/null || echo "")
|
||||
# #767: always emit the full diagnose JSON so operators see every step's
|
||||
# Detail field even when the Python extraction above fails or the shape
|
||||
# drifts. The burst is bracketed like steps 2 and 4 for grep-friendly CI.
|
||||
log "── DIAGNOSTIC BURST (step 7b — terminal diagnose for $wid) ──"
|
||||
echo "$DIAG_JSON" | python3 -m json.tool 2>/dev/null || echo "$DIAG_JSON"
|
||||
log "── END DIAGNOSTIC ──"
|
||||
fail "Workspace $wid terminal diagnose failed at step '$DIAG_FAIL': $DIAG_DETAIL — check tenant SG has tcp/22 from the configured EIC endpoint SG, MOLECULE_EIC_ENDPOINT_SG_ID is set in Railway, and EIC endpoint health"
|
||||
fi
|
||||
done
|
||||
|
||||
@@ -1050,12 +1050,13 @@ def test_reap_continues_on_per_sha_apierror(sr_module, monkeypatch, capsys):
|
||||
|
||||
|
||||
def test_main_soft_skips_when_commit_listing_times_out(sr_module, monkeypatch, capsys):
|
||||
"""A transient outage while listing recent commits should not paint main red.
|
||||
"""A transient outage while listing recent commits fails the tick visibly.
|
||||
|
||||
Per-SHA status read failures are already isolated inside `reap_branch`.
|
||||
The real 2026-05-14 failure was earlier: `/commits?sha=main&limit=30`
|
||||
timed out after all retries, aborting the tick. The next 5-minute tick can
|
||||
retry safely, so `main()` should emit an observable warning and return 0.
|
||||
retry safely, but the tick itself must be observable as red (exit 1 + error
|
||||
annotation) so the cron bot alerts on persistent infra issues.
|
||||
"""
|
||||
|
||||
monkeypatch.setattr(sr_module, "scan_workflows", lambda _: {"workflow-without-push": False})
|
||||
@@ -1068,9 +1069,9 @@ def test_main_soft_skips_when_commit_listing_times_out(sr_module, monkeypatch, c
|
||||
monkeypatch.setattr(sr_module, "list_recent_commit_shas", fake_list_recent_commit_shas)
|
||||
monkeypatch.setattr(sys, "argv", ["status-reaper.py"])
|
||||
|
||||
assert sr_module.main() == 0
|
||||
assert sr_module.main() == 1
|
||||
captured = capsys.readouterr()
|
||||
assert "::warning::status-reaper skipped this tick" in captured.out
|
||||
assert "::error::status-reaper cannot run" in captured.out
|
||||
assert '"skipped": true' in captured.out
|
||||
assert '"skip_reason": "commit-list-api-error"' in captured.out
|
||||
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
// Package approvals holds the single source of truth for which destructive
|
||||
// org operations require a human approval before they execute.
|
||||
//
|
||||
// (RFC docs/design/rfc-platform-agent.md — Phase 4)
|
||||
//
|
||||
// The org-level platform agent is driven by end-user chat and holds an org-admin
|
||||
// token, so destructive/irreversible operations it can trigger are gated: the
|
||||
// handler creates a pending approval and returns it instead of executing, and a
|
||||
// human decides via the existing approvals subsystem. Keeping the gated-action
|
||||
// list in ONE map makes the blast-radius boundary auditable in a single place —
|
||||
// a handler not listed here behaves exactly as before.
|
||||
package approvals
|
||||
|
||||
// Action is the canonical identifier of a gated destructive operation. The same
|
||||
// string is stored in approval_requests.action so the gate can match a pending/
|
||||
// approved request to the operation being retried.
|
||||
type Action string
|
||||
|
||||
const (
|
||||
ActionDeleteWorkspace Action = "delete_workspace"
|
||||
ActionDeprovision Action = "deprovision_workspace"
|
||||
ActionSecretWrite Action = "secret_write"
|
||||
ActionOrgTokenMint Action = "org_token_mint"
|
||||
)
|
||||
|
||||
// gated is the set of actions that require a human approval. Add an entry here
|
||||
// (and gate the corresponding handler with requireApproval) to expand the
|
||||
// boundary; remove one to drop a gate. This is the only place the policy lives.
|
||||
var gated = map[Action]bool{
|
||||
ActionDeleteWorkspace: true,
|
||||
ActionDeprovision: true,
|
||||
ActionSecretWrite: true,
|
||||
ActionOrgTokenMint: true,
|
||||
}
|
||||
|
||||
// IsGated reports whether action requires a human approval before executing.
|
||||
func IsGated(action Action) bool {
|
||||
return gated[action]
|
||||
}
|
||||
@@ -63,6 +63,31 @@ func TestSessionSearchReturnsActivityAndMemory(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSessionSearch_DBError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewActivityHandler(broadcaster)
|
||||
|
||||
mock.ExpectQuery("WITH session_items AS").
|
||||
WillReturnError(context.DeadlineExceeded)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/ws-123/session-search?q=test", bytes.NewBufferString(""))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-123"}}
|
||||
|
||||
handler.SessionSearch(c)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500 on DB error, got %d", w.Code)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- Activity List source filter ----------
|
||||
|
||||
func TestActivityList_SourceCanvas(t *testing.T) {
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"log"
|
||||
"net/http"
|
||||
"os"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -18,6 +19,7 @@ import (
|
||||
dockerclient "github.com/docker/docker/client"
|
||||
"github.com/gin-gonic/gin"
|
||||
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/providers"
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/provisioner"
|
||||
)
|
||||
|
||||
@@ -41,10 +43,53 @@ func NewWorkspaceImageService(docker *dockerclient.Client) *WorkspaceImageServic
|
||||
return &WorkspaceImageService{docker: docker}
|
||||
}
|
||||
|
||||
// AllRuntimes is the canonical list mirroring docs/workspace-runtime-package.md.
|
||||
// Update both when a new template is added.
|
||||
var AllRuntimes = []string{
|
||||
"claude-code", "codex", "hermes", "openclaw",
|
||||
// AllRuntimes is the canonical set of workspace runtimes this tenant will
|
||||
// pull/recreate template images for. It is DERIVED from the same providers
|
||||
// manifest SSOT (internal/providers/providers.yaml `runtimes:` block, mirrored
|
||||
// from CP's providers.yaml) that the rest of the platform routes against —
|
||||
// NOT a second hand-maintained list.
|
||||
//
|
||||
// Why derive instead of hardcode (controlplane#578): the old hardcoded slice
|
||||
// here ({claude-code, codex, hermes, openclaw}) silently DRIFTED from CP, which
|
||||
// already accepts `google-adk` for pin-promote/redeploy. A google-adk pin would
|
||||
// be accepted CP-side, then this tenant's POST /admin/workspace-images/refresh
|
||||
// ?runtime=google-adk rejected it 400 ("unknown runtime"), so google-adk image
|
||||
// fixes never deployed. Deriving from the manifest makes the tenant allowlist
|
||||
// and the CP allowlist provably the same set — they can't drift again.
|
||||
//
|
||||
// imageRefreshFallbackRuntimes is used ONLY if the embedded providers manifest
|
||||
// fails to load (which would be a build/CI failure caught by the providers
|
||||
// package's own tests, never a healthy prod). It preserves the historical
|
||||
// behavior — plus google-adk — so a manifest regression can never take the
|
||||
// refresh endpoint fully offline. Kept in lockstep with the providers.yaml
|
||||
// `runtimes:` keys; the drift guard in admin_workspace_images_test.go asserts
|
||||
// the two match.
|
||||
var imageRefreshFallbackRuntimes = []string{
|
||||
"claude-code", "codex", "google-adk", "hermes", "openclaw",
|
||||
}
|
||||
|
||||
// AllRuntimes is computed once at package init from the providers SSOT.
|
||||
var AllRuntimes = loadImageRefreshRuntimes()
|
||||
|
||||
// loadImageRefreshRuntimes returns the sorted runtime names declared in the
|
||||
// providers manifest, falling back to imageRefreshFallbackRuntimes if the
|
||||
// manifest can't be loaded.
|
||||
func loadImageRefreshRuntimes() []string {
|
||||
m, err := providers.LoadManifest()
|
||||
if err != nil || len(m.Runtimes) == 0 {
|
||||
if err != nil {
|
||||
log.Printf("workspace-images: providers.LoadManifest failed (%v); falling back to static runtime allowlist", err)
|
||||
}
|
||||
out := append([]string(nil), imageRefreshFallbackRuntimes...)
|
||||
sort.Strings(out)
|
||||
return out
|
||||
}
|
||||
out := make([]string, 0, len(m.Runtimes))
|
||||
for rt := range m.Runtimes {
|
||||
out = append(out, rt)
|
||||
}
|
||||
sort.Strings(out)
|
||||
return out
|
||||
}
|
||||
|
||||
// RefreshResult is the per-call outcome surfaced to HTTP callers AND logged
|
||||
@@ -197,7 +242,7 @@ func (s *WorkspaceImageService) Refresh(ctx context.Context, runtimes []string,
|
||||
|
||||
// AdminWorkspaceImagesHandler serves POST /admin/workspace-images/refresh.
|
||||
//
|
||||
// ?runtime=claude-code (optional; default = all 8 templates)
|
||||
// ?runtime=claude-code (optional; default = all runtimes in AllRuntimes)
|
||||
// &recreate=true|false (default true; false = pull only)
|
||||
//
|
||||
// Returns JSON {pulled: [...], failed: [...], recreated: [...]}
|
||||
|
||||
@@ -3,7 +3,14 @@ package handlers
|
||||
import (
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"sort"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/providers"
|
||||
)
|
||||
|
||||
func TestGHCRAuthHeader_NoEnvReturnsEmpty(t *testing.T) {
|
||||
@@ -92,6 +99,119 @@ func TestGHCRAuthHeader_RespectsRegistryEnv(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// runtimeListContains is a tiny membership helper for the runtime-allowlist tests.
|
||||
func runtimeListContains(s []string, v string) bool {
|
||||
for _, x := range s {
|
||||
if x == v {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// TestAllRuntimes_IncludesGoogleADK is the direct regression for
|
||||
// controlplane#578: a google-adk pin promote/redeploy is accepted CP-side, so
|
||||
// the tenant image-refresh allowlist MUST also accept google-adk or the image
|
||||
// fix never deploys (tenant returned 400 "unknown runtime"). google-adk lives
|
||||
// in the providers SSOT, so the derived AllRuntimes must contain it.
|
||||
func TestAllRuntimes_IncludesGoogleADK(t *testing.T) {
|
||||
if !runtimeListContains(AllRuntimes, "google-adk") {
|
||||
t.Fatalf("AllRuntimes must include google-adk (controlplane#578 drift); got %v", AllRuntimes)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAllRuntimes_MatchesProvidersSSOT is the drift guard. AllRuntimes is
|
||||
// derived from providers.LoadManifest().Runtimes — assert it equals exactly the
|
||||
// runtime keys the providers manifest (mirrored from CP's providers.yaml)
|
||||
// declares. If CP adds/removes a runtime, this test fails RED until the tenant
|
||||
// re-derives, so the tenant image-refresh allowlist can never silently drift
|
||||
// from the CP pin-promote allowlist again.
|
||||
func TestAllRuntimes_MatchesProvidersSSOT(t *testing.T) {
|
||||
m, err := providers.LoadManifest()
|
||||
if err != nil {
|
||||
t.Fatalf("providers.LoadManifest: %v", err)
|
||||
}
|
||||
want := make([]string, 0, len(m.Runtimes))
|
||||
for rt := range m.Runtimes {
|
||||
want = append(want, rt)
|
||||
}
|
||||
sort.Strings(want)
|
||||
|
||||
got := append([]string(nil), AllRuntimes...)
|
||||
sort.Strings(got)
|
||||
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("AllRuntimes drift: got %v, want %v (providers SSOT)", got, want)
|
||||
}
|
||||
for i := range want {
|
||||
if got[i] != want[i] {
|
||||
t.Fatalf("AllRuntimes drift at %d: got %v, want %v (providers SSOT)", i, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestImageRefreshFallbackMatchesSSOT pins the static fallback (used only when
|
||||
// the embedded manifest fails to load) to the providers SSOT. If a runtime is
|
||||
// added to providers.yaml but not to imageRefreshFallbackRuntimes, this fails
|
||||
// RED — so a manifest-load failure can't silently drop a supported runtime.
|
||||
func TestImageRefreshFallbackMatchesSSOT(t *testing.T) {
|
||||
m, err := providers.LoadManifest()
|
||||
if err != nil {
|
||||
t.Fatalf("providers.LoadManifest: %v", err)
|
||||
}
|
||||
want := make([]string, 0, len(m.Runtimes))
|
||||
for rt := range m.Runtimes {
|
||||
want = append(want, rt)
|
||||
}
|
||||
sort.Strings(want)
|
||||
|
||||
got := append([]string(nil), imageRefreshFallbackRuntimes...)
|
||||
sort.Strings(got)
|
||||
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("fallback drift: got %v, want %v (providers SSOT)", got, want)
|
||||
}
|
||||
for i := range want {
|
||||
if got[i] != want[i] {
|
||||
t.Fatalf("fallback drift at %d: got %v, want %v (providers SSOT)", i, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestRefresh_RejectsUnknownRuntime asserts a genuinely unknown runtime still
|
||||
// 400s (the guard isn't removed) AND that the 400 body lists google-adk in
|
||||
// known_runtimes (proving the allowlist now advertises it). This exercises the
|
||||
// gin handler's reject branch, which runs entirely before any Docker call.
|
||||
func TestRefresh_RejectsUnknownRuntime(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
|
||||
// nil docker client is safe: the unknown-runtime branch returns 400
|
||||
// before svc.Refresh (which is the only path that touches Docker).
|
||||
h := &AdminWorkspaceImagesHandler{svc: &WorkspaceImageService{}}
|
||||
|
||||
r := gin.New()
|
||||
r.POST("/admin/workspace-images/refresh", h.Refresh)
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/admin/workspace-images/refresh?runtime=not-a-real-runtime", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
r.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("unknown runtime: got status %d, want 400; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
var body struct {
|
||||
Error string `json:"error"`
|
||||
KnownRuntimes []string `json:"known_runtimes"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("decode 400 body: %v (raw=%s)", err, rec.Body.String())
|
||||
}
|
||||
if !runtimeListContains(body.KnownRuntimes, "google-adk") {
|
||||
t.Errorf("400 known_runtimes must advertise google-adk (controlplane#578); got %v", body.KnownRuntimes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGHCRAuthHeader_TrimsWhitespace(t *testing.T) {
|
||||
t.Setenv("MOLECULE_IMAGE_REGISTRY", "")
|
||||
// .env lines often have trailing newlines or accidental spaces. Without
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
package handlers
|
||||
|
||||
// approval_gate.go — server-side gate for destructive org operations.
|
||||
// (RFC docs/design/rfc-platform-agent.md — Phase 4)
|
||||
//
|
||||
// requireApproval is the choke point a destructive handler calls before
|
||||
// executing. It is the trust boundary: the platform-management MCP is a CLIENT
|
||||
// of these handlers, so enforcing here (not in the MCP) means anything holding
|
||||
// an org-admin token still goes through the gate. The flow:
|
||||
//
|
||||
// - if a matching APPROVED + unconsumed approval exists, consume it (single-
|
||||
// use) and let the operation proceed;
|
||||
// - otherwise create (or reuse) a PENDING approval, broadcast it to the canvas
|
||||
// (and escalate to the parent if any), and the handler returns HTTP 202 so a
|
||||
// human can decide. The agent retries after approval and the gate passes.
|
||||
//
|
||||
// Matching is by (workspace_id, action, request_hash) where request_hash is a
|
||||
// stable digest of the operation + its context, so a retried op reuses its own
|
||||
// request instead of flooding the table, and an approval for "delete ws A"
|
||||
// cannot be replayed to "delete ws B".
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"database/sql"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"net/http"
|
||||
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/approvals"
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db"
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/events"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// approvalRequestHash is a stable digest of the gated operation. Go's
|
||||
// json.Marshal sorts map keys, so the same context always hashes the same.
|
||||
func approvalRequestHash(workspaceID, action string, contextMap map[string]interface{}) string {
|
||||
cj, err := json.Marshal(contextMap)
|
||||
if err != nil || cj == nil {
|
||||
cj = []byte("{}")
|
||||
}
|
||||
sum := sha256.Sum256([]byte(workspaceID + "\x00" + action + "\x00" + string(cj)))
|
||||
return hex.EncodeToString(sum[:])
|
||||
}
|
||||
|
||||
// requireApproval returns (approved=true, consumedID) when a matching approval
|
||||
// exists and was just consumed; otherwise it creates/reuses a pending approval
|
||||
// and returns (false, pendingID). A non-nil error is a server error.
|
||||
func requireApproval(ctx context.Context, b *events.Broadcaster, workspaceID string, action approvals.Action, reason string, contextMap map[string]interface{}) (bool, string, error) {
|
||||
hash := approvalRequestHash(workspaceID, string(action), contextMap)
|
||||
|
||||
// 1. Atomically consume an approved + unconsumed request, if one exists.
|
||||
// The conditional UPDATE ... RETURNING makes consumption race-safe: two
|
||||
// concurrent destructive calls cannot both consume the same approval.
|
||||
var consumedID string
|
||||
err := db.DB.QueryRowContext(ctx, `
|
||||
UPDATE approval_requests SET consumed_at = now()
|
||||
WHERE id = (
|
||||
SELECT id FROM approval_requests
|
||||
WHERE workspace_id = $1 AND action = $2 AND request_hash = $3
|
||||
AND status = 'approved' AND consumed_at IS NULL
|
||||
ORDER BY decided_at DESC NULLS LAST
|
||||
LIMIT 1
|
||||
FOR UPDATE SKIP LOCKED
|
||||
)
|
||||
RETURNING id
|
||||
`, workspaceID, string(action), hash).Scan(&consumedID)
|
||||
if err == nil {
|
||||
return true, consumedID, nil
|
||||
}
|
||||
if !errors.Is(err, sql.ErrNoRows) {
|
||||
return false, "", fmt.Errorf("consume approval: %w", err)
|
||||
}
|
||||
|
||||
// 2. No usable approval — create a pending one, or reuse an existing pending
|
||||
// request for the same operation so retries don't flood the table.
|
||||
cj, mErr := json.Marshal(contextMap)
|
||||
if mErr != nil || cj == nil {
|
||||
cj = []byte("{}")
|
||||
}
|
||||
var approvalID string
|
||||
err = db.DB.QueryRowContext(ctx, `
|
||||
WITH existing AS (
|
||||
SELECT id FROM approval_requests
|
||||
WHERE workspace_id = $1 AND action = $2 AND request_hash = $3 AND status = 'pending'
|
||||
LIMIT 1
|
||||
), ins AS (
|
||||
INSERT INTO approval_requests (workspace_id, action, reason, context, request_hash)
|
||||
SELECT $1, $2, $4, $5::jsonb, $3
|
||||
WHERE NOT EXISTS (SELECT 1 FROM existing)
|
||||
RETURNING id
|
||||
)
|
||||
SELECT id FROM ins UNION ALL SELECT id FROM existing LIMIT 1
|
||||
`, workspaceID, string(action), hash, reason, string(cj)).Scan(&approvalID)
|
||||
if err != nil {
|
||||
return false, "", fmt.Errorf("create approval: %w", err)
|
||||
}
|
||||
|
||||
// Broadcast to the canvas (the user-facing signal). For a platform agent the
|
||||
// parent_id is NULL, so the requested-event on its own workspace IS the user
|
||||
// prompt; ordinary workspaces also escalate to their parent.
|
||||
if bErr := b.RecordAndBroadcast(ctx, string(events.EventApprovalRequested), workspaceID, map[string]interface{}{
|
||||
"approval_id": approvalID,
|
||||
"action": string(action),
|
||||
"reason": reason,
|
||||
}); bErr != nil {
|
||||
log.Printf("approval_gate: broadcast requested failed (ws=%s): %v", workspaceID, bErr)
|
||||
}
|
||||
var parentID *string
|
||||
if pErr := db.DB.QueryRowContext(ctx, `SELECT parent_id FROM workspaces WHERE id = $1`, workspaceID).Scan(&parentID); pErr != nil {
|
||||
log.Printf("approval_gate: parent lookup failed (ws=%s): %v", workspaceID, pErr)
|
||||
}
|
||||
if parentID != nil {
|
||||
if bErr := b.RecordAndBroadcast(ctx, string(events.EventApprovalEscalated), *parentID, map[string]interface{}{
|
||||
"approval_id": approvalID,
|
||||
"from_workspace_id": workspaceID,
|
||||
"action": string(action),
|
||||
"reason": reason,
|
||||
}); bErr != nil {
|
||||
log.Printf("approval_gate: broadcast escalated failed (ws=%s): %v", workspaceID, bErr)
|
||||
}
|
||||
}
|
||||
return false, approvalID, nil
|
||||
}
|
||||
|
||||
// gateDestructive runs requireApproval for a gated action and, when approval is
|
||||
// still pending, writes the 202 response and returns false (caller must stop).
|
||||
// Returns true when the caller may proceed (action consumed an approval).
|
||||
func gateDestructive(c *gin.Context, b *events.Broadcaster, workspaceID string, action approvals.Action, reason string, contextMap map[string]interface{}) bool {
|
||||
if !approvals.IsGated(action) {
|
||||
return true
|
||||
}
|
||||
approved, approvalID, err := requireApproval(c.Request.Context(), b, workspaceID, action, reason, contextMap)
|
||||
if err != nil {
|
||||
log.Printf("gateDestructive: %v (ws=%s action=%s)", err, workspaceID, action)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "approval gate failed"})
|
||||
return false
|
||||
}
|
||||
if !approved {
|
||||
c.JSON(http.StatusAccepted, gin.H{
|
||||
"status": "pending_approval",
|
||||
"approval_id": approvalID,
|
||||
"action": string(action),
|
||||
"reason": reason,
|
||||
})
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
@@ -0,0 +1,137 @@
|
||||
//go:build integration
|
||||
// +build integration
|
||||
|
||||
// approval_gate_integration_test.go — REAL Postgres gate for requireApproval.
|
||||
//
|
||||
// Run with:
|
||||
//
|
||||
// INTEGRATION_DB_URL="postgres://postgres:test@localhost:55432/molecule?sslmode=disable" \
|
||||
// go test -tags=integration ./internal/handlers/ -run Integration_RequireApproval -v
|
||||
//
|
||||
// Why this is NOT a sqlmock test
|
||||
// ------------------------------
|
||||
// The whole gate is about row state across calls: a pending request is created
|
||||
// once and reused (dedup), an approval is consumed exactly once (single-use via
|
||||
// the conditional UPDATE ... RETURNING), and a different operation context hashes
|
||||
// to a different request. sqlmock returns whatever the stub says; only a real
|
||||
// Postgres proves the consume-once semantics and the partial-index lookup.
|
||||
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"testing"
|
||||
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/approvals"
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db"
|
||||
"github.com/google/uuid"
|
||||
_ "github.com/lib/pq"
|
||||
)
|
||||
|
||||
func TestIntegration_RequireApproval_GateCycle(t *testing.T) {
|
||||
url := requireIntegrationDBURL(t)
|
||||
conn, err := sql.Open("postgres", url)
|
||||
if err != nil {
|
||||
t.Fatalf("open: %v", err)
|
||||
}
|
||||
if err := conn.Ping(); err != nil {
|
||||
t.Fatalf("ping: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { conn.Close() })
|
||||
|
||||
// requireApproval + the broadcaster's structure_events write use the db.DB
|
||||
// global; point it at the integration DB and restore afterwards.
|
||||
prev := db.DB
|
||||
db.DB = conn
|
||||
t.Cleanup(func() { db.DB = prev })
|
||||
setupTestRedis(t) // broadcaster publishes to db.RDB; miniredis backs it
|
||||
|
||||
ctx := context.Background()
|
||||
b := newTestBroadcaster()
|
||||
|
||||
wsID := uuid.New().String()
|
||||
t.Cleanup(func() {
|
||||
_, _ = conn.ExecContext(ctx, `DELETE FROM approval_requests WHERE workspace_id = $1`, wsID)
|
||||
_, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, wsID)
|
||||
})
|
||||
// A root workspace (parent_id NULL) — like the platform agent, it has no
|
||||
// parent, so the gate's escalation target is the user/canvas. (This branch
|
||||
// is off main and has no kind column; the gate is kind-agnostic.)
|
||||
if _, err := conn.ExecContext(ctx, `
|
||||
INSERT INTO workspaces (id, name, tier, status, runtime, parent_id)
|
||||
VALUES ($1, 'Org Concierge', 0, 'online', 'claude-code', NULL)`, wsID); err != nil {
|
||||
t.Fatalf("seed root workspace: %v", err)
|
||||
}
|
||||
|
||||
action := approvals.ActionDeleteWorkspace
|
||||
ctxA := map[string]interface{}{"target": "ws-A"}
|
||||
|
||||
// 1. First call → no approval yet → pending created.
|
||||
ok, id1, err := requireApproval(ctx, b, wsID, action, "delete ws-A", ctxA)
|
||||
if err != nil {
|
||||
t.Fatalf("call 1: %v", err)
|
||||
}
|
||||
if ok {
|
||||
t.Fatal("call 1: approved=true, want false (no approval exists yet)")
|
||||
}
|
||||
|
||||
// 2. Same operation again → must REUSE the same pending row (dedup), not flood.
|
||||
ok, id2, err := requireApproval(ctx, b, wsID, action, "delete ws-A", ctxA)
|
||||
if err != nil {
|
||||
t.Fatalf("call 2: %v", err)
|
||||
}
|
||||
if ok || id2 != id1 {
|
||||
t.Fatalf("call 2: ok=%v id2=%s, want false and id2==id1(%s) (dedup)", ok, id2, id1)
|
||||
}
|
||||
var nPending int
|
||||
if err := conn.QueryRowContext(ctx,
|
||||
`SELECT count(*) FROM approval_requests WHERE workspace_id=$1 AND status='pending'`, wsID).Scan(&nPending); err != nil {
|
||||
t.Fatalf("count pending: %v", err)
|
||||
}
|
||||
if nPending != 1 {
|
||||
t.Fatalf("pending rows = %d, want 1 (dedup must not flood)", nPending)
|
||||
}
|
||||
|
||||
// 3. A human approves it (simulating the Decide handler).
|
||||
if _, err := conn.ExecContext(ctx,
|
||||
`UPDATE approval_requests SET status='approved', decided_by='human', decided_at=now() WHERE id=$1`, id1); err != nil {
|
||||
t.Fatalf("approve: %v", err)
|
||||
}
|
||||
|
||||
// 4. Now the gate consumes the approval and lets the op proceed.
|
||||
ok, consumedID, err := requireApproval(ctx, b, wsID, action, "delete ws-A", ctxA)
|
||||
if err != nil {
|
||||
t.Fatalf("call 4: %v", err)
|
||||
}
|
||||
if !ok || consumedID != id1 {
|
||||
t.Fatalf("call 4: ok=%v consumedID=%s, want true and id1(%s)", ok, consumedID, id1)
|
||||
}
|
||||
|
||||
// 5. Single-use: the SAME approval cannot be replayed — the next call is
|
||||
// pending again (a fresh request), not approved.
|
||||
ok, id5, err := requireApproval(ctx, b, wsID, action, "delete ws-A", ctxA)
|
||||
if err != nil {
|
||||
t.Fatalf("call 5: %v", err)
|
||||
}
|
||||
if ok {
|
||||
t.Fatal("call 5: approved=true — a consumed approval was replayed")
|
||||
}
|
||||
if id5 == id1 {
|
||||
t.Fatal("call 5: reused the consumed request id; want a new pending request")
|
||||
}
|
||||
|
||||
// 6. Context isolation: an approval for ws-A must not authorize ws-B.
|
||||
// Approve the ws-A request, then a ws-B op must still be pending.
|
||||
if _, err := conn.ExecContext(ctx,
|
||||
`UPDATE approval_requests SET status='approved', decided_at=now() WHERE id=$1`, id5); err != nil {
|
||||
t.Fatalf("approve id5: %v", err)
|
||||
}
|
||||
ok, _, err = requireApproval(ctx, b, wsID, action, "delete ws-B", map[string]interface{}{"target": "ws-B"})
|
||||
if err != nil {
|
||||
t.Fatalf("call 6: %v", err)
|
||||
}
|
||||
if ok {
|
||||
t.Fatal("call 6: ws-B proceeded on a ws-A approval — context isolation broken")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,46 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/approvals"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// TestGateDestructive_NonGatedPassesThrough verifies a non-gated action skips
|
||||
// the gate entirely (no DB access, no 202) so handlers whose action isn't in the
|
||||
// policy map behave exactly as before.
|
||||
func TestGateDestructive_NonGatedPassesThrough(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/x", nil)
|
||||
|
||||
proceed := gateDestructive(c, newTestBroadcaster(), "ws-1",
|
||||
approvals.Action("not_a_gated_action"), "noop", nil)
|
||||
|
||||
if !proceed {
|
||||
t.Fatalf("non-gated action must proceed, got proceed=false (status %d)", w.Code)
|
||||
}
|
||||
if w.Code != http.StatusOK { // CreateTestContext default; nothing written
|
||||
t.Errorf("non-gated action wrote a response (status %d), want none", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestApprovalRequestHash_StableAndContextSensitive pins the two properties the
|
||||
// gate relies on: the same operation hashes identically across calls, and a
|
||||
// different context yields a different hash (so an approval can't be replayed
|
||||
// onto a different target).
|
||||
func TestApprovalRequestHash_StableAndContextSensitive(t *testing.T) {
|
||||
a := approvalRequestHash("ws", "delete_workspace", map[string]interface{}{"target": "A", "n": 1})
|
||||
aAgain := approvalRequestHash("ws", "delete_workspace", map[string]interface{}{"n": 1, "target": "A"})
|
||||
b := approvalRequestHash("ws", "delete_workspace", map[string]interface{}{"target": "B", "n": 1})
|
||||
if a != aAgain {
|
||||
t.Errorf("hash not stable across equal contexts: %s vs %s", a, aAgain)
|
||||
}
|
||||
if a == b {
|
||||
t.Errorf("hash not context-sensitive: target A and B collided (%s)", a)
|
||||
}
|
||||
}
|
||||
@@ -73,6 +73,7 @@ func (h *ChannelHandler) List(c *gin.Context) {
|
||||
var config map[string]interface{}
|
||||
if err := json.Unmarshal(configJSON, &config); err != nil {
|
||||
log.Printf("Channels: unmarshal config for channel %s: %v", id, err)
|
||||
config = map[string]interface{}{}
|
||||
}
|
||||
// #319: decrypt sensitive fields first so the mask operates on
|
||||
// plaintext (first-4 / last-4 of the real token, not the ciphertext
|
||||
@@ -94,6 +95,7 @@ func (h *ChannelHandler) List(c *gin.Context) {
|
||||
var allowed []string
|
||||
if err := json.Unmarshal(allowedJSON, &allowed); err != nil {
|
||||
log.Printf("Channels: unmarshal allowed_users for channel %s: %v", id, err)
|
||||
allowed = []string{}
|
||||
}
|
||||
|
||||
entry := map[string]interface{}{
|
||||
@@ -540,9 +542,11 @@ func (h *ChannelHandler) Webhook(c *gin.Context) {
|
||||
}
|
||||
if err := json.Unmarshal(configJSON, &row.Config); err != nil {
|
||||
log.Printf("Channels: unmarshal config for webhook row %s: %v", row.ID, err)
|
||||
row.Config = map[string]interface{}{}
|
||||
}
|
||||
if err := json.Unmarshal(allowedJSON, &row.AllowedUsers); err != nil {
|
||||
log.Printf("Channels: unmarshal allowed_users for webhook row %s: %v", row.ID, err)
|
||||
row.AllowedUsers = []string{}
|
||||
}
|
||||
if err := channels.DecryptSensitiveFields(row.Config); err != nil {
|
||||
log.Printf("Channels: decrypt webhook row %s: %v", row.ID, err)
|
||||
|
||||
@@ -116,6 +116,56 @@ func TestChannelHandler_List(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestChannelHandler_List_InvalidJSON_FallsBack(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewChannelHandler(newTestChannelManager())
|
||||
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "workspace_id", "channel_type", "channel_config", "enabled",
|
||||
"allowed_users", "last_message_at", "message_count", "created_at", "updated_at",
|
||||
}).AddRow(
|
||||
"ch-bad", "ws-1", "telegram",
|
||||
[]byte(`{not valid json`),
|
||||
true, []byte(`[also not json`), nil, 0, nil, nil,
|
||||
)
|
||||
mock.ExpectQuery("SELECT .* FROM workspace_channels WHERE workspace_id").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request, _ = http.NewRequest("GET", "/workspaces/ws-1/channels", nil)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != 200 {
|
||||
t.Errorf("expected 200, got %d", w.Code)
|
||||
}
|
||||
|
||||
var result []map[string]interface{}
|
||||
json.Unmarshal(w.Body.Bytes(), &result)
|
||||
if len(result) != 1 {
|
||||
t.Fatalf("expected 1 channel, got %d", len(result))
|
||||
}
|
||||
|
||||
config, ok := result[0]["config"].(map[string]interface{})
|
||||
if !ok {
|
||||
t.Fatalf("expected config to be a map, got %T", result[0]["config"])
|
||||
}
|
||||
if len(config) != 0 {
|
||||
t.Errorf("expected empty config after unmarshal fallback, got %v", config)
|
||||
}
|
||||
|
||||
allowed, ok := result[0]["allowed_users"].([]interface{})
|
||||
if !ok {
|
||||
t.Fatalf("expected allowed_users to be a slice, got %T", result[0]["allowed_users"])
|
||||
}
|
||||
if len(allowed) != 0 {
|
||||
t.Errorf("expected empty allowed_users after unmarshal fallback, got %v", allowed)
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== Create ====================
|
||||
|
||||
func TestChannelHandler_Create_Success(t *testing.T) {
|
||||
@@ -546,6 +596,41 @@ func TestChannelHandler_Webhook_UnknownType(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestChannelHandler_Webhook_InvalidJSON_FallsBack verifies that when the DB
|
||||
// row contains invalid JSON for channel_config or allowed_users, the webhook
|
||||
// handler logs the error and falls back to an empty map/slice rather than
|
||||
// leaving the fields nil (which would panic on downstream code that expects
|
||||
// concrete values). With empty config there is no chat_id match, so the
|
||||
// handler returns {"status":"no_channel"}.
|
||||
func TestChannelHandler_Webhook_InvalidJSON_FallsBack(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewChannelHandler(newTestChannelManager())
|
||||
|
||||
mock.ExpectQuery(`SELECT id, workspace_id, channel_type, channel_config, enabled, allowed_users FROM workspace_channels WHERE channel_type = .* AND enabled = true`).
|
||||
WithArgs("telegram").
|
||||
WillReturnRows(sqlmock.NewRows([]string{
|
||||
"id", "workspace_id", "channel_type", "channel_config", "enabled", "allowed_users",
|
||||
}).AddRow("ch-bad", "ws-1", "telegram", []byte(`{bad json`), true, []byte(`[bad json`)))
|
||||
|
||||
body := `{"update_id":1,"message":{"message_id":1,"from":{"id":111,"is_bot":false,"first_name":"Test","username":"testuser"},"chat":{"id":-100123,"title":"Test Group","type":"supergroup"},"date":1700000000,"text":"hello"}}`
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/webhooks/telegram", strings.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
c.Params = gin.Params{{Key: "type", Value: "telegram"}}
|
||||
|
||||
handler.Webhook(c)
|
||||
|
||||
if w.Code != 200 {
|
||||
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]interface{}
|
||||
json.Unmarshal(w.Body.Bytes(), &resp)
|
||||
if resp["status"] != "no_channel" {
|
||||
t.Errorf("expected status 'no_channel', got %v", resp["status"])
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== Discover ====================
|
||||
|
||||
func TestChannelHandler_Discover_MissingToken(t *testing.T) {
|
||||
|
||||
@@ -602,6 +602,33 @@ func TestDelegationRecord_RejectsInvalidUUID(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestDelegationRecord_DBInsertFails(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
h := NewDelegationHandler(wh, broadcaster)
|
||||
|
||||
mock.ExpectExec("INSERT INTO activity_logs").
|
||||
WillReturnError(fmt.Errorf("connection refused"))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "550e8400-e29b-41d4-a716-446655440000"}}
|
||||
body := `{"target_id":"550e8400-e29b-41d4-a716-446655440001","task":"hello","delegation_id":"del-xyz"}`
|
||||
c.Request = httptest.NewRequest("POST", "/delegations/record", bytes.NewBufferString(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Record(c)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500 on DB insert failure, got %d", w.Code)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDelegationUpdateStatus_CompletedInsertsResultRow(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
@@ -337,7 +337,7 @@ func TestRegister_ProvisionerURLPreserved(t *testing.T) {
|
||||
WillReturnError(sql.ErrNoRows)
|
||||
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs("ws-prov", "ws-prov", "http://localhost:8000", `{"name":"agent"}`, "push").
|
||||
WithArgs("ws-prov", "ws-prov", "http://localhost:8000", `{"name":"agent"}`, "push", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
// DB returns provisioner URL (127.0.0.1) — should take precedence over agent-reported URL
|
||||
|
||||
@@ -180,7 +180,7 @@ func TestRegisterHandler(t *testing.T) {
|
||||
|
||||
// Expect the upsert INSERT ... ON CONFLICT
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs("ws-123", "ws-123", "http://localhost:8000", `{"name":"test"}`, "push").
|
||||
WithArgs("ws-123", "ws-123", "http://localhost:8000", `{"name":"test"}`, "push", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
// Expect the SELECT url query (for cache URL logic)
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
//go:build integration
|
||||
// +build integration
|
||||
|
||||
// kind_platform_root_integration_test.go — REAL Postgres gate for the
|
||||
// platform-agent participant kind (RFC docs/design/rfc-platform-agent.md).
|
||||
//
|
||||
// Run with:
|
||||
//
|
||||
// INTEGRATION_DB_URL="postgres://postgres:test@localhost:55432/molecule?sslmode=disable" \
|
||||
// go test -tags=integration ./internal/handlers/ -run Integration_PlatformKind -v
|
||||
//
|
||||
// CI: piggybacks on the handlers-postgres-integration workflow (path filter
|
||||
// includes workspace-server/internal/handlers/** and migrations/**).
|
||||
//
|
||||
// Why this is NOT a sqlmock test
|
||||
// ------------------------------
|
||||
// The invariant "a platform agent must be the org root (parent_id IS NULL),
|
||||
// which structurally also means at most one platform agent per org" is enforced
|
||||
// by the workspaces_platform_root_check CHECK constraint in migration
|
||||
// 20260606000000_workspaces_kind. sqlmock cannot execute DDL or evaluate a CHECK
|
||||
// constraint, so only a real Postgres can prove the constraint actually rejects
|
||||
// a non-root platform agent and accepts a root one. The Register handler's
|
||||
// isPlatformRootViolation()/409 path depends on this constraint firing.
|
||||
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
_ "github.com/lib/pq"
|
||||
)
|
||||
|
||||
func integrationDB_PlatformKind(t *testing.T) *sql.DB {
|
||||
t.Helper()
|
||||
url := requireIntegrationDBURL(t)
|
||||
conn, err := sql.Open("postgres", url)
|
||||
if err != nil {
|
||||
t.Fatalf("open: %v", err)
|
||||
}
|
||||
if err := conn.Ping(); err != nil {
|
||||
t.Fatalf("ping: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { conn.Close() })
|
||||
return conn
|
||||
}
|
||||
|
||||
// TestIntegration_PlatformKind_RootAllowed_NonRootRejected proves the three
|
||||
// guarantees of the kind column against a real Postgres:
|
||||
//
|
||||
// 1. a fresh workspace defaults to kind='workspace';
|
||||
// 2. a root row (parent_id IS NULL) may be kind='platform';
|
||||
// 3. a non-root row (parent_id set) may NOT be kind='platform' — the
|
||||
// workspaces_platform_root_check constraint rejects it (23514).
|
||||
func TestIntegration_PlatformKind_RootAllowed_NonRootRejected(t *testing.T) {
|
||||
conn := integrationDB_PlatformKind(t)
|
||||
ctx := context.Background()
|
||||
|
||||
prefix := fmt.Sprintf("itest-kind-%s", uuid.New().String()[:8])
|
||||
cleanup := func() {
|
||||
if _, err := conn.ExecContext(ctx,
|
||||
`DELETE FROM workspaces WHERE name LIKE $1`, prefix+"%"); err != nil {
|
||||
t.Logf("cleanup (non-fatal): %v", err)
|
||||
}
|
||||
}
|
||||
t.Cleanup(cleanup)
|
||||
cleanup() // pre-test hygiene in the shared integration DB
|
||||
|
||||
rootID := uuid.New().String()
|
||||
childID := uuid.New().String()
|
||||
|
||||
// 1. Default kind is 'workspace' when the column is omitted on INSERT.
|
||||
if _, err := conn.ExecContext(ctx, `
|
||||
INSERT INTO workspaces (id, name, tier, runtime, status, parent_id)
|
||||
VALUES ($1, $2, 2, 'claude-code', 'online', NULL)
|
||||
`, rootID, prefix+"-root"); err != nil {
|
||||
t.Fatalf("seed root: %v", err)
|
||||
}
|
||||
var gotKind string
|
||||
if err := conn.QueryRowContext(ctx,
|
||||
`SELECT kind FROM workspaces WHERE id = $1`, rootID).Scan(&gotKind); err != nil {
|
||||
t.Fatalf("read kind: %v", err)
|
||||
}
|
||||
if gotKind != "workspace" {
|
||||
t.Fatalf("default kind = %q, want \"workspace\"", gotKind)
|
||||
}
|
||||
|
||||
// 2. The root row may become a platform agent.
|
||||
if _, err := conn.ExecContext(ctx,
|
||||
`UPDATE workspaces SET kind = 'platform' WHERE id = $1`, rootID); err != nil {
|
||||
t.Fatalf("promote root to platform: unexpected error: %v", err)
|
||||
}
|
||||
|
||||
// A child of the platform root (an ordinary workspace) inserts fine.
|
||||
if _, err := conn.ExecContext(ctx, `
|
||||
INSERT INTO workspaces (id, name, tier, runtime, status, parent_id)
|
||||
VALUES ($1, $2, 2, 'claude-code', 'online', $3)
|
||||
`, childID, prefix+"-child", rootID); err != nil {
|
||||
t.Fatalf("seed child: %v", err)
|
||||
}
|
||||
|
||||
// 3. The non-root child may NOT be a platform agent — the CHECK rejects it.
|
||||
_, err := conn.ExecContext(ctx,
|
||||
`UPDATE workspaces SET kind = 'platform' WHERE id = $1`, childID)
|
||||
if err == nil {
|
||||
t.Fatalf("non-root child accepted kind='platform' — constraint did not fire")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "workspaces_platform_root_check") {
|
||||
t.Fatalf("non-root platform rejection wanted workspaces_platform_root_check, got: %v", err)
|
||||
}
|
||||
|
||||
// And the unknown-kind value is rejected by workspaces_kind_check.
|
||||
_, err = conn.ExecContext(ctx,
|
||||
`UPDATE workspaces SET kind = 'bogus' WHERE id = $1`, rootID)
|
||||
if err == nil || !strings.Contains(err.Error(), "workspaces_kind_check") {
|
||||
t.Fatalf("unknown kind wanted workspaces_kind_check rejection, got: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -164,6 +164,20 @@ func (h *RegistryHandler) resolveDeliveryMode(ctx context.Context, workspaceID,
|
||||
return models.DeliveryModePush, nil
|
||||
}
|
||||
|
||||
// errPlatformNotRoot is the client-facing message when a register call tried to
|
||||
// mark a non-root workspace as a platform agent.
|
||||
const errPlatformNotRoot = "a platform agent must be the org root (parent_id must be null)"
|
||||
|
||||
// isPlatformRootViolation reports whether err is the DB rejecting a register
|
||||
// that tried to mark a non-root workspace as a platform agent (the
|
||||
// workspaces_platform_root_check CHECK constraint). The handler maps it to a
|
||||
// friendly HTTP 409 instead of a raw 500. The invariant — platform == org root,
|
||||
// which structurally also guarantees one platform agent per org — is enforced
|
||||
// race-proof at the DB level; this is just the friendly surface.
|
||||
func isPlatformRootViolation(err error) bool {
|
||||
return err != nil && strings.Contains(err.Error(), "workspaces_platform_root_check")
|
||||
}
|
||||
|
||||
// Returns a non-nil error suitable for including in a 400 Bad Request response.
|
||||
func validateAgentURL(rawURL string) error {
|
||||
if rawURL == "" {
|
||||
@@ -277,6 +291,14 @@ func (h *RegistryHandler) Register(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Validate explicit kind if the agent declared one; empty is allowed and
|
||||
// resolves to the row's existing value (or "workspace" default) in
|
||||
// resolveKind below. Only the platform-agent container declares 'platform'.
|
||||
if payload.Kind != "" && !models.IsValidKind(payload.Kind) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "kind must be 'workspace' or 'platform'"})
|
||||
return
|
||||
}
|
||||
|
||||
ctx := c.Request.Context()
|
||||
|
||||
// C18: prevent workspace URL hijacking on re-registration.
|
||||
@@ -390,9 +412,15 @@ func (h *RegistryHandler) Register(c *gin.Context) {
|
||||
// the row. Without this guard, bulk deletes left tier-3 stragglers because
|
||||
// the last pre-teardown heartbeat flipped status back to 'online' after
|
||||
// Delete's UPDATE.
|
||||
// kind ($6) is the raw payload value (validated above; "" = unspecified).
|
||||
// COALESCE(NULLIF($6,''), …) means: an explicit kind wins; an unspecified
|
||||
// kind defaults to 'workspace' for a NEW row and KEEPS the existing kind on
|
||||
// re-register (so a platform agent re-registering without kind is never
|
||||
// downgraded). A non-root row asking for 'platform' is rejected by the
|
||||
// workspaces_platform_root_check constraint → friendly 409 below.
|
||||
_, err = db.DB.ExecContext(ctx, `
|
||||
INSERT INTO workspaces (id, name, url, agent_card, status, last_heartbeat_at, delivery_mode)
|
||||
VALUES ($1, $2, $3, $4::jsonb, 'online', now(), $5)
|
||||
INSERT INTO workspaces (id, name, url, agent_card, status, last_heartbeat_at, delivery_mode, kind)
|
||||
VALUES ($1, $2, $3, $4::jsonb, 'online', now(), $5, COALESCE(NULLIF($6, ''), 'workspace'))
|
||||
ON CONFLICT (id) DO UPDATE SET
|
||||
url = CASE
|
||||
WHEN workspaces.url LIKE 'http://127.0.0.1%' THEN workspaces.url
|
||||
@@ -402,10 +430,15 @@ func (h *RegistryHandler) Register(c *gin.Context) {
|
||||
status = 'online',
|
||||
last_heartbeat_at = now(),
|
||||
delivery_mode = EXCLUDED.delivery_mode,
|
||||
kind = COALESCE(NULLIF($6, ''), workspaces.kind),
|
||||
updated_at = now()
|
||||
WHERE workspaces.status IS DISTINCT FROM 'removed'
|
||||
`, payload.ID, payload.ID, urlForUpsert, agentCardStr, modeForUpsert)
|
||||
`, payload.ID, payload.ID, urlForUpsert, agentCardStr, modeForUpsert, payload.Kind)
|
||||
if err != nil {
|
||||
if isPlatformRootViolation(err) {
|
||||
c.JSON(http.StatusConflict, gin.H{"error": errPlatformNotRoot})
|
||||
return
|
||||
}
|
||||
log.Printf("Registry register error: %v (id=%s)", err, payload.ID)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "registration failed"})
|
||||
return
|
||||
|
||||
@@ -72,7 +72,7 @@ func TestRegister_DBError(t *testing.T) {
|
||||
|
||||
// DB insert fails
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs("ws-fail", "ws-fail", "http://localhost:8000", `{"name":"test"}`, "push").
|
||||
WithArgs("ws-fail", "ws-fail", "http://localhost:8000", `{"name":"test"}`, "push", "").
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
@@ -647,7 +647,7 @@ func TestRegister_GuardAgainstResurrectingRemovedRow(t *testing.T) {
|
||||
// This regex-ish match requires the guard. If the handler ever drops
|
||||
// the clause the test fails because the emitted SQL won't match.
|
||||
mock.ExpectExec("ON CONFLICT.*WHERE workspaces.status IS DISTINCT FROM 'removed'").
|
||||
WithArgs("ws-resurrect", "ws-resurrect", "http://localhost:8000", `{"name":"x"}`, "push").
|
||||
WithArgs("ws-resurrect", "ws-resurrect", "http://localhost:8000", `{"name":"x"}`, "push", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 0)) // 0 rows affected = correctly guarded
|
||||
mock.ExpectQuery("SELECT url FROM workspaces WHERE id").
|
||||
WithArgs("ws-resurrect").
|
||||
@@ -917,7 +917,7 @@ func TestRegister_C18_BootstrapAllowedNoTokens(t *testing.T) {
|
||||
|
||||
// Workspace upsert proceeds normally.
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs("ws-new", "ws-new", "http://localhost:9100", `{"name":"new-agent"}`, "push").
|
||||
WithArgs("ws-new", "ws-new", "http://localhost:9100", `{"name":"new-agent"}`, "push", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
mock.ExpectQuery("SELECT url FROM workspaces WHERE id").
|
||||
@@ -1228,7 +1228,7 @@ func TestRegister_DBErrorResponseIsOpaque(t *testing.T) {
|
||||
|
||||
// DB upsert fails with a descriptive internal error.
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs("ws-errtest", "ws-errtest", "http://localhost:9200", `{"name":"err-agent"}`, "push").
|
||||
WithArgs("ws-errtest", "ws-errtest", "http://localhost:9200", `{"name":"err-agent"}`, "push", "").
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
@@ -1476,7 +1476,7 @@ func TestRegister_PollMode_AcceptsEmptyURL(t *testing.T) {
|
||||
|
||||
// Upsert MUST run with empty URL (sql.NullString) and delivery_mode=poll.
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"poll-agent"}`, "poll").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"poll-agent"}`, "poll", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
// SELECT url for cache: returns NULL/empty for poll-mode rows. The
|
||||
@@ -1591,6 +1591,89 @@ func TestRegister_InvalidDeliveryMode(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRegister_InvalidKind rejects payloads that declare an unrecognised kind —
|
||||
// only 'workspace' and 'platform' are valid. Mirrors the delivery_mode guard;
|
||||
// the rejection happens before any DB access.
|
||||
func TestRegister_InvalidKind(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewRegistryHandler(broadcaster)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/registry/register",
|
||||
bytes.NewBufferString(`{"id":"ws-x","url":"http://localhost:8000","agent_card":{"name":"a"},"kind":"bogus"}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Register(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("invalid kind: expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if !strings.Contains(w.Body.String(), "kind") {
|
||||
t.Errorf("expected error body to mention kind, got: %s", w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestRegister_PlatformKind_PersistsKind verifies that a workspace registering
|
||||
// with kind="platform" has that value written through the upsert (the platform
|
||||
// agent self-registers as the org root). The platform==root invariant itself is
|
||||
// enforced by the workspaces_platform_root_check DB constraint and exercised by
|
||||
// the integration test, which sqlmock cannot enforce.
|
||||
func TestRegister_PlatformKind_PersistsKind(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewRegistryHandler(broadcaster)
|
||||
|
||||
const wsID = "ws-platform-agent"
|
||||
|
||||
// Bootstrap path — no live tokens.
|
||||
mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
|
||||
|
||||
// delivery_mode="push" is set explicitly, so resolveDeliveryMode
|
||||
// short-circuits (no SELECT delivery_mode lookup). The upsert MUST carry
|
||||
// kind="platform" as the 6th arg.
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(wsID, wsID, "http://localhost:9100", `{"name":"concierge"}`, "push", "platform").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
mock.ExpectQuery("SELECT url FROM workspaces WHERE id").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("http://localhost:9100"))
|
||||
|
||||
mock.ExpectExec("INSERT INTO structure_events").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
// Token issuance — first-register path.
|
||||
mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
|
||||
mock.ExpectExec("INSERT INTO workspace_auth_tokens").
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
mock.ExpectQuery(`SELECT platform_inbound_secret FROM workspaces WHERE id = \$1`).
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"platform_inbound_secret"}).AddRow(nil))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/registry/register",
|
||||
bytes.NewBufferString(`{"id":"`+wsID+`","url":"http://localhost:9100","delivery_mode":"push","kind":"platform","agent_card":{"name":"concierge"}}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Register(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("platform register: expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRegister_PollMode_PreservesExistingValue: when the row already
|
||||
// has delivery_mode=poll and the payload doesn't set it, the resolved
|
||||
// mode should be poll — i.e. "absent payload mode" must NOT silently
|
||||
@@ -1616,7 +1699,7 @@ func TestRegister_PollMode_PreservesExistingValue(t *testing.T) {
|
||||
// Upsert carries the resolved poll mode forward — even though
|
||||
// payload didn't restate it. URL still empty (poll-mode shape).
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"a"}`, "poll").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"a"}`, "poll", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectQuery("SELECT url FROM workspaces WHERE id").
|
||||
WithArgs(wsID).
|
||||
@@ -1685,7 +1768,7 @@ func TestRegister_ExternalRuntime_DefaultsToPoll(t *testing.T) {
|
||||
AddRow(sql.NullString{}, "external"))
|
||||
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"a"}`, "poll").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"a"}`, "poll", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectQuery("SELECT url FROM workspaces WHERE id").
|
||||
WithArgs(wsID).
|
||||
@@ -1744,7 +1827,7 @@ func TestRegister_KimiRuntime_DefaultsToPoll(t *testing.T) {
|
||||
AddRow(sql.NullString{}, "kimi-cli"))
|
||||
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"a"}`, "poll").
|
||||
WithArgs(wsID, wsID, sql.NullString{}, `{"name":"a"}`, "poll", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectQuery("SELECT url FROM workspaces WHERE id").
|
||||
WithArgs(wsID).
|
||||
@@ -1804,7 +1887,7 @@ func TestRegister_NonExternalRuntime_StillDefaultsToPush(t *testing.T) {
|
||||
AddRow(sql.NullString{}, "claude-code"))
|
||||
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(wsID, wsID, "http://localhost:8000", `{"name":"a"}`, "push").
|
||||
WithArgs(wsID, wsID, "http://localhost:8000", `{"name":"a"}`, "push", "").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectQuery("SELECT url FROM workspaces WHERE id").
|
||||
WithArgs(wsID).
|
||||
|
||||
@@ -332,6 +332,7 @@ func (h *WorkspaceHandler) buildProvisionerConfig(
|
||||
InstanceType: payload.Compute.InstanceType,
|
||||
DiskGB: int32(payload.Compute.Volume.RootGB),
|
||||
DataPersistence: payload.Compute.DataPersistence,
|
||||
Provider: payload.Compute.Provider,
|
||||
Display: provisioner.WorkspaceDisplayConfig{
|
||||
Mode: payload.Compute.Display.Mode,
|
||||
Width: payload.Compute.Display.Width,
|
||||
|
||||
@@ -13,11 +13,16 @@ import (
|
||||
const DefaultMaxConcurrentTasks = 1
|
||||
|
||||
type Workspace struct {
|
||||
ID string `json:"id" db:"id"`
|
||||
Name string `json:"name" db:"name"`
|
||||
Role sql.NullString `json:"role" db:"role"`
|
||||
Tier int `json:"tier" db:"tier"`
|
||||
Status string `json:"status" db:"status"`
|
||||
ID string `json:"id" db:"id"`
|
||||
Name string `json:"name" db:"name"`
|
||||
Role sql.NullString `json:"role" db:"role"`
|
||||
Tier int `json:"tier" db:"tier"`
|
||||
Status string `json:"status" db:"status"`
|
||||
// Kind: "workspace" (default) or "platform". A "platform" workspace is the
|
||||
// org-level concierge (the platform agent) that sits at the org root and is
|
||||
// the user's default A2A target. See migration
|
||||
// 20260606000000_workspaces_kind + RFC docs/design/rfc-platform-agent.md.
|
||||
Kind string `json:"kind" db:"kind"`
|
||||
SourceBundleID sql.NullString `json:"source_bundle_id" db:"source_bundle_id"`
|
||||
AgentCard json.RawMessage `json:"agent_card" db:"agent_card"`
|
||||
URL sql.NullString `json:"url" db:"url"`
|
||||
@@ -63,6 +68,21 @@ func IsValidDeliveryMode(s string) bool {
|
||||
return s == DeliveryModePush || s == DeliveryModePoll
|
||||
}
|
||||
|
||||
// Workspace kind constants. Matches the CHECK constraint in migration
|
||||
// 20260606000000_workspaces_kind. KindPlatform marks the org-level concierge
|
||||
// (the platform agent) which sits at the org root; see
|
||||
// docs/design/rfc-platform-agent.md.
|
||||
const (
|
||||
KindWorkspace = "workspace"
|
||||
KindPlatform = "platform"
|
||||
)
|
||||
|
||||
// IsValidKind reports whether s is a recognised workspace kind. Empty string is
|
||||
// NOT valid here — callers resolve the default (KindWorkspace) before calling.
|
||||
func IsValidKind(s string) bool {
|
||||
return s == KindWorkspace || s == KindPlatform
|
||||
}
|
||||
|
||||
type RegisterPayload struct {
|
||||
ID string `json:"id" binding:"required"`
|
||||
// URL is required for push-mode workspaces; optional / unused for
|
||||
@@ -76,6 +96,12 @@ type RegisterPayload struct {
|
||||
// value on the workspace row, or default to push for new rows".
|
||||
// When set, must be one of DeliveryModePush / DeliveryModePoll.
|
||||
DeliveryMode string `json:"delivery_mode,omitempty"`
|
||||
// Kind is optional. Empty string means "keep the existing value on the
|
||||
// workspace row, or default to KindWorkspace for new rows". When set, must
|
||||
// be one of KindWorkspace / KindPlatform. KindPlatform additionally requires
|
||||
// the row to be its own org root (parent_id IS NULL) and to be the only
|
||||
// platform agent in the org — enforced by the Register handler.
|
||||
Kind string `json:"kind,omitempty"`
|
||||
}
|
||||
|
||||
type HeartbeatPayload struct {
|
||||
@@ -174,6 +200,11 @@ type WorkspaceCompute struct {
|
||||
// disk (wiped each recreate — privacy); "" = auto (desktop-control persists,
|
||||
// others follow the org flag). Forwarded verbatim to CP's data_persistence.
|
||||
DataPersistence string `json:"data_persistence,omitempty"`
|
||||
// Provider is the CLOUD/compute backend for this workspace box (multi-provider
|
||||
// RFC, per-workspace): ""/"aws" = default EC2; "hetzner"/"gcp" route to the
|
||||
// CP WorkspaceProvisioner. Distinct from the LLM/model provider. Forwarded to
|
||||
// CP /cp/workspaces/provision `provider`.
|
||||
Provider string `json:"provider,omitempty"`
|
||||
}
|
||||
|
||||
type CreateWorkspacePayload struct {
|
||||
|
||||
@@ -34,6 +34,35 @@ func TestIsValidDeliveryMode_Invalid(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== IsValidKind ====================
|
||||
|
||||
func TestIsValidKind_Valid(t *testing.T) {
|
||||
for _, k := range []string{KindWorkspace, KindPlatform} {
|
||||
if !IsValidKind(k) {
|
||||
t.Errorf("IsValidKind(%q) = false, want true", k)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsValidKind_Invalid(t *testing.T) {
|
||||
cases := []struct {
|
||||
val string
|
||||
want bool
|
||||
}{
|
||||
{"", false}, // empty is not valid — callers resolve the default
|
||||
{"platforms", false}, // typo
|
||||
{"Platform", false}, // case-sensitive
|
||||
{"platform ", false}, // trailing space
|
||||
{"root", false}, // not a kind
|
||||
{"user", false}, // the user is implicit, not a workspace kind
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := IsValidKind(tc.val); got != tc.want {
|
||||
t.Errorf("IsValidKind(%q) = %v, want %v", tc.val, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== WorkspaceStatus ====================
|
||||
|
||||
func TestWorkspaceStatus_String(t *testing.T) {
|
||||
|
||||
@@ -161,6 +161,9 @@ type cpProvisionRequest struct {
|
||||
Tier int `json:"tier"`
|
||||
InstanceType string `json:"instance_type,omitempty"`
|
||||
DiskGB int32 `json:"disk_gb,omitempty"`
|
||||
// Provider routes the CP to the compute backend for this workspace box
|
||||
// (multi-provider RFC, per-workspace). Distinct from the LLM/model provider.
|
||||
Provider string `json:"provider,omitempty"`
|
||||
// DataPersistence is the per-workspace durable-data choice (internal#734);
|
||||
// CP validates the enum at its provision edge and resolves the data volume
|
||||
// from it. Empty = auto (omitted on the wire).
|
||||
@@ -257,6 +260,7 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string,
|
||||
InstanceType: cfg.InstanceType,
|
||||
DiskGB: cfg.DiskGB,
|
||||
DataPersistence: cfg.DataPersistence,
|
||||
Provider: cfg.Provider,
|
||||
Display: cfg.Display,
|
||||
PlatformURL: cfg.PlatformURL,
|
||||
Env: env,
|
||||
|
||||
@@ -100,6 +100,7 @@ type WorkspaceConfig struct {
|
||||
InstanceType string // Optional CP EC2 instance type override (SaaS only)
|
||||
DiskGB int32 // Optional CP root volume size override in GiB (SaaS only)
|
||||
DataPersistence string // internal#734: "persist"|"ephemeral"|"" — durable-data choice forwarded to CP (SaaS only)
|
||||
Provider string // multi-provider RFC: ""/"aws"|"hetzner"|"gcp" compute backend for the workspace box (per-workspace; distinct from LLM/model provider). Forwarded to CP.
|
||||
Display WorkspaceDisplayConfig
|
||||
EnvVars map[string]string // Additional env vars (API keys, etc.)
|
||||
PlatformURL string
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
-- Reverse the participant-kind discriminator.
|
||||
-- Non-destructive: dropping the column makes every workspace an ordinary
|
||||
-- workspace again (the platform agent loses its marker but its row survives).
|
||||
DROP INDEX IF EXISTS idx_workspaces_kind;
|
||||
ALTER TABLE workspaces DROP CONSTRAINT IF EXISTS workspaces_platform_root_check;
|
||||
ALTER TABLE workspaces DROP CONSTRAINT IF EXISTS workspaces_kind_check;
|
||||
ALTER TABLE workspaces DROP COLUMN IF EXISTS kind;
|
||||
@@ -0,0 +1,45 @@
|
||||
-- Participant-kind discriminator for the org-level platform agent.
|
||||
-- (RFC: docs/design/rfc-platform-agent.md)
|
||||
--
|
||||
-- 'workspace' (default) = an ordinary workspace / agent.
|
||||
-- 'platform' = the org-level concierge (the "platform agent"). It is
|
||||
-- the single org root (parent_id IS NULL) and the user's
|
||||
-- default A2A chat target. Exactly one per org.
|
||||
--
|
||||
-- There is no org_id column — an "org" is the parent_id-chain root resolved by
|
||||
-- org_scope.go (orgRootID/sameOrg). "platform == org root" and "one platform
|
||||
-- agent per org" are therefore enforced in the Register/create handlers, not in
|
||||
-- pure SQL. This column is only the discriminator (default-target / billing
|
||||
-- exclusion / UX), defined once here and mirrored by the Go constants
|
||||
-- models.KindWorkspace / models.KindPlatform.
|
||||
--
|
||||
-- Backward-compatible: every existing row defaults to 'workspace'. The CHECK is
|
||||
-- added NOT VALID then validated so the ALTER can never fail on legacy data.
|
||||
ALTER TABLE workspaces
|
||||
ADD COLUMN IF NOT EXISTS kind TEXT NOT NULL DEFAULT 'workspace';
|
||||
|
||||
DO $$
|
||||
BEGIN
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'workspaces_kind_check') THEN
|
||||
ALTER TABLE workspaces
|
||||
ADD CONSTRAINT workspaces_kind_check CHECK (kind IN ('workspace', 'platform')) NOT VALID;
|
||||
ALTER TABLE workspaces VALIDATE CONSTRAINT workspaces_kind_check;
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
-- platform == org root, enforced at the DB level (race-proof). A platform agent
|
||||
-- MUST have parent_id IS NULL. Because an org is the subtree under a single
|
||||
-- parent_id IS NULL root (org_scope.go) and only a root may be 'platform', this
|
||||
-- also structurally guarantees at most ONE platform agent per org. The handler
|
||||
-- additionally pre-checks this to return a friendly 409 instead of a raw 23514.
|
||||
DO $$
|
||||
BEGIN
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'workspaces_platform_root_check') THEN
|
||||
ALTER TABLE workspaces
|
||||
ADD CONSTRAINT workspaces_platform_root_check
|
||||
CHECK (kind <> 'platform' OR parent_id IS NULL) NOT VALID;
|
||||
ALTER TABLE workspaces VALIDATE CONSTRAINT workspaces_platform_root_check;
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_workspaces_kind ON workspaces(kind);
|
||||
@@ -0,0 +1,5 @@
|
||||
-- Reverse the approval-gate single-use/dedup columns.
|
||||
DROP INDEX IF EXISTS approval_requests_gate_idx;
|
||||
ALTER TABLE approval_requests
|
||||
DROP COLUMN IF EXISTS request_hash,
|
||||
DROP COLUMN IF EXISTS consumed_at;
|
||||
@@ -0,0 +1,18 @@
|
||||
-- Single-use + dedup support for the destructive-op approval gate.
|
||||
-- (RFC docs/design/rfc-platform-agent.md — Phase 4)
|
||||
--
|
||||
-- consumed_at: an approval is single-use. Once a destructive op consumes an
|
||||
-- approved request, consumed_at is stamped so the same approval can't be
|
||||
-- replayed for a second destructive call.
|
||||
-- request_hash: a stable hash of (workspace_id, action, context) so a repeated
|
||||
-- destructive attempt matches its own pending/approved request instead of
|
||||
-- flooding the table with duplicates.
|
||||
ALTER TABLE approval_requests
|
||||
ADD COLUMN IF NOT EXISTS consumed_at TIMESTAMPTZ,
|
||||
ADD COLUMN IF NOT EXISTS request_hash TEXT;
|
||||
|
||||
-- Hot path: the gate looks up an approved + unconsumed row matching
|
||||
-- (workspace_id, action, request_hash). Partial index keeps that O(log live).
|
||||
CREATE INDEX IF NOT EXISTS approval_requests_gate_idx
|
||||
ON approval_requests (workspace_id, action, request_hash)
|
||||
WHERE status = 'approved' AND consumed_at IS NULL;
|
||||
Reference in New Issue
Block a user