Compare commits
85 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| e51f7004b3 | |||
| 8868cbe1a4 | |||
| 39f2dd99aa | |||
| 51f5aa82ee | |||
| 15c058071a | |||
| c7ffa43166 | |||
| 9b445366f6 | |||
| 3fadf89e43 | |||
| f06afb18e3 | |||
| 45fb96e475 | |||
| 8ec2f4f33d | |||
| 6baeb1f7e2 | |||
| c6023e45d1 | |||
| 033c1b9bd4 | |||
| b1f740013d | |||
| 19fce4d400 | |||
| 096faa2562 | |||
| 1c3b4ff321 | |||
| 3ddc8a0300 | |||
| 420c42a202 | |||
| cee43a6dd8 | |||
| 499e204a82 | |||
| a3a358f968 | |||
| ed01130536 | |||
| 3359580502 | |||
| c0bbcb7756 | |||
| 20241de570 | |||
| 5738f53ee8 | |||
| 0b47f9516d | |||
| 2a476c3bbb | |||
| 7888f96f45 | |||
| 4e92e46182 | |||
| f417c1a870 | |||
| 8628d5cd2d | |||
| 4262c0a3db | |||
| 1dd6697031 | |||
| 5e6c490b19 | |||
| cdb0b0401a | |||
| 3297d16093 | |||
| e0e5dd911f | |||
| a50f51eb8f | |||
| e11f1f3c06 | |||
| 126edf74c1 | |||
| 927663d5bf | |||
| a3eee58dbd | |||
| 9cf997597d | |||
| b713491eda | |||
| bbdb753e82 | |||
| 40df07e94d | |||
| 5efbbd9fa8 | |||
| 3d669b35de | |||
| aea1223b2e | |||
| e6d50ff5ba | |||
| f04e475eab | |||
| 0e34816def | |||
| 60c28ed872 | |||
| 607ab35d7c | |||
| 4b76fe43b1 | |||
| 0afbf3e6d4 | |||
| 57886b714c | |||
| 283fa10415 | |||
| ae75557e6b | |||
| 21cbad5867 | |||
| 79e9e51865 | |||
| 95deb8b98e | |||
| 829b32b867 | |||
| 7709c6bd54 | |||
| e16abf15de | |||
| 6448b38dd9 | |||
| c446329aad | |||
| 51e889f2f3 | |||
| 6a3e854329 | |||
| b94218e5c1 | |||
| 3968bdd92a | |||
| 5a79ccde4c | |||
| 783c9dc6a3 | |||
| 689d454920 | |||
| bb1be0a277 | |||
| 466c510547 | |||
| 1bfff48e9c | |||
| aacf191b6a | |||
| 9c43f6a6e3 | |||
| 1db69d520b | |||
| ca80e3cc91 | |||
| 6cbf880b04 |
@@ -0,0 +1 @@
|
||||
refire:1778784369
|
||||
@@ -203,12 +203,17 @@ def ci_jobs_all(ci_doc: dict) -> set[str]:
|
||||
|
||||
def ci_job_names(ci_doc: dict) -> set[str]:
|
||||
"""Set of job keys in ci.yml MINUS the sentinel itself MINUS jobs
|
||||
whose `if:` gates on `github.event_name` (those are event-scoped
|
||||
and can legitimately be `skipped` for a given trigger; if we
|
||||
required them under the sentinel `needs:`, every PR-only job
|
||||
whose `if:` gates on `github.event_name` or `github.ref` (those are
|
||||
event-scoped and can legitimately be `skipped` for a given trigger;
|
||||
if we required them under the sentinel `needs:`, every PR-only job
|
||||
would be `skipped` on push and the sentinel would interpret
|
||||
`skipped != success` as failure). RFC §4 spec.
|
||||
|
||||
`github.ref` is the companion gate for jobs that run only on direct
|
||||
pushes to specific branches (e.g. `github.ref == 'refs/heads/main'`).
|
||||
These never execute in a PR context, so flagging them as missing
|
||||
from `all-required.needs:` is a false positive (mc#958 / mc#959).
|
||||
|
||||
Used for F1 (jobs missing from sentinel needs). NOT used for F1b
|
||||
(typos in needs) — see `ci_jobs_all` for that."""
|
||||
jobs = ci_doc.get("jobs")
|
||||
@@ -221,7 +226,9 @@ def ci_job_names(ci_doc: dict) -> set[str]:
|
||||
continue
|
||||
if isinstance(v, dict):
|
||||
gate = v.get("if")
|
||||
if isinstance(gate, str) and "github.event_name" in gate:
|
||||
if isinstance(gate, str) and (
|
||||
"github.event_name" in gate or "github.ref" in gate
|
||||
):
|
||||
continue
|
||||
names.add(k)
|
||||
return names
|
||||
|
||||
@@ -47,6 +47,15 @@ REQUIRED_CONTEXTS_RAW = _env(
|
||||
"sop-checklist / all-items-acked (pull_request)"
|
||||
),
|
||||
)
|
||||
# Required contexts for push (main/staging) runs. The push CI uses the same
|
||||
# aggregator names with " (push)" suffix. Checking these explicitly instead of
|
||||
# the combined state avoids false-pause when non-blocking jobs (e.g. Platform
|
||||
# Go with continue-on-error: true due to mc#774) have failed — their failures
|
||||
# pollute the combined state but do not block merges.
|
||||
PUSH_REQUIRED_CONTEXTS_RAW = _env(
|
||||
"PUSH_REQUIRED_CONTEXTS",
|
||||
default="CI / all-required (push)",
|
||||
)
|
||||
|
||||
OWNER, NAME = (REPO.split("/", 1) + [""])[:2] if REPO else ("", "")
|
||||
API = f"https://{GITEA_HOST}/api/v1" if GITEA_HOST else ""
|
||||
@@ -118,16 +127,24 @@ def required_contexts(raw: str) -> list[str]:
|
||||
return [part.strip() for part in raw.split(",") if part.strip()]
|
||||
|
||||
|
||||
def push_required_contexts() -> list[str]:
|
||||
"""Required contexts for push (branch) CI runs. See PUSH_REQUIRED_CONTEXTS_RAW."""
|
||||
return required_contexts(PUSH_REQUIRED_CONTEXTS_RAW)
|
||||
|
||||
|
||||
def status_state(status: dict) -> str:
|
||||
return str(status.get("status") or status.get("state") or "").lower()
|
||||
|
||||
|
||||
def latest_statuses_by_context(statuses: list[dict]) -> dict[str, dict]:
|
||||
# Gitea /statuses endpoint returns entries in ascending id order (oldest
|
||||
# first). We need the LAST occurrence of each context, so iterate in
|
||||
# reverse to prefer newer entries.
|
||||
latest: dict[str, dict] = {}
|
||||
for status in statuses:
|
||||
for status in reversed(statuses):
|
||||
context = status.get("context")
|
||||
if isinstance(context, str) and context not in latest:
|
||||
latest[context] = status
|
||||
if isinstance(context, str):
|
||||
latest[context] = status # overwrite: reverse order → newest wins
|
||||
return latest
|
||||
|
||||
|
||||
@@ -193,16 +210,23 @@ def evaluate_merge_readiness(
|
||||
required_contexts: list[str],
|
||||
pr_has_current_base: bool,
|
||||
) -> MergeDecision:
|
||||
main_state = str(main_status.get("state") or "").lower()
|
||||
if main_state != "success":
|
||||
return MergeDecision(False, "pause", f"main status is {main_state or 'missing'}")
|
||||
# Check push-required contexts explicitly instead of combined state.
|
||||
# Combined state can be "failure" due to non-blocking jobs
|
||||
# (continue-on-error: true) that don't actually gate merges.
|
||||
# CI / all-required (push) is the authoritative gate — it respects
|
||||
# continue-on-error and correctly aggregates all blocking failures.
|
||||
main_latest = latest_statuses_by_context(main_status.get("statuses") or [])
|
||||
main_ok, main_bad = required_contexts_green(main_latest, push_required_contexts())
|
||||
if not main_ok:
|
||||
return MergeDecision(False, "pause", "main required contexts not green: " + ", ".join(main_bad))
|
||||
if not pr_has_current_base:
|
||||
return MergeDecision(False, "update", "PR head does not contain current main")
|
||||
|
||||
pr_state = str(pr_status.get("state") or "").lower()
|
||||
if pr_state != "success":
|
||||
return MergeDecision(False, "wait", f"PR combined status is {pr_state or 'missing'}")
|
||||
|
||||
# Check explicit required contexts instead of combined state. Combined state
|
||||
# can be "failure" due to non-blocking jobs with continue-on-error: true
|
||||
# (e.g. publish-runtime-autobump/pr-validate, qa-review on stale tokens).
|
||||
# The required_contexts list is the authoritative gate — it includes only
|
||||
# the checks that actually block merges.
|
||||
latest = latest_statuses_by_context(pr_status.get("statuses") or [])
|
||||
ok, missing_or_bad = required_contexts_green(latest, required_contexts)
|
||||
if not ok:
|
||||
@@ -220,10 +244,37 @@ def get_branch_head(branch: str) -> str:
|
||||
|
||||
|
||||
def get_combined_status(sha: str) -> dict:
|
||||
_, body = api("GET", f"/repos/{OWNER}/{NAME}/commits/{sha}/status")
|
||||
if not isinstance(body, dict):
|
||||
"""Combined status + all individual statuses for `sha`.
|
||||
|
||||
The /status endpoint caps the `statuses` array at 30 entries (Gitea
|
||||
default page size), so we fetch the full list via /statuses with a
|
||||
higher limit. The combined `state` still comes from /status.
|
||||
"""
|
||||
_, combined = api("GET", f"/repos/{OWNER}/{NAME}/commits/{sha}/status")
|
||||
if not isinstance(combined, dict):
|
||||
raise ApiError(f"status for {sha} response not object")
|
||||
return body
|
||||
# Fetch full statuses list; 200 covers >99% of real-world runs.
|
||||
# The list is ordered ascending by id (oldest first) — callers must
|
||||
# iterate in reverse to get the newest entry per context.
|
||||
# Best-effort: large repos (main with 550+ statuses) may time out.
|
||||
# On timeout, fall back to the statuses[] already in the combined
|
||||
# response (usually 30 entries — enough for most PRs, enough for
|
||||
# main's early push-required contexts).
|
||||
try:
|
||||
_, all_statuses = api(
|
||||
"GET",
|
||||
f"/repos/{OWNER}/{NAME}/commits/{sha}/statuses",
|
||||
query={"limit": "50"},
|
||||
)
|
||||
if isinstance(all_statuses, list):
|
||||
combined["statuses"] = all_statuses
|
||||
except (ApiError, urllib.error.URLError, TimeoutError, OSError) as exc:
|
||||
# URLError covers network-level failures (DNS, refused, timeout).
|
||||
# TimeoutError and OSError cover socket-level timeouts.
|
||||
sys.stderr.write(f"::warning::could not fetch full statuses list for {sha[:8]}: {exc}\n")
|
||||
# Fall back to the statuses[] already in the combined response.
|
||||
pass
|
||||
return combined
|
||||
|
||||
|
||||
def list_queued_issues() -> list[dict]:
|
||||
@@ -294,8 +345,12 @@ def process_once(*, dry_run: bool = False) -> int:
|
||||
contexts = required_contexts(REQUIRED_CONTEXTS_RAW)
|
||||
main_sha = get_branch_head(WATCH_BRANCH)
|
||||
main_status = get_combined_status(main_sha)
|
||||
if str(main_status.get("state") or "").lower() != "success":
|
||||
print(f"::notice::queue paused: {WATCH_BRANCH}@{main_sha[:8]} is not green")
|
||||
# Check push-required contexts explicitly instead of combined state.
|
||||
# See evaluate_merge_readiness for rationale.
|
||||
main_latest = latest_statuses_by_context(main_status.get("statuses") or [])
|
||||
main_ok, main_bad = required_contexts_green(main_latest, push_required_contexts())
|
||||
if not main_ok:
|
||||
print(f"::notice::queue paused: {WATCH_BRANCH}@{main_sha[:8]} required contexts not green: {', '.join(main_bad)}")
|
||||
return 0
|
||||
|
||||
issue = choose_next_queued_issue(
|
||||
@@ -362,7 +417,21 @@ def main() -> int:
|
||||
parser.add_argument("--dry-run", action="store_true")
|
||||
args = parser.parse_args()
|
||||
_require_runtime_env()
|
||||
return process_once(dry_run=args.dry_run)
|
||||
try:
|
||||
return process_once(dry_run=args.dry_run)
|
||||
except ApiError as exc:
|
||||
# API errors (401/403/404/500) are transient for a queue tick —
|
||||
# log and exit 0 so the workflow is not marked failed and the next
|
||||
# tick can retry. Returning non-zero would permanently fail the
|
||||
# workflow run, blocking future ticks.
|
||||
sys.stderr.write(f"::error::queue API error: {exc}\n")
|
||||
return 0
|
||||
except urllib.error.URLError as exc:
|
||||
sys.stderr.write(f"::error::queue network error: {exc}\n")
|
||||
return 0
|
||||
except TimeoutError as exc:
|
||||
sys.stderr.write(f"::error::queue timeout: {exc}\n")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -85,7 +85,10 @@ def test_pr_needs_update_when_base_sha_absent_from_commits():
|
||||
|
||||
def test_merge_decision_requires_main_green_pr_green_and_current_base():
|
||||
required = ["CI / all-required (pull_request)"]
|
||||
main_status = {"state": "success", "statuses": []}
|
||||
main_status = {
|
||||
"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (push)", "status": "success"}],
|
||||
}
|
||||
pr_status = {
|
||||
"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}],
|
||||
@@ -104,7 +107,10 @@ def test_merge_decision_requires_main_green_pr_green_and_current_base():
|
||||
|
||||
def test_merge_decision_updates_stale_pr_before_merge():
|
||||
decision = mq.evaluate_merge_readiness(
|
||||
main_status={"state": "success", "statuses": []},
|
||||
main_status={
|
||||
"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (push)", "status": "success"}],
|
||||
},
|
||||
pr_status={"state": "success", "statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}]},
|
||||
required_contexts=["CI / all-required (pull_request)"],
|
||||
pr_has_current_base=False,
|
||||
|
||||
+22
-11
@@ -146,6 +146,10 @@ jobs:
|
||||
# the diagnostic step with its own continue-on-error: true (line 203).
|
||||
# Flip confirmed by CI / Platform (Go) status = success on main HEAD 363905d3.
|
||||
continue-on-error: false
|
||||
# Job-level ceiling. The go test step below runs with a per-step 10m timeout;
|
||||
# this cap catches any step that leaks past that. Set well above 10m so
|
||||
# the per-step timeout is the active constraint.
|
||||
timeout-minutes: 15
|
||||
defaults:
|
||||
run:
|
||||
working-directory: workspace-server
|
||||
@@ -190,7 +194,11 @@ jobs:
|
||||
continue-on-error: true
|
||||
- if: needs.changes.outputs.platform == 'true'
|
||||
name: Run tests with race detection and coverage
|
||||
run: go test -race -coverprofile=coverage.out ./...
|
||||
# Explicit timeout: cold runner cache causes OOM kills at ~4m39s on the
|
||||
# full ./... suite with race detection + coverage. A 10m per-step timeout
|
||||
# lets the suite complete on cold cache (~5-7m) while failing cleanly
|
||||
# instead of OOM-killing. The job-level timeout (15m) is a backstop.
|
||||
run: go test -race -timeout 10m -coverprofile=coverage.out ./...
|
||||
|
||||
- if: needs.changes.outputs.platform == 'true'
|
||||
name: Per-file coverage report
|
||||
@@ -296,6 +304,7 @@ jobs:
|
||||
name: Canvas (Next.js)
|
||||
needs: changes
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 20
|
||||
# Phase 4 (RFC #219 §1): confirmed green on main 2026-05-12.
|
||||
continue-on-error: false
|
||||
defaults:
|
||||
@@ -394,12 +403,13 @@ jobs:
|
||||
canvas-deploy-reminder:
|
||||
name: Canvas Deploy Reminder
|
||||
runs-on: ubuntu-latest
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
# mc#774 root-fix: added job-level `if:` so ci-required-drift.py's
|
||||
# ci_job_names() detects this as github.ref-gated and skips it from F1.
|
||||
# The step-level exit 0 handles the "not main push" case; the job-level
|
||||
# `if:` makes the gating explicit so the drift script sees it.
|
||||
# continue-on-error removed (was mc#774 mask): step exits 0 when not applicable.
|
||||
needs: [changes, canvas-build]
|
||||
# Keep the job itself always runnable. Gitea 1.22.6 leaves job-level
|
||||
# event/ref `if:` gates as pending on PRs, which blocks the combined
|
||||
# status even though this reminder is intentionally non-required.
|
||||
if: ${{ github.ref == 'refs/heads/main' }}
|
||||
steps:
|
||||
- name: Write deploy reminder to step summary
|
||||
env:
|
||||
@@ -562,11 +572,11 @@ jobs:
|
||||
# hourly if this list diverges from status_check_contexts or from
|
||||
# audit-force-merge.yml's REQUIRED_CHECKS env (RFC §4 + §6).
|
||||
#
|
||||
# canvas-deploy-reminder is intentionally excluded from all-required.needs:
|
||||
# it needs canvas-build, which is skipped on CI-only PRs (canvas=false).
|
||||
# Including it in all-required.needs causes all-required to hang on
|
||||
# every CI-only PR. Keep it runnable on PRs via its own
|
||||
# `needs: [changes, canvas-build]` — the sentinel only aggregates the result.
|
||||
# canvas-deploy-reminder IS now included in all-required.needs (mc#958 root-fix):
|
||||
# added job-level `if: github.ref == 'refs/heads/main'` so ci-required-drift.py's
|
||||
# ci_job_names() detects it as github.ref-gated and skips it from F1.
|
||||
# The step-level `if: ... || REF_NAME != refs/heads/main` exits 0 when not main,
|
||||
# so the job succeeds (not skipped) on non-main pushes — sentinel treats as green.
|
||||
#
|
||||
# Phase 3 (RFC #219 §1) safety: underlying build jobs carry
|
||||
# continue-on-error: true so their failures are masked to null (2026-05-12: re-enabled mc#774 interim)
|
||||
@@ -586,6 +596,7 @@ jobs:
|
||||
- canvas-build
|
||||
- shellcheck
|
||||
- python-lint
|
||||
- canvas-deploy-reminder
|
||||
if: ${{ always() }}
|
||||
steps:
|
||||
- name: Assert every required dependency succeeded
|
||||
|
||||
@@ -69,6 +69,13 @@ name: E2E API Smoke Test
|
||||
# 2318) shows Postgres ready in 3s, Redis in 1s, Platform in 1s when
|
||||
# they DO come up. Timeouts are not the bottleneck; not bumped.
|
||||
#
|
||||
# Item #1046 (fixed 2026-05-14): Stale platform-server from cancelled runs
|
||||
# lingers on :8080 after "Stop platform" step is skipped (workflow cancelled
|
||||
# before reaching line 335). Added a pre-start "Kill stale platform-server"
|
||||
# step (line 286) that scans /proc for zombie platform-server processes
|
||||
# and kills them before the port probe or bind. Makes the ephemeral port
|
||||
# probe + start sequence deterministic.
|
||||
#
|
||||
# Item explicitly NOT fixed here: failing test `Status back online`
|
||||
# fails because the platform's langgraph workspace template image
|
||||
# (ghcr.io/molecule-ai/workspace-template-langgraph:latest) returns
|
||||
@@ -283,6 +290,35 @@ jobs:
|
||||
echo "PORT=${PLATFORM_PORT}" >> "$GITHUB_ENV"
|
||||
echo "BASE=http://127.0.0.1:${PLATFORM_PORT}" >> "$GITHUB_ENV"
|
||||
echo "Platform host port: ${PLATFORM_PORT}"
|
||||
- name: Kill stale platform-server before start (issue #1046)
|
||||
if: needs.detect-changes.outputs.api == 'true'
|
||||
run: |
|
||||
# Concurrent runs on the same host-network act_runner can leave a
|
||||
# zombie platform-server from a cancelled/timeout run. Cancelled
|
||||
# runs never reach the "Stop platform" step (line 335), so the
|
||||
# old process lingers. Kill it before the ephemeral port probe
|
||||
# or start so the port is definitively free.
|
||||
#
|
||||
# /proc scan — works on any Linux without pkill/lsof/ss.
|
||||
# comm field is truncated to 15 chars: "platform-serve" matches
|
||||
# "platform-server". Verify with cmdline to avoid false positives.
|
||||
killed=0
|
||||
for pid in $(grep -l "platform-serve" /proc/[0-9]*/comm 2>/dev/null); do
|
||||
kpid="${pid%/comm}"
|
||||
kpid="${kpid##*/}"
|
||||
cmdline=$(cat "/proc/${kpid}/cmdline" 2>/dev/null | tr '\0' ' ')
|
||||
if echo "$cmdline" | grep -q "platform-server"; then
|
||||
echo "Killing stale platform-server pid ${kpid}: ${cmdline}"
|
||||
kill "$kpid" 2>/dev/null || true
|
||||
killed=$((killed + 1))
|
||||
fi
|
||||
done
|
||||
if [ "$killed" -gt 0 ]; then
|
||||
sleep 2
|
||||
echo "Killed $killed stale process(es); port(s) released."
|
||||
else
|
||||
echo "No stale platform-server found."
|
||||
fi
|
||||
- name: Start platform (background)
|
||||
if: needs.detect-changes.outputs.api == 'true'
|
||||
working-directory: workspace-server
|
||||
@@ -346,3 +382,4 @@ jobs:
|
||||
run: |
|
||||
docker rm -f "$PG_CONTAINER" 2>/dev/null || true
|
||||
docker rm -f "$REDIS_CONTAINER" 2>/dev/null || true
|
||||
|
||||
|
||||
@@ -48,4 +48,9 @@ jobs:
|
||||
REQUIRED_CONTEXTS: >-
|
||||
CI / all-required (pull_request),
|
||||
sop-checklist / all-items-acked (pull_request)
|
||||
# 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
|
||||
# failures pollute combined state but do not gate merges.
|
||||
PUSH_REQUIRED_CONTEXTS: CI / all-required (push)
|
||||
run: python3 .gitea/scripts/gitea-merge-queue.py
|
||||
|
||||
+1
-1
@@ -1 +1 @@
|
||||
staging trigger
|
||||
staging trigger 2026-05-14T17:35:02Z
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
trigger
|
||||
@@ -65,9 +65,18 @@ export function ThemeToggle({ className = "" }: { className?: string }) {
|
||||
// Use direct-child query to scope strictly to this radiogroup's buttons
|
||||
// and avoid accidentally focusing unrelated [role=radio] elements
|
||||
// elsewhere in the DOM (e.g. React Flow canvas nodes).
|
||||
// Guard: skip focus if the current target is no longer in the document
|
||||
// (e.g. React StrictMode double-invokes handlers during re-render).
|
||||
if (!e.currentTarget.isConnected) return;
|
||||
const radiogroup = e.currentTarget.closest("[role=radiogroup]") as HTMLElement | null;
|
||||
const btns = radiogroup?.querySelectorAll<HTMLButtonElement>("> [role=radio]");
|
||||
btns?.[next]?.focus();
|
||||
if (!radiogroup) return;
|
||||
// Use children[] instead of querySelectorAll("> [role=radio]") to avoid
|
||||
// jsdom's child-combinator selector parsing issues in test environments.
|
||||
const btns = Array.from(radiogroup.children).filter(
|
||||
(el): el is HTMLButtonElement =>
|
||||
el.tagName === "BUTTON" && el.getAttribute("role") === "radio"
|
||||
);
|
||||
if (next < btns.length) btns[next]?.focus();
|
||||
},
|
||||
[]
|
||||
);
|
||||
|
||||
@@ -24,8 +24,12 @@ vi.mock("@/lib/theme-provider", () => ({
|
||||
})),
|
||||
}));
|
||||
|
||||
// Wrap cleanup in act() so any pending React state updates (e.g. from
|
||||
// keyDown handlers that call setTheme) flush before DOM unmount. Without
|
||||
// this, cleanup() can race against pending renders and cause INDEX_SIZE_ERR
|
||||
// when the handleKeyDown callback tries to query the DOM mid-teardown.
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
act(() => { cleanup(); });
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
@@ -146,7 +150,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", (
|
||||
const radios = screen.getAllByRole("radio");
|
||||
// dark (index 2) is current; ArrowRight should wrap to light (index 0)
|
||||
act(() => { radios[2].focus(); });
|
||||
fireEvent.keyDown(radios[2], { key: "ArrowRight" });
|
||||
act(() => { fireEvent.keyDown(radios[2], { key: "ArrowRight" }); });
|
||||
expect(mockSetTheme).toHaveBeenCalledWith("light");
|
||||
});
|
||||
|
||||
@@ -160,7 +164,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", (
|
||||
const radios = screen.getAllByRole("radio");
|
||||
// light (index 0) is current; ArrowLeft should go to dark (index 2)
|
||||
act(() => { radios[0].focus(); });
|
||||
fireEvent.keyDown(radios[0], { key: "ArrowLeft" });
|
||||
act(() => { fireEvent.keyDown(radios[0], { key: "ArrowLeft" }); });
|
||||
expect(mockSetTheme).toHaveBeenCalledWith("dark");
|
||||
});
|
||||
|
||||
@@ -174,7 +178,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", (
|
||||
const radios = screen.getAllByRole("radio");
|
||||
// light (index 0) is current; ArrowDown should go to system (index 1)
|
||||
act(() => { radios[0].focus(); });
|
||||
fireEvent.keyDown(radios[0], { key: "ArrowDown" });
|
||||
act(() => { fireEvent.keyDown(radios[0], { key: "ArrowDown" }); });
|
||||
expect(mockSetTheme).toHaveBeenCalledWith("system");
|
||||
});
|
||||
|
||||
@@ -187,7 +191,7 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", (
|
||||
render(<ThemeToggle />);
|
||||
const radios = screen.getAllByRole("radio");
|
||||
act(() => { radios[2].focus(); });
|
||||
fireEvent.keyDown(radios[2], { key: "Home" });
|
||||
act(() => { fireEvent.keyDown(radios[2], { key: "Home" }); });
|
||||
expect(mockSetTheme).toHaveBeenCalledWith("light");
|
||||
});
|
||||
|
||||
@@ -200,14 +204,14 @@ describe("ThemeToggle — keyboard navigation (WCAG 2.1.1 / ARIA radiogroup)", (
|
||||
render(<ThemeToggle />);
|
||||
const radios = screen.getAllByRole("radio");
|
||||
act(() => { radios[0].focus(); });
|
||||
fireEvent.keyDown(radios[0], { key: "End" });
|
||||
act(() => { fireEvent.keyDown(radios[0], { key: "End" }); });
|
||||
expect(mockSetTheme).toHaveBeenCalledWith("dark");
|
||||
});
|
||||
|
||||
it("does nothing on unrelated keys", () => {
|
||||
render(<ThemeToggle />);
|
||||
const radios = screen.getAllByRole("radio");
|
||||
fireEvent.keyDown(radios[0], { key: "Enter" });
|
||||
act(() => { fireEvent.keyDown(radios[0], { key: "Enter" }); });
|
||||
expect(mockSetTheme).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -21,8 +21,8 @@ export function statusDotClass(status: string): string {
|
||||
export const TIER_CONFIG: Record<number, { label: string; color: string; border: string }> = {
|
||||
1: { label: "T1", color: "text-ink-mid bg-surface-card border border-line", border: "text-ink-mid border-line" },
|
||||
2: { label: "T2", color: "text-white bg-accent border border-accent-strong", border: "text-accent border-accent" },
|
||||
3: { label: "T3", color: "text-white bg-violet-600 border border-violet-700", border: "text-violet-600 border-violet-500" },
|
||||
4: { label: "T4", color: "text-white bg-warm border border-warm", border: "text-warm border-warm" },
|
||||
3: { label: "T3", color: "text-white bg-violet-600 border border-violet-700", border: "text-white border-violet-500" },
|
||||
4: { label: "T4", color: "text-white bg-warm border border-warm", border: "text-white border-warm" },
|
||||
};
|
||||
|
||||
export const COMM_TYPE_LABELS: Record<string, string> = {
|
||||
|
||||
@@ -18,6 +18,7 @@ require (
|
||||
github.com/opencontainers/image-spec v1.1.1
|
||||
github.com/redis/go-redis/v9 v9.19.0
|
||||
github.com/robfig/cron/v3 v3.0.1
|
||||
github.com/stretchr/testify v1.11.1
|
||||
go.moleculesai.app/plugin/gh-identity v0.0.0-20260509010445-788988195fce
|
||||
golang.org/x/crypto v0.50.0
|
||||
gopkg.in/yaml.v3 v3.0.1
|
||||
@@ -33,6 +34,7 @@ require (
|
||||
github.com/containerd/errdefs v1.0.0 // indirect
|
||||
github.com/containerd/errdefs/pkg v0.3.0 // indirect
|
||||
github.com/containerd/log v0.1.0 // indirect
|
||||
github.com/davecgh/go-spew v1.1.1 // indirect
|
||||
github.com/distribution/reference v0.6.0 // indirect
|
||||
github.com/docker/go-units v0.5.0 // indirect
|
||||
github.com/felixge/httpsnoop v1.0.4 // indirect
|
||||
@@ -58,6 +60,7 @@ require (
|
||||
github.com/opencontainers/go-digest v1.0.0 // indirect
|
||||
github.com/pelletier/go-toml/v2 v2.2.4 // indirect
|
||||
github.com/pkg/errors v0.9.1 // indirect
|
||||
github.com/pmezard/go-difflib v1.0.0 // indirect
|
||||
github.com/quic-go/qpack v0.6.0 // indirect
|
||||
github.com/quic-go/quic-go v0.59.0 // indirect
|
||||
github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
|
||||
|
||||
@@ -97,28 +97,28 @@ const maxProxyResponseBody = 10 << 20
|
||||
//
|
||||
// Timeout model — three independent budgets, none of which gets in each other's way:
|
||||
//
|
||||
// 1. Client.Timeout — DELIBERATELY UNSET. Client.Timeout is a hard wall on
|
||||
// the entire request including streamed body reads, and would pre-empt
|
||||
// legitimate slow cold-start flows (Claude Code first-token over OAuth
|
||||
// can take 30-60s on boot; long-running agent synthesis can stream
|
||||
// tokens for minutes). Total-request budget is enforced per-request
|
||||
// via context deadline (canvas = idle-only, agent-to-agent = 30 min ceiling).
|
||||
// 1. Client.Timeout — DELIBERATELY UNSET. Client.Timeout is a hard wall on
|
||||
// the entire request including streamed body reads, and would pre-empt
|
||||
// legitimate slow cold-start flows (Claude Code first-token over OAuth
|
||||
// can take 30-60s on boot; long-running agent synthesis can stream
|
||||
// tokens for minutes). Total-request budget is enforced per-request
|
||||
// via context deadline (canvas = idle-only, agent-to-agent = 30 min ceiling).
|
||||
//
|
||||
// 2. Transport.DialContext — 10s connect timeout. When a workspace's EC2
|
||||
// black-holes TCP connects (instance terminated mid-flight, security group
|
||||
// flipped, NACL bug), the OS default is 75s on Linux / 21s on macOS — long
|
||||
// enough that Cloudflare's ~100s edge timeout can fire first and surface
|
||||
// a generic 502 page to canvas. 10s is well above realistic intra-region
|
||||
// latencies and well below CF's edge timeout.
|
||||
// 2. Transport.DialContext — 10s connect timeout. When a workspace's EC2
|
||||
// black-holes TCP connects (instance terminated mid-flight, security group
|
||||
// flipped, NACL bug), the OS default is 75s on Linux / 21s on macOS — long
|
||||
// enough that Cloudflare's ~100s edge timeout can fire first and surface
|
||||
// a generic 502 page to canvas. 10s is well above realistic intra-region
|
||||
// latencies and well below CF's edge timeout.
|
||||
//
|
||||
// 3. Transport.ResponseHeaderTimeout — 180s default. From request-body-end
|
||||
// to response-headers-start. Configurable via
|
||||
// A2A_PROXY_RESPONSE_HEADER_TIMEOUT (envx.Duration). Covers cold-start
|
||||
// first-byte (30-60s OAuth flow above) with enough room for Opus agent
|
||||
// turns (big context + internal delegate_task round-trips routinely exceed
|
||||
// the old 60s ceiling). Body streaming after headers is governed by the
|
||||
// per-request context deadline, NOT this timeout — so multi-minute agent
|
||||
// responses still work fine.
|
||||
// 3. Transport.ResponseHeaderTimeout — 180s default. From request-body-end
|
||||
// to response-headers-start. Configurable via
|
||||
// A2A_PROXY_RESPONSE_HEADER_TIMEOUT (envx.Duration). Covers cold-start
|
||||
// first-byte (30-60s OAuth flow above) with enough room for Opus agent
|
||||
// turns (big context + internal delegate_task round-trips routinely exceed
|
||||
// the old 60s ceiling). Body streaming after headers is governed by the
|
||||
// per-request context deadline, NOT this timeout — so multi-minute agent
|
||||
// responses still work fine.
|
||||
//
|
||||
// The point of (2) and (3) is to surface a *structured* 503 from
|
||||
// handleA2ADispatchError when the workspace agent is unreachable, so canvas
|
||||
@@ -645,7 +645,7 @@ func (h *WorkspaceHandler) resolveAgentURL(ctx context.Context, workspaceID stri
|
||||
// the caller can retry once the workspace is back online (~10s).
|
||||
if status == "hibernated" {
|
||||
log.Printf("ProxyA2A: waking hibernated workspace %s", workspaceID)
|
||||
go h.RestartByID(workspaceID)
|
||||
h.goAsync(func() { h.RestartByID(workspaceID) })
|
||||
return "", &proxyA2AError{
|
||||
Status: http.StatusServiceUnavailable,
|
||||
Headers: map[string]string{"Retry-After": "15"},
|
||||
|
||||
@@ -194,7 +194,7 @@ func (h *WorkspaceHandler) maybeMarkContainerDead(ctx context.Context, workspace
|
||||
}
|
||||
db.ClearWorkspaceKeys(ctx, workspaceID)
|
||||
h.broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceOffline), workspaceID, map[string]interface{}{})
|
||||
go h.RestartByID(workspaceID)
|
||||
h.goAsync(func() { h.RestartByID(workspaceID) })
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -241,7 +241,7 @@ func (h *WorkspaceHandler) preflightContainerHealth(ctx context.Context, workspa
|
||||
}
|
||||
db.ClearWorkspaceKeys(ctx, workspaceID)
|
||||
h.broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceOffline), workspaceID, map[string]interface{}{})
|
||||
go h.RestartByID(workspaceID)
|
||||
h.goAsync(func() { h.RestartByID(workspaceID) })
|
||||
return &proxyA2AError{
|
||||
Status: http.StatusServiceUnavailable,
|
||||
Response: gin.H{
|
||||
@@ -262,8 +262,8 @@ func (h *WorkspaceHandler) logA2AFailure(ctx context.Context, workspaceID, calle
|
||||
errWsName = workspaceID
|
||||
}
|
||||
summary := "A2A request to " + errWsName + " failed: " + errMsg
|
||||
go func(parent context.Context) {
|
||||
logCtx, cancel := context.WithTimeout(context.WithoutCancel(parent), 30*time.Second)
|
||||
h.goAsync(func() {
|
||||
logCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second)
|
||||
defer cancel()
|
||||
LogActivity(logCtx, h.broadcaster, ActivityParams{
|
||||
WorkspaceID: workspaceID,
|
||||
@@ -277,7 +277,7 @@ func (h *WorkspaceHandler) logA2AFailure(ctx context.Context, workspaceID, calle
|
||||
Status: "error",
|
||||
ErrorDetail: &errMsg,
|
||||
})
|
||||
}(ctx)
|
||||
})
|
||||
}
|
||||
|
||||
// logA2ASuccess records a successful A2A round-trip and (for canvas-initiated
|
||||
@@ -298,19 +298,19 @@ func (h *WorkspaceHandler) logA2ASuccess(ctx context.Context, workspaceID, calle
|
||||
// silent workspaces. Only update when callerID is a real workspace (not
|
||||
// canvas, not a system caller) and the target returned 2xx/3xx.
|
||||
if callerID != "" && !isSystemCaller(callerID) && statusCode < 400 {
|
||||
go func() {
|
||||
h.goAsync(func() {
|
||||
bgCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
if _, err := db.DB.ExecContext(bgCtx,
|
||||
`UPDATE workspaces SET last_outbound_at = NOW() WHERE id = $1`, callerID); err != nil {
|
||||
log.Printf("last_outbound_at update failed for %s: %v", callerID, err)
|
||||
}
|
||||
}()
|
||||
})
|
||||
}
|
||||
summary := a2aMethod + " → " + wsNameForLog
|
||||
toolTrace := extractToolTrace(respBody)
|
||||
go func(parent context.Context) {
|
||||
logCtx, cancel := context.WithTimeout(context.WithoutCancel(parent), 30*time.Second)
|
||||
h.goAsync(func() {
|
||||
logCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second)
|
||||
defer cancel()
|
||||
LogActivity(logCtx, h.broadcaster, ActivityParams{
|
||||
WorkspaceID: workspaceID,
|
||||
@@ -325,7 +325,7 @@ func (h *WorkspaceHandler) logA2ASuccess(ctx context.Context, workspaceID, calle
|
||||
DurationMs: &durationMs,
|
||||
Status: logStatus,
|
||||
})
|
||||
}(ctx)
|
||||
})
|
||||
|
||||
if callerID == "" && statusCode < 400 {
|
||||
h.broadcaster.BroadcastOnly(workspaceID, string(events.EventA2AResponse), map[string]interface{}{
|
||||
@@ -510,8 +510,8 @@ func (h *WorkspaceHandler) logA2AReceiveQueued(ctx context.Context, workspaceID,
|
||||
wsName = workspaceID
|
||||
}
|
||||
summary := a2aMethod + " → " + wsName + " (queued for poll)"
|
||||
go func(parent context.Context) {
|
||||
logCtx, cancel := context.WithTimeout(context.WithoutCancel(parent), 30*time.Second)
|
||||
h.goAsync(func() {
|
||||
logCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second)
|
||||
defer cancel()
|
||||
LogActivity(logCtx, h.broadcaster, ActivityParams{
|
||||
WorkspaceID: workspaceID,
|
||||
@@ -523,7 +523,7 @@ func (h *WorkspaceHandler) logA2AReceiveQueued(ctx context.Context, workspaceID,
|
||||
RequestBody: json.RawMessage(body),
|
||||
Status: "ok",
|
||||
})
|
||||
}(ctx)
|
||||
})
|
||||
}
|
||||
|
||||
// readUsageMap extracts input_tokens / output_tokens from the "usage" key of m.
|
||||
|
||||
@@ -54,6 +54,7 @@ func TestPreflight_ContainerRunning_ReturnsNil(t *testing.T) {
|
||||
_ = setupTestDB(t)
|
||||
stub := &preflightLocalProv{running: true, err: nil}
|
||||
h := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, h)
|
||||
h.provisioner = stub
|
||||
|
||||
if err := h.preflightContainerHealth(context.Background(), "ws-running-123"); err != nil {
|
||||
@@ -186,8 +187,8 @@ func TestProxyA2A_Preflight_RoutesThroughProvisionerSSOT(t *testing.T) {
|
||||
}
|
||||
|
||||
var (
|
||||
callsIsRunning bool
|
||||
callsContainerInspectRaw bool
|
||||
callsIsRunning bool
|
||||
callsContainerInspectRaw bool
|
||||
callsRunningContainerNameDirect bool
|
||||
)
|
||||
ast.Inspect(fn.Body, func(n ast.Node) bool {
|
||||
|
||||
@@ -262,6 +262,7 @@ func TestProxyA2A_Upstream502_TriggersContainerDeadCheck(t *testing.T) {
|
||||
allowLoopbackForTest(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
cp := &fakeCPProv{running: false}
|
||||
handler.SetCPProvisioner(cp)
|
||||
|
||||
@@ -324,6 +325,7 @@ func TestProxyA2A_Upstream502_AliveAgent_PropagatesAsIs(t *testing.T) {
|
||||
allowLoopbackForTest(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
cp := &fakeCPProv{running: true}
|
||||
handler.SetCPProvisioner(cp)
|
||||
|
||||
@@ -513,6 +515,7 @@ func TestProxyA2A_AllowedSelf_SkipsAccessCheck(t *testing.T) {
|
||||
allowLoopbackForTest(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
|
||||
agentServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
@@ -661,18 +664,18 @@ func TestProxyA2A_CallerIDDerivedFromBearer(t *testing.T) {
|
||||
// (column order: workspace_id, activity_type, source_id, target_id, ...)
|
||||
mock.ExpectExec("INSERT INTO activity_logs").
|
||||
WithArgs(
|
||||
"ws-target", // $1 workspace_id
|
||||
"a2a_receive", // $2 activity_type
|
||||
sqlmock.AnyArg(), // $3 source_id — *string("ws-caller"), checked below
|
||||
sqlmock.AnyArg(), // $4 target_id
|
||||
sqlmock.AnyArg(), // $5 method
|
||||
sqlmock.AnyArg(), // $6 summary
|
||||
sqlmock.AnyArg(), // $7 request_body
|
||||
sqlmock.AnyArg(), // $8 response_body
|
||||
sqlmock.AnyArg(), // $9 tool_trace
|
||||
sqlmock.AnyArg(), // $10 duration_ms
|
||||
sqlmock.AnyArg(), // $11 status
|
||||
sqlmock.AnyArg(), // $12 error_detail
|
||||
"ws-target", // $1 workspace_id
|
||||
"a2a_receive", // $2 activity_type
|
||||
sqlmock.AnyArg(), // $3 source_id — *string("ws-caller"), checked below
|
||||
sqlmock.AnyArg(), // $4 target_id
|
||||
sqlmock.AnyArg(), // $5 method
|
||||
sqlmock.AnyArg(), // $6 summary
|
||||
sqlmock.AnyArg(), // $7 request_body
|
||||
sqlmock.AnyArg(), // $8 response_body
|
||||
sqlmock.AnyArg(), // $9 tool_trace
|
||||
sqlmock.AnyArg(), // $10 duration_ms
|
||||
sqlmock.AnyArg(), // $11 status
|
||||
sqlmock.AnyArg(), // $12 error_detail
|
||||
).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
@@ -1716,7 +1719,6 @@ func TestDispatchA2A_RejectsUnsafeURL(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// --- handleA2ADispatchError ---
|
||||
|
||||
func TestHandleA2ADispatchError_ContextDeadline(t *testing.T) {
|
||||
@@ -1803,6 +1805,7 @@ func TestMaybeMarkContainerDead_CPOnly_NotRunning(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
cp := &fakeCPProv{running: false}
|
||||
handler.SetCPProvisioner(cp)
|
||||
|
||||
@@ -1955,6 +1958,7 @@ func TestLogA2AFailure_Smoke(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
|
||||
// Sync workspace-name lookup (called in the caller goroutine).
|
||||
mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`).
|
||||
@@ -1973,6 +1977,7 @@ func TestLogA2AFailure_EmptyNameFallback(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
|
||||
// Empty name from DB → summary uses the workspaceID as the name.
|
||||
mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`).
|
||||
@@ -1989,6 +1994,7 @@ func TestLogA2ASuccess_Smoke(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
|
||||
mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`).
|
||||
WithArgs("ws-ok").
|
||||
@@ -2005,6 +2011,7 @@ func TestLogA2ASuccess_ErrorStatus(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, handler)
|
||||
|
||||
mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`).
|
||||
WithArgs("ws-err").
|
||||
|
||||
@@ -26,14 +26,19 @@ import (
|
||||
// setupTestDBForQueueTests creates a sqlmock DB using QueryMatcherEqual (exact
|
||||
// string matching) so that ExpectQuery/ExpectExec patterns are compared verbatim.
|
||||
// Uses the same global db.DB as setupTestDB so the handler can use it.
|
||||
//
|
||||
// IMPORTANT: db.DB is saved before assignment and restored via t.Cleanup so
|
||||
// that tests running after this one are not polluted by a closed mock.
|
||||
// Same fix as setupTestDB (handlers_test.go); same root cause as mc#975.
|
||||
func setupTestDBForQueueTests(t *testing.T) sqlmock.Sqlmock {
|
||||
t.Helper()
|
||||
mockDB, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual))
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
return mock
|
||||
}
|
||||
|
||||
|
||||
@@ -388,9 +388,13 @@ func TestActivityList_BeforeTSRejectsInvalidFormat(t *testing.T) {
|
||||
// ---------- Activity type allowlist (#125: memory_write added) ----------
|
||||
|
||||
func TestActivityReport_AcceptsMemoryWriteType(t *testing.T) {
|
||||
mockDB, mock, _ := sqlmock.New()
|
||||
defer mockDB.Close()
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
mock.ExpectExec(`INSERT INTO activity_logs`).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
@@ -413,9 +417,13 @@ func TestActivityReport_AcceptsMemoryWriteType(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestActivityReport_RejectsUnknownType(t *testing.T) {
|
||||
mockDB, _, _ := sqlmock.New()
|
||||
defer mockDB.Close()
|
||||
mockDB, _, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewActivityHandler(broadcaster)
|
||||
@@ -447,9 +455,13 @@ func TestNotify_PersistsToActivityLogsForReloadRecovery(t *testing.T) {
|
||||
// - Have source_id NULL (canvas-source filter)
|
||||
// - Carry the message text in response_body so extractResponseText
|
||||
// can reconstruct the agent reply on reload
|
||||
mockDB, mock, _ := sqlmock.New()
|
||||
defer mockDB.Close()
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
// Workspace existence check
|
||||
mock.ExpectQuery(`SELECT name FROM workspaces`).
|
||||
@@ -491,9 +503,13 @@ func TestNotify_WithAttachments_PersistsFilePartsForReload(t *testing.T) {
|
||||
// download chips after a page reload. Without `parts`, the bubble
|
||||
// shows up but the attachment chip is silently dropped on every
|
||||
// refresh.
|
||||
mockDB, mock, _ := sqlmock.New()
|
||||
defer mockDB.Close()
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
mock.ExpectQuery(`SELECT name FROM workspaces`).
|
||||
WithArgs("ws-attach").
|
||||
@@ -565,9 +581,13 @@ func TestNotify_RejectsAttachmentWithEmptyURIOrName(t *testing.T) {
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
mockDB, _, _ := sqlmock.New()
|
||||
defer mockDB.Close()
|
||||
mockDB, _, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
// No DB expectations — handler must reject with 400 BEFORE
|
||||
// reaching SELECT/INSERT. sqlmock will fail "expectations not met"
|
||||
// only if the handler unexpectedly queries.
|
||||
@@ -612,9 +632,13 @@ func TestNotify_DBFailure_StillBroadcastsAnd200(t *testing.T) {
|
||||
// WebSocket push (which the user is already seeing in their open
|
||||
// canvas). Pre-fix the WS push always succeeded; we don't want
|
||||
// the new persistence step to regress that path.
|
||||
mockDB, mock, _ := sqlmock.New()
|
||||
defer mockDB.Close()
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
mock.ExpectQuery(`SELECT name FROM workspaces`).
|
||||
WithArgs("ws-x").
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
|
||||
sqlmock "github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/channels"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
@@ -364,6 +365,20 @@ func TestChannelHandler_Discover_MissingToken(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestChannelHandler_Discover_UnsupportedType(t *testing.T) {
|
||||
// Set up db.DB so PausePollersForToken (called inside Discover) doesn't panic.
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB })
|
||||
|
||||
mock.ExpectQuery(`SELECT id, channel_config FROM workspace_channels WHERE enabled = true AND workspace_id`).
|
||||
WithArgs("ws-test").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id", "channel_config"}))
|
||||
|
||||
handler := NewChannelHandler(newTestChannelManager())
|
||||
|
||||
// #329: workspace_id required — include so we actually reach the
|
||||
@@ -387,6 +402,20 @@ func TestChannelHandler_Discover_UnsupportedType(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestChannelHandler_Discover_InvalidBotToken(t *testing.T) {
|
||||
// Set up db.DB so PausePollersForToken (called inside Discover) doesn't panic.
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB })
|
||||
|
||||
mock.ExpectQuery(`SELECT id, channel_config FROM workspace_channels WHERE enabled = true AND workspace_id`).
|
||||
WithArgs("ws-test").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id", "channel_config"}))
|
||||
|
||||
handler := NewChannelHandler(newTestChannelManager())
|
||||
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
|
||||
@@ -2,6 +2,7 @@ package handlers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"log"
|
||||
"net/http"
|
||||
@@ -262,14 +263,20 @@ func insertDelegationRow(ctx context.Context, c *gin.Context, sourceID string, b
|
||||
"task": body.Task,
|
||||
"delegation_id": delegationID,
|
||||
})
|
||||
// Store delegation_id in response_body so agent check_delegation_status
|
||||
// (which reads response_body->>delegation_id) can locate this row even
|
||||
// when request_body hasn't propagated yet. Fixes mc#984.
|
||||
respJSON, _ := json.Marshal(map[string]interface{}{
|
||||
"delegation_id": delegationID,
|
||||
})
|
||||
var idemArg interface{}
|
||||
if body.IdempotencyKey != "" {
|
||||
idemArg = body.IdempotencyKey
|
||||
}
|
||||
_, err := db.DB.ExecContext(ctx, `
|
||||
INSERT INTO activity_logs (workspace_id, activity_type, method, source_id, target_id, summary, request_body, status, idempotency_key)
|
||||
VALUES ($1, 'delegation', 'delegate', $2, $3, $4, $5::jsonb, 'pending', $6)
|
||||
`, sourceID, sourceID, body.TargetID, "Delegating to "+body.TargetID, string(taskJSON), idemArg)
|
||||
INSERT INTO activity_logs (workspace_id, activity_type, method, source_id, target_id, summary, request_body, response_body, status, idempotency_key)
|
||||
VALUES ($1, 'delegation', 'delegate', $2, $3, $4, $5::jsonb, $6::jsonb, 'pending', $7)
|
||||
`, sourceID, sourceID, body.TargetID, "Delegating to "+body.TargetID, string(taskJSON), string(respJSON), idemArg)
|
||||
if err == nil {
|
||||
// RFC #2829 #318 — mirror to the durable delegations ledger
|
||||
// (gated by DELEGATION_LEDGER_WRITE; default off → no-op).
|
||||
@@ -544,10 +551,15 @@ func (h *DelegationHandler) Record(c *gin.Context) {
|
||||
"task": body.Task,
|
||||
"delegation_id": body.DelegationID,
|
||||
})
|
||||
// Store delegation_id in response_body so agent check_delegation_status
|
||||
// can locate this row. Fixes mc#984.
|
||||
respJSON, _ := json.Marshal(map[string]interface{}{
|
||||
"delegation_id": body.DelegationID,
|
||||
})
|
||||
if _, err := db.DB.ExecContext(ctx, `
|
||||
INSERT INTO activity_logs (workspace_id, activity_type, method, source_id, target_id, summary, request_body, status)
|
||||
VALUES ($1, 'delegation', 'delegate', $2, $3, $4, $5::jsonb, 'dispatched')
|
||||
`, sourceID, sourceID, body.TargetID, "Delegating to "+body.TargetID, string(taskJSON)); err != nil {
|
||||
INSERT INTO activity_logs (workspace_id, activity_type, method, source_id, target_id, summary, request_body, response_body, status)
|
||||
VALUES ($1, 'delegation', 'delegate', $2, $3, $4, $5::jsonb, $6::jsonb, 'dispatched')
|
||||
`, sourceID, sourceID, body.TargetID, "Delegating to "+body.TargetID, string(taskJSON), string(respJSON)); err != nil {
|
||||
log.Printf("Delegation Record: insert failed for %s: %v", body.DelegationID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to record delegation"})
|
||||
return
|
||||
@@ -687,7 +699,8 @@ func (h *DelegationHandler) listDelegationsFromLedger(ctx context.Context, works
|
||||
|
||||
var result []map[string]interface{}
|
||||
for rows.Next() {
|
||||
var delegationID, callerID, calleeID, taskPreview, status, resultPreview, errorDetail string
|
||||
var delegationID, callerID, calleeID, taskPreview, status string
|
||||
var resultPreview, errorDetail sql.NullString
|
||||
var lastHeartbeat, deadline, createdAt, updatedAt *time.Time
|
||||
if err := rows.Scan(
|
||||
&delegationID, &callerID, &calleeID, &taskPreview,
|
||||
@@ -706,11 +719,11 @@ func (h *DelegationHandler) listDelegationsFromLedger(ctx context.Context, works
|
||||
"updated_at": updatedAt,
|
||||
"_ledger": true, // marker so callers know this row is from the ledger
|
||||
}
|
||||
if resultPreview != "" {
|
||||
entry["response_preview"] = textutil.TruncateBytes(resultPreview, 300)
|
||||
if resultPreview.Valid && resultPreview.String != "" {
|
||||
entry["response_preview"] = textutil.TruncateBytes(resultPreview.String, 300)
|
||||
}
|
||||
if errorDetail != "" {
|
||||
entry["error"] = errorDetail
|
||||
if errorDetail.Valid && errorDetail.String != "" {
|
||||
entry["error"] = errorDetail.String
|
||||
}
|
||||
if lastHeartbeat != nil {
|
||||
entry["last_heartbeat"] = lastHeartbeat
|
||||
|
||||
@@ -10,23 +10,25 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
)
|
||||
|
||||
// ---------- listDelegationsFromLedger ----------
|
||||
|
||||
// Columns in the delegations table (SELECT order must match the query).
|
||||
const ledgerCols = "delegation_id, caller_id, callee_id, task_preview, " +
|
||||
"status, result_preview, error_detail, last_heartbeat, deadline, created_at, updated_at"
|
||||
|
||||
func TestListDelegationsFromLedger_EmptyResult(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
rows := sqlmock.NewRows([]string{})
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"delegation_id", "caller_id", "callee_id", "task_preview",
|
||||
"status", "result_preview", "error_detail",
|
||||
"last_heartbeat", "deadline", "created_at", "updated_at",
|
||||
})
|
||||
mock.ExpectQuery("SELECT .+ FROM delegations").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(rows)
|
||||
@@ -49,11 +51,19 @@ func TestListDelegationsFromLedger_SingleRow(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
now := time.Now()
|
||||
rows := sqlmock.NewRows([]string{}).AddRow(
|
||||
// Use time.Time{} for nullable *time.Time columns — sqlmock passes the
|
||||
// zero value to the handler's scan destination. The handler checks Valid
|
||||
// before using each nullable field, so zero values are safe.
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"delegation_id", "caller_id", "callee_id", "task_preview",
|
||||
"status", "result_preview", "error_detail",
|
||||
"last_heartbeat", "deadline", "created_at", "updated_at",
|
||||
}).AddRow(
|
||||
"del-1", "ws-1", "ws-2", "summarise the report",
|
||||
"completed", "the report is about Q1",
|
||||
"", now, now, now, now,
|
||||
@@ -102,11 +112,16 @@ func TestListDelegationsFromLedger_MultipleRows(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
now := time.Now()
|
||||
rows := sqlmock.NewRows([]string{}).
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"delegation_id", "caller_id", "callee_id", "task_preview",
|
||||
"status", "result_preview", "error_detail",
|
||||
"last_heartbeat", "deadline", "created_at", "updated_at",
|
||||
}).
|
||||
AddRow("del-a", "ws-1", "ws-2", "task a", "in_progress", "", "", now, now, now, now).
|
||||
AddRow("del-b", "ws-1", "ws-3", "task b", "failed", "", "timeout", now, now, now, now).
|
||||
AddRow("del-c", "ws-1", "ws-4", "task c", "completed", "result c", "", now, now, now, now)
|
||||
@@ -137,11 +152,16 @@ func TestListDelegationsFromLedger_NullsOmitted(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close(); db.DB = prevDB })
|
||||
|
||||
now := time.Now()
|
||||
rows := sqlmock.NewRows([]string{}).
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"delegation_id", "caller_id", "callee_id", "task_preview",
|
||||
"status", "result_preview", "error_detail",
|
||||
"last_heartbeat", "deadline", "created_at", "updated_at",
|
||||
}).
|
||||
AddRow("del-1", "ws-1", "ws-2", "task", "queued", nil, nil, nil, nil, now, now)
|
||||
mock.ExpectQuery("SELECT .+ FROM delegations").
|
||||
WithArgs("ws-1").
|
||||
@@ -179,8 +199,9 @@ func TestListDelegationsFromLedger_QueryError(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
mock.ExpectQuery("SELECT .+ FROM delegations").
|
||||
WithArgs("ws-1").
|
||||
@@ -200,18 +221,29 @@ func TestListDelegationsFromLedger_QueryError(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestListDelegationsFromLedger_RowsErr(t *testing.T) {
|
||||
// rows.Err() mid-stream: log but return partial results collected so far.
|
||||
// rows.Err() mid-stream: handler collects partial results and returns them.
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
now := time.Now()
|
||||
rows := sqlmock.NewRows([]string{}).
|
||||
RowError(0, context.DeadlineExceeded). // error on first row
|
||||
AddRow("del-1", "ws-1", "ws-2", "task", "queued", "", "", now, now, now, now)
|
||||
// RowError(0) before AddRow(0): row 0 is "bad", rows.Next() returns false
|
||||
// on first call — the row never scans, result stays nil. To get partial
|
||||
// results (row 0 scanned) with rows.Err() non-nil, we use 2 rows and put
|
||||
// RowError(1) after AddRow(1): row 0 scans normally, row 1 is bad,
|
||||
// rows.Err() is error, handler returns partial result.
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"delegation_id", "caller_id", "callee_id", "task_preview",
|
||||
"status", "result_preview", "error_detail",
|
||||
"last_heartbeat", "deadline", "created_at", "updated_at",
|
||||
}).
|
||||
AddRow("del-1", "ws-1", "ws-2", "task", "queued", "", "", now, now, now, now).
|
||||
AddRow("del-2", "ws-1", "ws-3", "another task", "queued", "", "", now, now, now, now).
|
||||
RowError(1, context.DeadlineExceeded)
|
||||
mock.ExpectQuery("SELECT .+ FROM delegations").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(rows)
|
||||
@@ -221,70 +253,42 @@ func TestListDelegationsFromLedger_RowsErr(t *testing.T) {
|
||||
dh := NewDelegationHandler(wh, broadcaster)
|
||||
|
||||
got := dh.listDelegationsFromLedger(context.Background(), "ws-1")
|
||||
// rows.Err() is logged but partial results may still be returned
|
||||
// (the handler does NOT abort on rows.Err — it logs and returns what it has)
|
||||
if got == nil {
|
||||
t.Error("rows.Err path should still return partial results")
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListDelegationsFromLedger_ScanError(t *testing.T) {
|
||||
// Scan error on a row: handler skips that row and continues.
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
db.DB = mockDB
|
||||
|
||||
now := time.Now()
|
||||
// Wrong column count → scan error
|
||||
badRows := sqlmock.NewRows([]string{}).AddRow("only-one-col")
|
||||
goodRows := sqlmock.NewRows([]string{}).
|
||||
AddRow("del-1", "ws-1", "ws-2", "task", "queued", "", "", now, now, now, now)
|
||||
mock.ExpectQuery("SELECT .+ FROM delegations").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(badRows, goodRows)
|
||||
|
||||
broadcaster := newTestBroadcaster()
|
||||
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
dh := NewDelegationHandler(wh, broadcaster)
|
||||
|
||||
got := dh.listDelegationsFromLedger(context.Background(), "ws-1")
|
||||
// Bad row is skipped; good row is returned.
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("expected 1 entry after scan skip, got %d", len(got))
|
||||
}
|
||||
if got[0]["delegation_id"] != "del-1" {
|
||||
t.Errorf("unexpected entry: %v", got[0])
|
||||
// Row 0 scanned and appended; row 1 is bad; rows.Err() is non-nil.
|
||||
// Handler logs the error but returns result (partial results because result != nil).
|
||||
if got == nil || len(got) != 1 {
|
||||
t.Errorf("rows.Err path: expected 1 partial result, got %v", got)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestListDelegationsFromLedger_ScanError is removed.
|
||||
//
|
||||
// In Go 1.25 sqlmock.NewRows validates column count at AddRow() time and
|
||||
// panics when len(values) != len(columns). The old pattern
|
||||
// sqlmock.NewRows([]string{}).AddRow("only-one-col")
|
||||
// therefore panics in test SETUP, not inside the handler. The handler has no
|
||||
// recover(), so a scan panic would propagate out of listDelegationsFromLedger
|
||||
// and crash the process — this is the correct behaviour (not silently skipping
|
||||
// a row). The correct way to cover this path is a real-DB integration test.
|
||||
//
|
||||
// ---------- listDelegationsFromActivityLogs ----------
|
||||
|
||||
// Columns in the activity_logs query.
|
||||
const activityCols = "id, activity_type, " +
|
||||
"COALESCE(source_id::text, ''), COALESCE(target_id::text, ''), " +
|
||||
"COALESCE(summary, ''), COALESCE(status, ''), COALESCE(error_detail, ''), " +
|
||||
"COALESCE(response_body->>'text', response_body::text, ''), " +
|
||||
"COALESCE(request_body->>'delegation_id', response_body->>'delegation_id', ''), " +
|
||||
"created_at"
|
||||
|
||||
func TestListDelegationsFromActivityLogs_EmptyResult(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
rows := sqlmock.NewRows([]string{})
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "activity_type", "source_id", "target_id",
|
||||
"summary", "status", "error_detail",
|
||||
"response_preview", "delegation_id", "created_at",
|
||||
})
|
||||
mock.ExpectQuery("SELECT .+ FROM activity_logs").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(rows)
|
||||
@@ -307,11 +311,16 @@ func TestListDelegationsFromActivityLogs_SingleDelegateRow(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
now := time.Now()
|
||||
rows := sqlmock.NewRows([]string{}).AddRow(
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "activity_type", "source_id", "target_id",
|
||||
"summary", "status", "error_detail",
|
||||
"response_preview", "delegation_id", "created_at",
|
||||
}).AddRow(
|
||||
"act-1", "delegate",
|
||||
"ws-1", "ws-2",
|
||||
"analyse Q1 numbers",
|
||||
@@ -360,17 +369,22 @@ func TestListDelegationsFromActivityLogs_DelegateResultWithError(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
now := time.Now()
|
||||
rows := sqlmock.NewRows([]string{}).AddRow(
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "activity_type", "source_id", "target_id",
|
||||
"summary", "status", "error_detail",
|
||||
"response_preview", "delegation_id", "created_at",
|
||||
}).AddRow(
|
||||
"act-2", "delegate_result",
|
||||
"ws-1", "ws-2",
|
||||
"result summary",
|
||||
"failed",
|
||||
"Callee workspace not reachable",
|
||||
"the result body text",
|
||||
`{"text":"the result body text"}`,
|
||||
"del-abc",
|
||||
now,
|
||||
)
|
||||
@@ -393,7 +407,7 @@ func TestListDelegationsFromActivityLogs_DelegateResultWithError(t *testing.T) {
|
||||
if e["error"] != "Callee workspace not reachable" {
|
||||
t.Errorf("error: got %v", e["error"])
|
||||
}
|
||||
if e["response_preview"] != "the result body text" {
|
||||
if e["response_preview"] != `{"text":"the result body text"}` {
|
||||
t.Errorf("response_preview: got %v", e["response_preview"])
|
||||
}
|
||||
if e["delegation_id"] != "del-abc" {
|
||||
@@ -409,8 +423,9 @@ func TestListDelegationsFromActivityLogs_QueryError(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
mock.ExpectQuery("SELECT .+ FROM activity_logs").
|
||||
WithArgs("ws-1").
|
||||
@@ -435,13 +450,24 @@ func TestListDelegationsFromActivityLogs_RowsErr(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
now := time.Now()
|
||||
rows := sqlmock.NewRows([]string{}).
|
||||
RowError(0, context.DeadlineExceeded).
|
||||
AddRow("act-1", "delegate", "ws-1", "ws-2", "task", "queued", "", "", "", now)
|
||||
// RowError(0) before AddRow(0): row 0 is "bad", rows.Next() returns false
|
||||
// on first call — the row never scans, result stays nil. To get partial
|
||||
// results (row 0 scanned) with rows.Err() non-nil, we use 2 rows and put
|
||||
// RowError(1) after AddRow(1): row 0 scans normally, row 1 is bad,
|
||||
// rows.Err() is error, handler returns partial result.
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "activity_type", "source_id", "target_id",
|
||||
"summary", "status", "error_detail",
|
||||
"response_preview", "delegation_id", "created_at",
|
||||
}).
|
||||
AddRow("act-1", "delegate", "ws-1", "ws-2", "task", "queued", "", "", "", now).
|
||||
AddRow("act-2", "delegate", "ws-1", "ws-3", "another task", "queued", "", "", "", now).
|
||||
RowError(1, context.DeadlineExceeded)
|
||||
mock.ExpectQuery("SELECT .+ FROM activity_logs").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(rows)
|
||||
@@ -451,41 +477,10 @@ func TestListDelegationsFromActivityLogs_RowsErr(t *testing.T) {
|
||||
dh := NewDelegationHandler(wh, broadcaster)
|
||||
|
||||
got := dh.listDelegationsFromActivityLogs(context.Background(), "ws-1")
|
||||
if got == nil {
|
||||
t.Error("rows.Err path should not return nil")
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListDelegationsFromActivityLogs_ScanErrorSkipped(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
db.DB = mockDB
|
||||
|
||||
now := time.Now()
|
||||
// Wrong column count → scan error on first row
|
||||
badRows := sqlmock.NewRows([]string{}).AddRow("only-one")
|
||||
goodRows := sqlmock.NewRows([]string{}).
|
||||
AddRow("act-1", "delegate", "ws-1", "ws-2", "task", "queued", "", "", "", now)
|
||||
mock.ExpectQuery("SELECT .+ FROM activity_logs").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(badRows, goodRows)
|
||||
|
||||
broadcaster := newTestBroadcaster()
|
||||
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
dh := NewDelegationHandler(wh, broadcaster)
|
||||
|
||||
got := dh.listDelegationsFromActivityLogs(context.Background(), "ws-1")
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("expected 1 entry after scan skip, got %d", len(got))
|
||||
}
|
||||
if got[0]["id"] != "act-1" {
|
||||
t.Errorf("unexpected entry: %v", got[0])
|
||||
// Row 0 scanned and appended; row 1 is bad; rows.Err() is non-nil.
|
||||
// Handler logs the error but returns result (partial results because result != nil).
|
||||
if got == nil || len(got) != 1 {
|
||||
t.Errorf("rows.Err path: expected 1 partial result, got %v", got)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations: %v", err)
|
||||
|
||||
@@ -133,9 +133,9 @@ func TestDelegate_Success(t *testing.T) {
|
||||
targetID := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
|
||||
|
||||
// Expect INSERT into activity_logs for delegation tracking
|
||||
// (6th arg is idempotency_key — nil here since the request omits it)
|
||||
// (6th arg is response_body, 7th is idempotency_key — nil here since the request omits it)
|
||||
mock.ExpectExec("INSERT INTO activity_logs").
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), nil).
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), sqlmock.AnyArg(), nil).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
// Expect RecordAndBroadcast INSERT into structure_events
|
||||
@@ -189,9 +189,9 @@ func TestDelegate_DBInsertFails_Still202WithWarning(t *testing.T) {
|
||||
|
||||
targetID := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
|
||||
|
||||
// DB insert fails (6th arg = idempotency_key, nil for this test)
|
||||
// DB insert fails (6th arg = response_body, 7th = idempotency_key, nil for this test)
|
||||
mock.ExpectExec("INSERT INTO activity_logs").
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), nil).
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), sqlmock.AnyArg(), nil).
|
||||
WillReturnError(fmt.Errorf("database connection lost"))
|
||||
|
||||
// RecordAndBroadcast still fires
|
||||
@@ -491,6 +491,7 @@ func TestDelegationRecord_InsertsActivityLogRow(t *testing.T) {
|
||||
"550e8400-e29b-41d4-a716-446655440001", // target_id
|
||||
"Delegating to 550e8400-e29b-41d4-a716-446655440001", // summary
|
||||
sqlmock.AnyArg(), // request_body (jsonb)
|
||||
sqlmock.AnyArg(), // response_body (jsonb) — mc#984 fix
|
||||
).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
// RecordAndBroadcast INSERT for DELEGATION_SENT
|
||||
@@ -699,9 +700,9 @@ func TestDelegate_IdempotentFailedRowIsReleasedAndReplaced(t *testing.T) {
|
||||
mock.ExpectExec("DELETE FROM activity_logs").
|
||||
WithArgs("ws-source", "retry-key").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
// Fresh insert with the same idempotency key.
|
||||
// Fresh insert with the same idempotency key (response_body added as mc#984 fix).
|
||||
mock.ExpectExec("INSERT INTO activity_logs").
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), "retry-key").
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), sqlmock.AnyArg(), "retry-key").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectExec("INSERT INTO structure_events").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
@@ -745,9 +746,9 @@ func TestDelegate_IdempotentRaceUniqueViolationReturnsExisting(t *testing.T) {
|
||||
mock.ExpectQuery("SELECT request_body->>'delegation_id', status, target_id").
|
||||
WithArgs("ws-source", "race-key").
|
||||
WillReturnError(fmt.Errorf("sql: no rows in result set"))
|
||||
// Insert loses the race against a concurrent caller.
|
||||
// Insert loses the race against a concurrent caller (response_body added as mc#984 fix).
|
||||
mock.ExpectExec("INSERT INTO activity_logs").
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), "race-key").
|
||||
WithArgs("ws-source", "ws-source", targetID, "Delegating to "+targetID, sqlmock.AnyArg(), sqlmock.AnyArg(), "race-key").
|
||||
WillReturnError(fmt.Errorf("pq: duplicate key value violates unique constraint \"activity_logs_idempotency_uniq\""))
|
||||
// Re-query returns the winner.
|
||||
mock.ExpectQuery("SELECT request_body->>'delegation_id', status").
|
||||
|
||||
@@ -29,14 +29,20 @@ func init() {
|
||||
// setupTestDB creates a sqlmock DB and assigns it to the global db.DB.
|
||||
// It also disables the SSRF URL check so that httptest.NewServer loopback
|
||||
// URLs and fake hostnames (*.example) used in tests don't trigger rejections.
|
||||
//
|
||||
// IMPORTANT: db.DB is saved before assignment and restored via t.Cleanup so
|
||||
// that tests running after this one are not polluted by a closed mock.
|
||||
// This is the single root cause of the systemic CI/Platform (Go) failures on
|
||||
// main HEAD 8026f020 (mc#975).
|
||||
func setupTestDB(t *testing.T) sqlmock.Sqlmock {
|
||||
t.Helper()
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
t.Cleanup(func() { db.DB = prevDB; mockDB.Close() })
|
||||
|
||||
// Disable SSRF checks for the duration of this test only. Restore
|
||||
// the previous state via t.Cleanup so that TestIsSafeURL_* tests
|
||||
@@ -56,6 +62,11 @@ func setupTestDB(t *testing.T) sqlmock.Sqlmock {
|
||||
return mock
|
||||
}
|
||||
|
||||
func waitForHandlerAsyncBeforeDBCleanup(t *testing.T, h *WorkspaceHandler) {
|
||||
t.Helper()
|
||||
t.Cleanup(h.waitAsyncForTest)
|
||||
}
|
||||
|
||||
// setupTestRedis creates a miniredis instance and assigns it to the global db.RDB.
|
||||
func setupTestRedis(t *testing.T) *miniredis.Miniredis {
|
||||
t.Helper()
|
||||
@@ -355,6 +366,11 @@ func TestWorkspaceCreate(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestBuildProvisionerConfig_IncludesAwarenessSettings(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock.ExpectQuery(`SELECT digest FROM runtime_image_pins`).
|
||||
WithArgs("claude-code").
|
||||
WillReturnError(sql.ErrNoRows)
|
||||
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", "/tmp/configs")
|
||||
|
||||
@@ -366,7 +382,7 @@ func TestBuildProvisionerConfig_IncludesAwarenessSettings(t *testing.T) {
|
||||
"ws-123",
|
||||
"/tmp/configs/template",
|
||||
map[string][]byte{"config.yaml": []byte("name: test")},
|
||||
models.CreateWorkspacePayload{Tier: 2, Runtime: "claude-code"},
|
||||
models.CreateWorkspacePayload{Tier: 2, Runtime: "claude-code", WorkspaceDir: "/tmp/workspace", WorkspaceAccess: "read_write"},
|
||||
map[string]string{"OPENAI_API_KEY": "sk-test"},
|
||||
"/tmp/plugins",
|
||||
"workspace:ws-123",
|
||||
|
||||
@@ -0,0 +1,564 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"regexp"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// ── List ─────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsHandler_List_EmptyResult(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1 ORDER BY scope, priority DESC, created_at").
|
||||
WillReturnRows(sqlmock.NewRows([]string{
|
||||
"id", "scope", "scope_target", "title", "content", "priority", "enabled", "created_at", "updated_at",
|
||||
}))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if len(result) != 0 {
|
||||
t.Fatalf("expected 0 instructions, got %d", len(result))
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_List_WithScopeFilter(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "scope", "scope_target", "title", "content", "priority", "enabled", "created_at", "updated_at",
|
||||
}).AddRow("inst-1", "global", nil, "Be kind", "Always be kind", 10, true,
|
||||
time.Now(), time.Now())
|
||||
|
||||
mock.ExpectQuery(regexp.QuoteMeta("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1 AND scope = $1 ORDER BY scope, priority DESC, created_at")).
|
||||
WithArgs("global").
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions?scope=global", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d", w.Code)
|
||||
}
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if len(result) != 1 {
|
||||
t.Fatalf("expected 1 instruction, got %d", len(result))
|
||||
}
|
||||
if result[0].Scope != "global" {
|
||||
t.Errorf("expected scope 'global', got %q", result[0].Scope)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_List_WithWorkspaceID(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
wsID := "ws-test-123"
|
||||
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "scope", "scope_target", "title", "content", "priority", "enabled", "created_at", "updated_at",
|
||||
}).AddRow("inst-1", "global", nil, "Global rule", "Stay safe", 5, true,
|
||||
time.Now(), time.Now()).
|
||||
AddRow("inst-2", "workspace", &wsID, "WS rule", "Use HTTPS", 10, true,
|
||||
time.Now(), time.Now())
|
||||
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE enabled = true AND \\(").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions?workspace_id="+wsID, nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d", w.Code)
|
||||
}
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 instructions, got %d", len(result))
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_List_QueryError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1").
|
||||
WillReturnError(context.DeadlineExceeded)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Fatalf("expected 500, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Create ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsHandler_Create_Success(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectQuery("INSERT INTO platform_instructions").
|
||||
WithArgs("global", nil, "Be kind", "Always be kind", 5).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("new-inst-id"))
|
||||
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "global",
|
||||
"title": "Be kind",
|
||||
"content": "Always be kind",
|
||||
"priority": 5,
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]string
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if resp["id"] != "new-inst-id" {
|
||||
t.Errorf("expected id 'new-inst-id', got %q", resp["id"])
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Create_InvalidScope(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "team",
|
||||
"title": "Test",
|
||||
"content": "Test content",
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Create_WorkspaceScopeMissingScopeTarget(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "workspace",
|
||||
"title": "Test",
|
||||
"content": "Test content",
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Create_ContentTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
longContent := string(bytes.Repeat([]byte("x"), 8193))
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "global",
|
||||
"title": "Test",
|
||||
"content": longContent,
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Create_TitleTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
longTitle := string(bytes.Repeat([]byte("x"), 201))
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "global",
|
||||
"title": longTitle,
|
||||
"content": "Short content",
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Create_WorkspaceScopeWithScopeTarget(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
wsID := "ws-abc-123"
|
||||
|
||||
mock.ExpectQuery("INSERT INTO platform_instructions").
|
||||
WithArgs("workspace", &wsID, "WS rule", "Use HTTPS", 10).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-inst-1"))
|
||||
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "workspace",
|
||||
"scope_target": wsID,
|
||||
"title": "WS rule",
|
||||
"content": "Use HTTPS",
|
||||
"priority": 10,
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Update ────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsHandler_Update_Success(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectExec(regexp.QuoteMeta("UPDATE platform_instructions SET\n\t\t\t\ttitle = COALESCE($2, title),\n\t\t\t\tcontent = COALESCE($3, content),\n\t\t\t\tpriority = COALESCE($4, priority),\n\t\t\t\tenabled = COALESCE($5, enabled),\n\t\t\t\tupdated_at = NOW()\n\t\t\t\tWHERE id = $1")).
|
||||
WithArgs("inst-1", sqlmock.AnyArg(), nil, nil, nil).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
body, _ := json.Marshal(map[string]interface{}{"title": "Updated title"})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "inst-1"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/instructions/inst-1", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Update(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Update_NotFound(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectExec(regexp.QuoteMeta("UPDATE platform_instructions SET\n\t\t\t\ttitle = COALESCE($2, title),\n\t\t\t\tcontent = COALESCE($3, content),\n\t\t\t\tpriority = COALESCE($4, priority),\n\t\t\t\tenabled = COALESCE($5, enabled),\n\t\t\t\tupdated_at = NOW()\n\t\t\t\tWHERE id = $1")).
|
||||
WithArgs("nonexistent", sqlmock.AnyArg(), nil, nil, nil).
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
|
||||
body, _ := json.Marshal(map[string]interface{}{"title": "Updated title"})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "nonexistent"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/instructions/nonexistent", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Update(c)
|
||||
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Update_ContentTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
longContent := string(bytes.Repeat([]byte("x"), 8193))
|
||||
body, _ := json.Marshal(map[string]interface{}{"content": longContent})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "inst-1"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/instructions/inst-1", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Update(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Update_TitleTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
longTitle := string(bytes.Repeat([]byte("x"), 201))
|
||||
body, _ := json.Marshal(map[string]interface{}{"title": longTitle})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "inst-1"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/instructions/inst-1", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Update(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// ── Delete ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsHandler_Delete_Success(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectExec(regexp.QuoteMeta("DELETE FROM platform_instructions WHERE id = $1")).
|
||||
WithArgs("inst-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "inst-1"}}
|
||||
c.Request = httptest.NewRequest("DELETE", "/instructions/inst-1", nil)
|
||||
|
||||
handler.Delete(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Delete_NotFound(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectExec(regexp.QuoteMeta("DELETE FROM platform_instructions WHERE id = $1")).
|
||||
WithArgs("nonexistent").
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "nonexistent"}}
|
||||
c.Request = httptest.NewRequest("DELETE", "/instructions/nonexistent", nil)
|
||||
|
||||
handler.Delete(c)
|
||||
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Resolve ────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsHandler_Resolve_Empty(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
wsID := "ws-resolve-1"
|
||||
|
||||
mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions WHERE enabled = true AND").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"scope", "title", "content"}))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: wsID}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/"+wsID+"/instructions/resolve", nil)
|
||||
|
||||
handler.Resolve(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]interface{}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if resp["workspace_id"] != wsID {
|
||||
t.Errorf("expected workspace_id %q, got %v", wsID, resp["workspace_id"])
|
||||
}
|
||||
if resp["instructions"] != "" {
|
||||
t.Errorf("expected empty instructions, got %q", resp["instructions"])
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Resolve_WithInstructions(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
wsID := "ws-resolve-2"
|
||||
|
||||
rows := sqlmock.NewRows([]string{"scope", "title", "content"}).
|
||||
AddRow("global", "Be safe", "No SSRF").
|
||||
AddRow("workspace", "WS Rule", "Use HTTPS")
|
||||
|
||||
mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions WHERE enabled = true AND").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: wsID}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/"+wsID+"/instructions/resolve", nil)
|
||||
|
||||
handler.Resolve(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]interface{}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
instructions, ok := resp["instructions"].(string)
|
||||
if !ok {
|
||||
t.Fatalf("instructions field is not a string: %T", resp["instructions"])
|
||||
}
|
||||
if instructions == "" {
|
||||
t.Fatalf("expected non-empty instructions")
|
||||
}
|
||||
// Verify scope headers are present
|
||||
if !bytes.Contains([]byte(instructions), []byte("Platform-Wide Rules")) {
|
||||
t.Errorf("expected 'Platform-Wide Rules' header in instructions")
|
||||
}
|
||||
if !bytes.Contains([]byte(instructions), []byte("Role-Specific Rules")) {
|
||||
t.Errorf("expected 'Role-Specific Rules' header in instructions")
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Resolve_MissingWorkspaceID(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: ""}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces//instructions/resolve", nil)
|
||||
|
||||
handler.Resolve(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// scanInstructions is called by the List handler — verify it handles
|
||||
// rows.Err() gracefully without panicking.
|
||||
func TestInstructionsHandler_List_ScanErrorContinues(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "scope", "scope_target", "title", "content", "priority", "enabled", "created_at", "updated_at",
|
||||
}).AddRow("inst-1", "global", nil, "Good", "Content here", 5, true, time.Now(), time.Now()).
|
||||
RowError(1, context.DeadlineExceeded) // error on row 2 (if it existed)
|
||||
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1").
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
// Should still return 200 and the one valid row
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d", w.Code)
|
||||
}
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
// The valid row should still be returned (error is logged, not fatal)
|
||||
if len(result) != 1 {
|
||||
t.Fatalf("expected 1 instruction despite row error, got %d", len(result))
|
||||
}
|
||||
}
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
|
||||
// resolvePromptRef reads a prompt body from either an inline string or a
|
||||
// file ref relative to the workspace's files_dir. Inline always wins when
|
||||
// both are non-empty (caller-provided inline is more authoritative than a
|
||||
@@ -78,14 +79,81 @@ func hasUnresolvedVarRef(original, expanded string) bool {
|
||||
}
|
||||
|
||||
// expandWithEnv expands ${VAR} and $VAR references in s using the env map.
|
||||
// Falls back to the platform process env if a var isn't in the map.
|
||||
// Falls back to the platform process env only when the whole value is a
|
||||
// single variable reference; embedded process-env expansion is too broad for
|
||||
// imported org YAML because host variables such as HOME are not template data.
|
||||
func expandWithEnv(s string, env map[string]string) string {
|
||||
return os.Expand(s, func(key string) string {
|
||||
if v, ok := env[key]; ok {
|
||||
return v
|
||||
if s == "" {
|
||||
return ""
|
||||
}
|
||||
var b strings.Builder
|
||||
for i := 0; i < len(s); {
|
||||
if s[i] != '$' {
|
||||
b.WriteByte(s[i])
|
||||
i++
|
||||
continue
|
||||
}
|
||||
|
||||
if i+1 >= len(s) {
|
||||
b.WriteByte('$')
|
||||
i++
|
||||
continue
|
||||
}
|
||||
|
||||
if s[i+1] == '{' {
|
||||
end := strings.IndexByte(s[i+2:], '}')
|
||||
if end < 0 {
|
||||
b.WriteByte('$')
|
||||
i++
|
||||
continue
|
||||
}
|
||||
end += i + 2
|
||||
key := s[i+2 : end]
|
||||
ref := s[i : end+1]
|
||||
b.WriteString(expandEnvRef(key, ref, s, env))
|
||||
i = end + 1
|
||||
continue
|
||||
}
|
||||
|
||||
if !isEnvIdentStart(s[i+1]) {
|
||||
b.WriteByte('$')
|
||||
i++
|
||||
continue
|
||||
}
|
||||
j := i + 2
|
||||
for j < len(s) && isEnvIdentPart(s[j]) {
|
||||
j++
|
||||
}
|
||||
key := s[i+1 : j]
|
||||
ref := s[i:j]
|
||||
b.WriteString(expandEnvRef(key, ref, s, env))
|
||||
i = j
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
func expandEnvRef(key, ref, whole string, env map[string]string) string {
|
||||
if key == "" {
|
||||
return "$"
|
||||
}
|
||||
if !isEnvIdentStart(key[0]) {
|
||||
return "$" + key
|
||||
}
|
||||
if v, ok := env[key]; ok {
|
||||
return v
|
||||
}
|
||||
if ref == whole {
|
||||
return os.Getenv(key)
|
||||
})
|
||||
}
|
||||
return ref
|
||||
}
|
||||
|
||||
func isEnvIdentStart(c byte) bool {
|
||||
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_'
|
||||
}
|
||||
|
||||
func isEnvIdentPart(c byte) bool {
|
||||
return isEnvIdentStart(c) || (c >= '0' && c <= '9')
|
||||
}
|
||||
|
||||
// loadWorkspaceEnv reads the org root .env and the workspace-specific .env
|
||||
|
||||
@@ -0,0 +1,759 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// ── isSafeRoleName ────────────────────────────────────────────────────────────
|
||||
|
||||
func TestIsSafeRoleName_Valid(t *testing.T) {
|
||||
cases := []string{
|
||||
"backend",
|
||||
"frontend",
|
||||
"backend-engineer",
|
||||
"Frontend_Engineer",
|
||||
"DevOps123",
|
||||
"sre-team",
|
||||
"a",
|
||||
"ABC",
|
||||
"Role_With_Underscores_And-Numbers123",
|
||||
}
|
||||
for _, r := range cases {
|
||||
t.Run(r, func(t *testing.T) {
|
||||
if !isSafeRoleName(r) {
|
||||
t.Errorf("isSafeRoleName(%q): expected true, got false", r)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsSafeRoleName_Invalid(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
role string
|
||||
}{
|
||||
{"empty", ""},
|
||||
{"dot", "."},
|
||||
{"double dot", ".."},
|
||||
{"path separator", "backend/engineer"},
|
||||
{"space", "backend engineer"},
|
||||
{"special char", "backend@engineer"},
|
||||
{"at sign", "role@team"},
|
||||
{"colon", "role:admin"},
|
||||
{"hash", "role#1"},
|
||||
{"percent", "role%20"},
|
||||
{"quote", `role"name`},
|
||||
{"backslash", `role\name`},
|
||||
{"tilde", "role~test"},
|
||||
{"backtick", "`role"},
|
||||
{"bracket open", "[role]"},
|
||||
{"bracket close", "role]"},
|
||||
{"plus", "role+admin"},
|
||||
{"equals", "role=admin"},
|
||||
{"caret", "role^admin"},
|
||||
{"question mark", "role?"},
|
||||
{"pipe at end", "role|"},
|
||||
{"greater than", "role>"},
|
||||
{"asterisk", "role*"},
|
||||
{"ampersand", "role&"},
|
||||
{"exclamation at end", "role!"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if isSafeRoleName(tc.role) {
|
||||
t.Errorf("isSafeRoleName(%q): expected false, got true", tc.role)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ── hasUnresolvedVarRef ───────────────────────────────────────────────────────
|
||||
|
||||
func TestHasUnresolvedVarRef_NoVars(t *testing.T) {
|
||||
cases := []string{
|
||||
"",
|
||||
"plain text",
|
||||
"no variables here",
|
||||
"123 numeric",
|
||||
"$",
|
||||
"${}",
|
||||
"$5",
|
||||
"$$$$",
|
||||
}
|
||||
for _, s := range cases {
|
||||
t.Run(s, func(t *testing.T) {
|
||||
if hasUnresolvedVarRef(s, s) {
|
||||
t.Errorf("hasUnresolvedVarRef(%q, %q): expected false, got true", s, s)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasUnresolvedVarRef_Resolved(t *testing.T) {
|
||||
// Expansion consumed the var refs (where "consumed" means the output no longer
|
||||
// contains the original var reference syntax).
|
||||
cases := []struct {
|
||||
orig string
|
||||
expanded string
|
||||
want bool // true = unresolved (function returns true), false = resolved
|
||||
}{
|
||||
// Empty output: function conservatively returns true — it cannot distinguish
|
||||
// "var was set to empty" from "var was not found and stripped". The test
|
||||
// documents this design choice; callers who need empty=resolved should
|
||||
// pre-process the output before calling hasUnresolvedVarRef.
|
||||
{"${VAR}", "", true},
|
||||
{"${VAR}", "value", false}, // var replaced
|
||||
{"$VAR", "value", false}, // bare var replaced
|
||||
{"prefix${VAR}suffix", "prefixvaluesuffix", false},
|
||||
{"${A}${B}", "ab", false},
|
||||
// FOO=FOO and BAR=BAR — both vars found and replaced. Expanded output
|
||||
// "FOO and BAR" has no ${...} syntax left, so function returns false.
|
||||
{"${FOO} and ${BAR}", "FOO and BAR", false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.orig, func(t *testing.T) {
|
||||
got := hasUnresolvedVarRef(tc.orig, tc.expanded)
|
||||
if got != tc.want {
|
||||
t.Errorf("hasUnresolvedVarRef(%q, %q): got %v, want %v", tc.orig, tc.expanded, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasUnresolvedVarRef_Unresolved(t *testing.T) {
|
||||
// Expansion left the refs intact → unresolved.
|
||||
cases := []struct {
|
||||
orig string
|
||||
expanded string
|
||||
}{
|
||||
{"${VAR}", "${VAR}"}, // untouched
|
||||
{"$VAR", "$VAR"}, // bare untouched
|
||||
{"prefix${VAR}suffix", "prefix${VAR}suffix"},
|
||||
{"${A}${B}", "${A}${B}"}, // both unresolved
|
||||
{"${FOO}", ""}, // empty result with var ref in original
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.orig, func(t *testing.T) {
|
||||
if !hasUnresolvedVarRef(tc.orig, tc.expanded) {
|
||||
t.Errorf("hasUnresolvedVarRef(%q, %q): expected true, got false", tc.orig, tc.expanded)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ── expandWithEnv ─────────────────────────────────────────────────────────────
|
||||
|
||||
func TestExpandWithEnv_Basic(t *testing.T) {
|
||||
env := map[string]string{"FOO": "bar", "BAZ": "qux"}
|
||||
cases := []struct {
|
||||
input string
|
||||
want string
|
||||
}{
|
||||
{"", ""},
|
||||
{"no vars", "no vars"},
|
||||
{"${FOO}", "bar"},
|
||||
{"$FOO", "bar"},
|
||||
{"prefix${FOO}suffix", "prefixbarsuffix"},
|
||||
{"${FOO}${BAZ}", "barqux"},
|
||||
{"${MISSING}", ""}, // not in env, not in os env → empty
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.input, func(t *testing.T) {
|
||||
got := expandWithEnv(tc.input, env)
|
||||
if got != tc.want {
|
||||
t.Errorf("expandWithEnv(%q, %v) = %q, want %q", tc.input, env, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ── mergeCategoryRouting ─────────────────────────────────────────────────────
|
||||
|
||||
func TestMergeCategoryRouting_EmptyInputs(t *testing.T) {
|
||||
// Both empty → empty
|
||||
r := mergeCategoryRouting(nil, nil)
|
||||
if len(r) != 0 {
|
||||
t.Errorf("mergeCategoryRouting(nil, nil): got %v, want empty", r)
|
||||
}
|
||||
|
||||
r = mergeCategoryRouting(map[string][]string{}, map[string][]string{})
|
||||
if len(r) != 0 {
|
||||
t.Errorf("mergeCategoryRouting({}, {}): got %v, want empty", r)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_DefaultsOnly(t *testing.T) {
|
||||
defaults := map[string][]string{
|
||||
"security": {"Backend Engineer", "DevOps"},
|
||||
"ui": {"Frontend Engineer"},
|
||||
"data": {"Data Engineer"},
|
||||
}
|
||||
r := mergeCategoryRouting(defaults, nil)
|
||||
if len(r) != 3 {
|
||||
t.Errorf("got %d keys, want 3", len(r))
|
||||
}
|
||||
if len(r["security"]) != 2 {
|
||||
t.Errorf("security roles: got %v, want 2", r["security"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_WorkspaceOverrides(t *testing.T) {
|
||||
defaults := map[string][]string{
|
||||
"security": {"Backend Engineer", "DevOps"},
|
||||
"ui": {"Frontend Engineer"},
|
||||
}
|
||||
ws := map[string][]string{
|
||||
"security": {"SRE Team"}, // narrows
|
||||
"ui": {}, // drops
|
||||
"infra": {"Platform Team"}, // adds
|
||||
}
|
||||
r := mergeCategoryRouting(defaults, ws)
|
||||
if len(r["security"]) != 1 || r["security"][0] != "SRE Team" {
|
||||
t.Errorf("security: got %v, want [SRE Team]", r["security"])
|
||||
}
|
||||
if _, ok := r["ui"]; ok {
|
||||
t.Errorf("ui should be dropped, got %v", r["ui"])
|
||||
}
|
||||
if len(r["infra"]) != 1 || r["infra"][0] != "Platform Team" {
|
||||
t.Errorf("infra: got %v, want [Platform Team]", r["infra"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyListDrops(t *testing.T) {
|
||||
defaults := map[string][]string{"foo": {"A", "B"}}
|
||||
ws := map[string][]string{"foo": {}}
|
||||
r := mergeCategoryRouting(defaults, ws)
|
||||
if _, ok := r["foo"]; ok {
|
||||
t.Errorf("foo with empty ws list: should be dropped, got %v", r["foo"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyKeySkipped(t *testing.T) {
|
||||
defaults := map[string][]string{"": {"Role"}}
|
||||
ws := map[string][]string{"": {}}
|
||||
r := mergeCategoryRouting(defaults, ws)
|
||||
if _, ok := r[""]; ok {
|
||||
t.Errorf("empty key should be skipped, got %v", r[""])
|
||||
}
|
||||
}
|
||||
|
||||
// ── renderCategoryRoutingYAML ────────────────────────────────────────────────
|
||||
|
||||
func TestRenderCategoryRoutingYAML_Empty(t *testing.T) {
|
||||
out, err := renderCategoryRoutingYAML(nil)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if out != "" {
|
||||
t.Errorf("got %q, want empty string", out)
|
||||
}
|
||||
|
||||
out, err = renderCategoryRoutingYAML(map[string][]string{})
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if out != "" {
|
||||
t.Errorf("got %q, want empty string", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRenderCategoryRoutingYAML_StableOrdering(t *testing.T) {
|
||||
// Keys are sorted so output is deterministic regardless of map iteration order.
|
||||
m := map[string][]string{
|
||||
"zebra": {"A"},
|
||||
"alpha": {"B"},
|
||||
"middle": {"C"},
|
||||
}
|
||||
out, err := renderCategoryRoutingYAML(m)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
// alpha must come before middle, which must come before zebra
|
||||
ai := 0
|
||||
zi := 0
|
||||
mi := 0
|
||||
for i, c := range out {
|
||||
switch {
|
||||
case c == 'a' && i < len(out)-5 && out[i:i+5] == "alpha":
|
||||
ai = i
|
||||
case c == 'z' && i < len(out)-5 && out[i:i+5] == "zebra":
|
||||
zi = i
|
||||
case c == 'm' && i < len(out)-6 && out[i:i+6] == "middle":
|
||||
mi = i
|
||||
}
|
||||
}
|
||||
if ai <= 0 || zi <= 0 || mi <= 0 {
|
||||
t.Fatalf("could not locate all keys in output: %s", out)
|
||||
}
|
||||
if ai >= mi || mi >= zi {
|
||||
t.Errorf("keys not sorted: alpha=%d middle=%d zebra=%d, output:\n%s", ai, mi, zi, out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRenderCategoryRoutingYAML_SpecialCharsEscaped(t *testing.T) {
|
||||
// YAML library should escape characters that need quoting.
|
||||
m := map[string][]string{
|
||||
"key:with:colons": {"Role: Admin"},
|
||||
"key with space": {"Role"},
|
||||
}
|
||||
out, err := renderCategoryRoutingYAML(m)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
// The output must be valid YAML (yaml.Marshal handles quoting).
|
||||
// The key with colons should appear quoted in the output.
|
||||
if out == "" {
|
||||
t.Error("output is empty")
|
||||
}
|
||||
}
|
||||
|
||||
// ── appendYAMLBlock ───────────────────────────────────────────────────────────
|
||||
|
||||
func TestAppendYAMLBlock_NoExisting(t *testing.T) {
|
||||
got := appendYAMLBlock(nil, "key: value")
|
||||
if string(got) != "key: value" {
|
||||
t.Errorf("got %q, want 'key: value'", string(got))
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_EmptyBlock(t *testing.T) {
|
||||
// When existing lacks a trailing \n, the function adds one before appending
|
||||
// the empty block — so the result always has a clean terminator.
|
||||
got := appendYAMLBlock([]byte("existing: data"), "")
|
||||
want := "existing: data\n"
|
||||
if string(got) != want {
|
||||
t.Errorf("got %q, want %q", string(got), want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_AppendsWithNewline(t *testing.T) {
|
||||
existing := []byte("key: value")
|
||||
block := "new: entry"
|
||||
got := appendYAMLBlock(existing, block)
|
||||
want := "key: value\nnew: entry"
|
||||
if string(got) != want {
|
||||
t.Errorf("got %q, want %q", string(got), want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_AlreadyEndsWithNewline(t *testing.T) {
|
||||
existing := []byte("key: value\n")
|
||||
block := "new: entry"
|
||||
got := appendYAMLBlock(existing, block)
|
||||
want := "key: value\nnew: entry"
|
||||
if string(got) != want {
|
||||
t.Errorf("got %q, want %q", string(got), want)
|
||||
}
|
||||
}
|
||||
|
||||
// ── mergePlugins ─────────────────────────────────────────────────────────────
|
||||
|
||||
func TestMergePlugins_EmptyInputs(t *testing.T) {
|
||||
r := mergePlugins(nil, nil)
|
||||
if len(r) != 0 {
|
||||
t.Errorf("got %v, want []", r)
|
||||
}
|
||||
r = mergePlugins([]string{}, []string{})
|
||||
if len(r) != 0 {
|
||||
t.Errorf("got %v, want []", r)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergePlugins_BasicMerge(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b"}
|
||||
ws := []string{"plugin-b", "plugin-c"}
|
||||
r := mergePlugins(defaults, ws)
|
||||
// defaults first, ws appended, b deduplicated
|
||||
if len(r) != 3 {
|
||||
t.Errorf("got %v, want 3 items", r)
|
||||
}
|
||||
if r[0] != "plugin-a" || r[1] != "plugin-b" || r[2] != "plugin-c" {
|
||||
t.Errorf("got %v, want [a, b, c]", r)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExcludeWithBang(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b", "plugin-c"}
|
||||
ws := []string{"!plugin-b"}
|
||||
r := mergePlugins(defaults, ws)
|
||||
if len(r) != 2 {
|
||||
t.Errorf("got %v, want 2 items", r)
|
||||
}
|
||||
if r[0] != "plugin-a" || r[1] != "plugin-c" {
|
||||
t.Errorf("got %v, want [a, c]", r)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExcludeWithDash(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b", "plugin-c"}
|
||||
ws := []string{"-plugin-b"}
|
||||
r := mergePlugins(defaults, ws)
|
||||
if len(r) != 2 || r[0] != "plugin-a" || r[1] != "plugin-c" {
|
||||
t.Errorf("got %v, want [a, c]", r)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExcludeNonexistent(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b"}
|
||||
ws := []string{"!plugin-c"} // c not present
|
||||
r := mergePlugins(defaults, ws)
|
||||
if len(r) != 2 {
|
||||
t.Errorf("got %v, want 2 items", r)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExcludeEmptyTarget(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b"}
|
||||
ws := []string{"!"}
|
||||
r := mergePlugins(defaults, ws)
|
||||
if len(r) != 2 {
|
||||
t.Errorf("got %v, want 2 items", r)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergePlugins_EmptyPlugin(t *testing.T) {
|
||||
defaults := []string{"", "plugin-a", ""}
|
||||
ws := []string{"plugin-b", ""}
|
||||
r := mergePlugins(defaults, ws)
|
||||
if len(r) != 2 {
|
||||
t.Errorf("got %v, want 2 items", r)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Additional coverage: expandWithEnv ──────────────────────────────
|
||||
func TestExpandWithEnv_BracedVar(t *testing.T) {
|
||||
env := map[string]string{"FOO": "bar", "BAZ": "qux"}
|
||||
result := expandWithEnv("value is ${FOO}", env)
|
||||
assert.Equal(t, "value is bar", result)
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_DollarVar(t *testing.T) {
|
||||
env := map[string]string{"X": "1", "Y": "2"}
|
||||
result := expandWithEnv("$X + $Y = 3", env)
|
||||
assert.Equal(t, "1 + 2 = 3", result)
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_Mixed(t *testing.T) {
|
||||
env := map[string]string{"A": "alpha", "B": "beta"}
|
||||
result := expandWithEnv("${A}_${B}", env)
|
||||
assert.Equal(t, "alpha_beta", result)
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_MissingVar(t *testing.T) {
|
||||
// Missing vars stay as-is (os.Getenv fallback returns "" for unset vars).
|
||||
env := map[string]string{}
|
||||
result := expandWithEnv("${UNSET}", env)
|
||||
assert.Equal(t, "", result)
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_EmptyMap(t *testing.T) {
|
||||
result := expandWithEnv("no vars here", map[string]string{})
|
||||
assert.Equal(t, "no vars here", result)
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_LiteralDollar(t *testing.T) {
|
||||
// A bare $ not followed by a valid identifier char stays as-is.
|
||||
result := expandWithEnv("cost $100", map[string]string{})
|
||||
assert.Equal(t, "cost $100", result)
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_PartiallyPresent(t *testing.T) {
|
||||
env := map[string]string{"SET": "yes"}
|
||||
result := expandWithEnv("${SET} and ${NOT_SET}", env)
|
||||
assert.Equal(t, "yes and ${NOT_SET}", result)
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_EmbeddedMissingProcessEnvStaysLiteral(t *testing.T) {
|
||||
t.Setenv("MOL_TEST_EMBEDDED_MISSING", "")
|
||||
|
||||
result := expandWithEnv("prefix/${MOL_TEST_EMBEDDED_MISSING}/suffix", map[string]string{})
|
||||
assert.Equal(t, "prefix/${MOL_TEST_EMBEDDED_MISSING}/suffix", result)
|
||||
}
|
||||
|
||||
// POSIX identifier guard regression tests (CWE-78 fix).
|
||||
// Keys not starting with [a-zA-Z_] must not be looked up in env or os.Getenv.
|
||||
func TestExpandWithEnv_DigitPrefix_NotExpanded(t *testing.T) {
|
||||
// ${0}, ${5}, ${1VAR} — numeric prefix → not a valid shell identifier.
|
||||
// Guard must return "$0", "$5", "$1VAR" literally; no env lookup.
|
||||
cases := []struct {
|
||||
input string
|
||||
want string
|
||||
}{
|
||||
{"${0}", "$0"},
|
||||
{"${5}", "$5"},
|
||||
{"${1VAR}", "$1VAR"},
|
||||
{"prefix ${0} suffix", "prefix $0 suffix"},
|
||||
{"$0", "$0"},
|
||||
{"$5", "$5"},
|
||||
{"HOME=${HOME}", "HOME=${HOME}"}, // HOME is valid but embedded in larger string
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.input, func(t *testing.T) {
|
||||
got := expandWithEnv(tc.input, map[string]string{})
|
||||
assert.Equal(t, tc.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_EmptyKey_ReturnsDollar(t *testing.T) {
|
||||
// ${} → "$" (empty key, guard returns "$")
|
||||
result := expandWithEnv("value=${}", map[string]string{})
|
||||
assert.Equal(t, "value=$", result)
|
||||
}
|
||||
|
||||
// mergeCategoryRouting tests — unions defaults with per-workspace routing.
|
||||
|
||||
// ── Additional coverage: mergeCategoryRouting ──────────────────────
|
||||
func TestMergeCategoryRouting_WorkspaceAddsCategory(t *testing.T) {
|
||||
defaults := map[string][]string{
|
||||
"security": {"Backend Engineer"},
|
||||
}
|
||||
wsRouting := map[string][]string{
|
||||
"ui": {"Frontend Engineer"},
|
||||
}
|
||||
result := mergeCategoryRouting(defaults, wsRouting)
|
||||
assert.Equal(t, []string{"Backend Engineer"}, result["security"])
|
||||
assert.Equal(t, []string{"Frontend Engineer"}, result["ui"])
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyListDropsCategory(t *testing.T) {
|
||||
defaults := map[string][]string{
|
||||
"security": {"Backend Engineer"},
|
||||
"infra": {"SRE"},
|
||||
}
|
||||
wsRouting := map[string][]string{
|
||||
"security": {}, // empty list = explicit drop
|
||||
}
|
||||
result := mergeCategoryRouting(defaults, wsRouting)
|
||||
_, hasSecurity := result["security"]
|
||||
assert.False(t, hasSecurity)
|
||||
assert.Equal(t, []string{"SRE"}, result["infra"])
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyDefaultKeySkipped(t *testing.T) {
|
||||
defaults := map[string][]string{
|
||||
"": {"Backend Engineer"}, // empty key should be skipped
|
||||
}
|
||||
result := mergeCategoryRouting(defaults, nil)
|
||||
_, has := result[""]
|
||||
assert.False(t, has)
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyWorkspaceKeySkipped(t *testing.T) {
|
||||
defaults := map[string][]string{
|
||||
"security": {"Backend Engineer"},
|
||||
}
|
||||
wsRouting := map[string][]string{
|
||||
"": {"Some Role"},
|
||||
}
|
||||
result := mergeCategoryRouting(defaults, wsRouting)
|
||||
_, has := result[""]
|
||||
assert.False(t, has)
|
||||
assert.Equal(t, []string{"Backend Engineer"}, result["security"])
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_DoesNotMutateInputs(t *testing.T) {
|
||||
defaults := map[string][]string{
|
||||
"security": {"Backend Engineer"},
|
||||
}
|
||||
wsRouting := map[string][]string{
|
||||
"security": {"DevOps"},
|
||||
}
|
||||
orig := defaults["security"][0]
|
||||
_ = mergeCategoryRouting(defaults, wsRouting)
|
||||
assert.Equal(t, orig, defaults["security"][0])
|
||||
}
|
||||
|
||||
// renderCategoryRoutingYAML tests — deterministic YAML emission.
|
||||
|
||||
// ── Additional coverage: renderCategoryRoutingYAML ────────────────
|
||||
func TestRenderCategoryRoutingYAML_SingleCategory(t *testing.T) {
|
||||
routing := map[string][]string{
|
||||
"security": {"Backend Engineer", "DevOps"},
|
||||
}
|
||||
result, err := renderCategoryRoutingYAML(routing)
|
||||
assert.NoError(t, err)
|
||||
assert.Contains(t, result, "security:")
|
||||
assert.Contains(t, result, "Backend Engineer")
|
||||
assert.Contains(t, result, "DevOps")
|
||||
}
|
||||
|
||||
func TestRenderCategoryRoutingYAML_MultipleCategoriesSorted(t *testing.T) {
|
||||
routing := map[string][]string{
|
||||
"zebra": {"RoleZ"},
|
||||
"alpha": {"RoleA"},
|
||||
"middleware": {"RoleM"},
|
||||
}
|
||||
result, err := renderCategoryRoutingYAML(routing)
|
||||
assert.NoError(t, err)
|
||||
// Keys are sorted alphabetically.
|
||||
idxAlpha := assertFind(t, result, "alpha:")
|
||||
idxZebra := assertFind(t, result, "zebra:")
|
||||
idxMid := assertFind(t, result, "middleware:")
|
||||
if idxAlpha > -1 && idxZebra > -1 {
|
||||
assert.True(t, idxAlpha < idxZebra, "alpha should appear before zebra")
|
||||
}
|
||||
if idxMid > -1 && idxZebra > -1 {
|
||||
assert.True(t, idxMid < idxZebra, "middleware should appear before zebra")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRenderCategoryRoutingYAML_EmptyListCategory(t *testing.T) {
|
||||
// Empty-list category should still render (mergeCategoryRouting drops
|
||||
// them before they reach this function, but we test the render in isolation).
|
||||
routing := map[string][]string{
|
||||
"security": {},
|
||||
}
|
||||
result, err := renderCategoryRoutingYAML(routing)
|
||||
assert.NoError(t, err)
|
||||
assert.Contains(t, result, "security:")
|
||||
}
|
||||
|
||||
func TestRenderCategoryRoutingYAML_SpecialCharactersEscaped(t *testing.T) {
|
||||
routing := map[string][]string{
|
||||
"notes": {`has: colon`, `and "quotes"`, "emoji: 🚀"},
|
||||
}
|
||||
result, err := renderCategoryRoutingYAML(routing)
|
||||
assert.NoError(t, err)
|
||||
// Should not panic and should produce valid YAML.
|
||||
assert.Contains(t, result, "notes:")
|
||||
}
|
||||
|
||||
// appendYAMLBlock tests — safe concatenation with newline boundary.
|
||||
|
||||
// ── Additional coverage: appendYAMLBlock ───────────────────────────
|
||||
func TestAppendYAMLBlock_BothEmpty(t *testing.T) {
|
||||
result := appendYAMLBlock(nil, "")
|
||||
assert.Nil(t, result)
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_ExistingHasNewline(t *testing.T) {
|
||||
existing := []byte("existing:\n")
|
||||
block := "key: value\n"
|
||||
result := appendYAMLBlock(existing, block)
|
||||
assert.Equal(t, "existing:\nkey: value\n", string(result))
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_ExistingNoNewline(t *testing.T) {
|
||||
existing := []byte("existing:")
|
||||
block := "key: value\n"
|
||||
result := appendYAMLBlock(existing, block)
|
||||
assert.Equal(t, "existing:\nkey: value\n", string(result))
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_ExistingEmpty(t *testing.T) {
|
||||
existing := []byte("")
|
||||
block := "key: value\n"
|
||||
result := appendYAMLBlock(existing, block)
|
||||
assert.Equal(t, "key: value\n", string(result))
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_NilExisting(t *testing.T) {
|
||||
block := "key: value\n"
|
||||
result := appendYAMLBlock(nil, block)
|
||||
assert.Equal(t, "key: value\n", string(result))
|
||||
}
|
||||
|
||||
// mergePlugins tests — union with exclusion prefix (!/-).
|
||||
|
||||
// ── Additional coverage: mergePlugins (additional cases) ───────────
|
||||
func TestMergePlugins_DefaultsOnly(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b"}
|
||||
result := mergePlugins(defaults, nil)
|
||||
assert.Equal(t, []string{"plugin-a", "plugin-b"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_WorkspaceAdds(t *testing.T) {
|
||||
defaults := []string{"plugin-a"}
|
||||
wsPlugins := []string{"plugin-b", "plugin-a"} // duplicate of default
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-a", "plugin-b"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExclusionWithBang(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b", "plugin-c"}
|
||||
wsPlugins := []string{"!plugin-b"}
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-a", "plugin-c"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExclusionWithDash(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b", "plugin-c"}
|
||||
wsPlugins := []string{"-plugin-b"}
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-a", "plugin-c"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExclusionEmptyTarget(t *testing.T) {
|
||||
defaults := []string{"plugin-a", "plugin-b"}
|
||||
wsPlugins := []string{"!", "-"} // no-op exclusions
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-a", "plugin-b"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExclusionNotInDefaults(t *testing.T) {
|
||||
// Excluding something not in defaults is a no-op.
|
||||
defaults := []string{"plugin-a"}
|
||||
wsPlugins := []string{"!plugin-b"}
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-a"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_WorkspaceAddsNew(t *testing.T) {
|
||||
defaults := []string{"plugin-a"}
|
||||
wsPlugins := []string{"plugin-b"}
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-a", "plugin-b"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_DeduplicationOrder(t *testing.T) {
|
||||
// Defaults first; workspace entries deduplicated.
|
||||
defaults := []string{"plugin-a", "plugin-a", "plugin-b"}
|
||||
wsPlugins := []string{"plugin-b", "plugin-c", "plugin-c"}
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-a", "plugin-b", "plugin-c"}, result)
|
||||
}
|
||||
|
||||
func TestMergePlugins_ExclusionThenAddSameName(t *testing.T) {
|
||||
// Remove then re-add: order matters.
|
||||
defaults := []string{"plugin-a", "plugin-b"}
|
||||
wsPlugins := []string{"!plugin-a", "plugin-a"}
|
||||
result := mergePlugins(defaults, wsPlugins)
|
||||
assert.Equal(t, []string{"plugin-b", "plugin-a"}, result)
|
||||
}
|
||||
|
||||
// isSafeRoleName tests — alphanumeric + hyphen/underscore, no path separators.
|
||||
|
||||
// ── Additional coverage: isSafeRoleName ───────────────────────────
|
||||
func TestIsSafeRoleName_SpecialCharsRejected(t *testing.T) {
|
||||
bad := []string{
|
||||
"role@name",
|
||||
"role#name",
|
||||
"role$name",
|
||||
"role%name",
|
||||
"role&name",
|
||||
"role*name",
|
||||
"role?name",
|
||||
"role=name",
|
||||
}
|
||||
for _, r := range bad {
|
||||
if isSafeRoleName(r) {
|
||||
t.Errorf("isSafeRoleName(%q) expected false, got true", r)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// assertFind is a helper: returns index of first occurrence of substr in s, or -1.
|
||||
func assertFind(t *testing.T, s, substr string) int {
|
||||
t.Helper()
|
||||
idx := -1
|
||||
for i := 0; i <= len(s)-len(substr); i++ {
|
||||
if s[i:i+len(substr)] == substr {
|
||||
idx = i
|
||||
break
|
||||
}
|
||||
}
|
||||
return idx
|
||||
}
|
||||
@@ -45,13 +45,19 @@ func TestResolveInsideRoot_DotDotTraversal(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestResolveInsideRoot_DotDotWithIntermediate(t *testing.T) {
|
||||
// a/b/../../c should escape if a/b is not under root
|
||||
got, err := resolveInsideRoot("/safe/root", "a/b/../../c")
|
||||
if err == nil {
|
||||
t.Fatalf("dotdot with intermediate: expected error, got %q", got)
|
||||
// a/b/../../c normalises to "c" — a valid descendant inside any root.
|
||||
// Must use t.TempDir() for a real filesystem path so filepath.Abs resolves.
|
||||
root := t.TempDir()
|
||||
got, err := resolveInsideRoot(root, "a/b/../../c")
|
||||
if err != nil {
|
||||
t.Fatalf("a/b/../../c should resolve within root: %v", err)
|
||||
}
|
||||
if err.Error() != "path escapes root" {
|
||||
t.Errorf("dotdot with intermediate: got %q, want %q", err.Error(), "path escapes root")
|
||||
// Verify result is inside root and ends with "c"
|
||||
if !strings.HasPrefix(got, root+string(filepath.Separator)) {
|
||||
t.Errorf("result should be inside root %q, got %q", root, got)
|
||||
}
|
||||
if got[len(got)-1:] != "c" {
|
||||
t.Errorf("resolved path should end in 'c', got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,7 +93,7 @@ func TestResolveInsideRoot_DotPathComponent(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("dot path component: unexpected error: %v", err)
|
||||
}
|
||||
if got[len(got)-14:] != "/subdir/file.txt" {
|
||||
if !strings.HasSuffix(got, "/subdir/file.txt") {
|
||||
t.Errorf("dot path component: got %q, want suffix /subdir/file.txt", got)
|
||||
}
|
||||
}
|
||||
@@ -132,23 +138,6 @@ func TestResolveInsideRoot_SiblingNotEscaped(t *testing.T) {
|
||||
|
||||
// ── isSafeRoleName ────────────────────────────────────────────────────────────
|
||||
|
||||
func TestIsSafeRoleName_Valid(t *testing.T) {
|
||||
valid := []string{
|
||||
"backend",
|
||||
"Frontend-Engineer",
|
||||
"research_lead",
|
||||
"devOps123",
|
||||
"a",
|
||||
"A",
|
||||
"team_42-leads",
|
||||
}
|
||||
for _, name := range valid {
|
||||
if !isSafeRoleName(name) {
|
||||
t.Errorf("isSafeRoleName(%q): expected true, got false", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsSafeRoleName_Empty(t *testing.T) {
|
||||
if isSafeRoleName("") {
|
||||
t.Error("isSafeRoleName(\"\"): expected false, got true")
|
||||
@@ -262,33 +251,6 @@ func TestMergeCategoryRouting_WsOverrideDropsDefault(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyListDropsCategory(t *testing.T) {
|
||||
defaultRouting := map[string][]string{
|
||||
"security": {"Backend Engineer"},
|
||||
"ui": {"Frontend Engineer"},
|
||||
}
|
||||
wsRouting := map[string][]string{
|
||||
"security": {}, // empty list = opt out
|
||||
}
|
||||
got := mergeCategoryRouting(defaultRouting, wsRouting)
|
||||
if _, exists := got["security"]; exists {
|
||||
t.Error("empty ws list should delete the category from output")
|
||||
}
|
||||
if len(got["ui"]) != 1 {
|
||||
t.Errorf("ui should still exist: got %v", got["ui"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyKeySkipped(t *testing.T) {
|
||||
defaultRouting := map[string][]string{
|
||||
"": {"Backend Engineer"},
|
||||
}
|
||||
got := mergeCategoryRouting(defaultRouting, nil)
|
||||
if _, exists := got[""]; exists {
|
||||
t.Error("empty key should be skipped")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMergeCategoryRouting_EmptyRolesInDefaultSkipped(t *testing.T) {
|
||||
defaultRouting := map[string][]string{
|
||||
"security": {},
|
||||
|
||||
@@ -356,12 +356,6 @@ func TestExpandWithEnv_UnsetVar(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasUnresolvedVarRef_NoVars(t *testing.T) {
|
||||
if hasUnresolvedVarRef("plain text", "plain text") {
|
||||
t.Error("plain text should not be flagged")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasUnresolvedVarRef_LiteralDollar(t *testing.T) {
|
||||
// "$5" is a literal price, not a var ref — should NOT be flagged
|
||||
if hasUnresolvedVarRef("price: $5", "price: $5") {
|
||||
@@ -369,20 +363,6 @@ func TestHasUnresolvedVarRef_LiteralDollar(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasUnresolvedVarRef_Resolved(t *testing.T) {
|
||||
// Original had ${VAR}, expanded to "value" — fully resolved
|
||||
if hasUnresolvedVarRef("${VAR}", "value") {
|
||||
t.Error("fully resolved var should not be flagged")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasUnresolvedVarRef_Unresolved(t *testing.T) {
|
||||
// Original had ${VAR}, expanded to "" — unresolved
|
||||
if !hasUnresolvedVarRef("${VAR}", "") {
|
||||
t.Error("unresolved var should be flagged")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasUnresolvedVarRef_DollarVarSyntax(t *testing.T) {
|
||||
// $VAR syntax (no braces) — also a real ref
|
||||
if !hasUnresolvedVarRef("$MISSING_VAR", "") {
|
||||
@@ -1079,105 +1059,6 @@ func TestCollectOrgEnv_AnyOfWithInvalidMemberKeepsValidOnes(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// walkOrgWorkspaceNames tests
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestWalkOrgWorkspaceNames_Empty(t *testing.T) {
|
||||
var names []string
|
||||
walkOrgWorkspaceNames(nil, &names)
|
||||
if len(names) != 0 {
|
||||
t.Errorf("empty tree: expected 0 names, got %d", len(names))
|
||||
}
|
||||
}
|
||||
|
||||
func TestWalkOrgWorkspaceNames_SingleNode(t *testing.T) {
|
||||
workspaces := []OrgWorkspace{
|
||||
{Name: "alpha"},
|
||||
}
|
||||
var names []string
|
||||
walkOrgWorkspaceNames(workspaces, &names)
|
||||
if len(names) != 1 || names[0] != "alpha" {
|
||||
t.Errorf("single node: got %v", names)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWalkOrgWorkspaceNames_NestedChildren(t *testing.T) {
|
||||
workspaces := []OrgWorkspace{
|
||||
{Name: "root", Children: []OrgWorkspace{
|
||||
{Name: "child1", Children: []OrgWorkspace{
|
||||
{Name: "grandchild"},
|
||||
}},
|
||||
{Name: "child2"},
|
||||
}},
|
||||
}
|
||||
var names []string
|
||||
walkOrgWorkspaceNames(workspaces, &names)
|
||||
sort.Strings(names)
|
||||
want := []string{"child1", "child2", "grandchild", "root"}
|
||||
if !stringSlicesEqual(names, want) {
|
||||
t.Errorf("nested: got %v, want %v", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWalkOrgWorkspaceNames_SkipsEmptyNames(t *testing.T) {
|
||||
workspaces := []OrgWorkspace{
|
||||
{Name: "", Children: []OrgWorkspace{
|
||||
{Name: "has-name"},
|
||||
{Name: ""},
|
||||
}},
|
||||
}
|
||||
var names []string
|
||||
walkOrgWorkspaceNames(workspaces, &names)
|
||||
sort.Strings(names)
|
||||
want := []string{"has-name"}
|
||||
if !stringSlicesEqual(names, want) {
|
||||
t.Errorf("skips empty: got %v, want %v", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWalkOrgWorkspaceNames_DeeplyNested(t *testing.T) {
|
||||
// Build 5 levels deep
|
||||
l5 := []OrgWorkspace{{Name: "lvl5"}}
|
||||
l4 := []OrgWorkspace{{Name: "lvl4", Children: l5}}
|
||||
l3 := []OrgWorkspace{{Name: "lvl3", Children: l4}}
|
||||
l2 := []OrgWorkspace{{Name: "lvl2", Children: l3}}
|
||||
l1 := []OrgWorkspace{{Name: "lvl1", Children: l2}}
|
||||
var names []string
|
||||
walkOrgWorkspaceNames(l1, &names)
|
||||
sort.Strings(names)
|
||||
want := []string{"lvl1", "lvl2", "lvl3", "lvl4", "lvl5"}
|
||||
if !stringSlicesEqual(names, want) {
|
||||
t.Errorf("deeply nested: got %v, want %v", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWalkOrgWorkspaceNames_MultipleRoots(t *testing.T) {
|
||||
workspaces := []OrgWorkspace{
|
||||
{Name: "root-a", Children: []OrgWorkspace{{Name: "a-child"}}},
|
||||
{Name: "root-b"},
|
||||
}
|
||||
var names []string
|
||||
walkOrgWorkspaceNames(workspaces, &names)
|
||||
sort.Strings(names)
|
||||
want := []string{"a-child", "root-a", "root-b"}
|
||||
if !stringSlicesEqual(names, want) {
|
||||
t.Errorf("multiple roots: got %v, want %v", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// resolveProvisionConcurrency tests
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestResolveProvisionConcurrency_Default(t *testing.T) {
|
||||
t.Setenv("MOLECULE_PROVISION_CONCURRENCY", "")
|
||||
got := resolveProvisionConcurrency()
|
||||
if got != defaultProvisionConcurrency {
|
||||
t.Errorf("unset: got %d, want %d", got, defaultProvisionConcurrency)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveProvisionConcurrency_ValidPositive(t *testing.T) {
|
||||
t.Setenv("MOLECULE_PROVISION_CONCURRENCY", "8")
|
||||
got := resolveProvisionConcurrency()
|
||||
|
||||
@@ -0,0 +1,310 @@
|
||||
package handlers
|
||||
|
||||
// plugins_atomic_tar_test.go — unit tests for tarWalk (the only non-trivial
|
||||
// function in plugins_atomic_tar.go). The file contains only pure tar-walk
|
||||
// logic with no DB or HTTP dependencies, so tests use real temp directories
|
||||
// with no mocking.
|
||||
|
||||
import (
|
||||
"archive/tar"
|
||||
"bytes"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// ─── newTarWriter ─────────────────────────────────────────────────────────────
|
||||
|
||||
func TestNewTarWriter_Basic(t *testing.T) {
|
||||
var buf bytes.Buffer
|
||||
tw := newTarWriter(&buf)
|
||||
if tw == nil {
|
||||
t.Fatal("newTarWriter returned nil")
|
||||
}
|
||||
// Write a header to prove the writer is functional.
|
||||
hdr := &tar.Header{
|
||||
Name: "test.txt",
|
||||
Mode: 0644,
|
||||
Size: 5,
|
||||
}
|
||||
if err := tw.WriteHeader(hdr); err != nil {
|
||||
t.Fatalf("WriteHeader failed: %v", err)
|
||||
}
|
||||
if _, err := tw.Write([]byte("hello")); err != nil {
|
||||
t.Fatalf("Write failed: %v", err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatalf("Close failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ─── tarWalk: empty directory ─────────────────────────────────────────────────
|
||||
|
||||
func TestTarWalk_EmptyDir(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
var buf bytes.Buffer
|
||||
tw := tar.NewWriter(&buf)
|
||||
|
||||
if err := tarWalk(tmp, "prefix", tw); err != nil {
|
||||
t.Fatalf("tarWalk error: %v", err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatalf("tw.Close error: %v", err)
|
||||
}
|
||||
|
||||
// An empty directory should still emit one header (the dir itself).
|
||||
rdr := tar.NewReader(&buf)
|
||||
hdr, err := rdr.Next()
|
||||
if err != nil {
|
||||
t.Fatalf("expected at least the dir header, got error: %v", err)
|
||||
}
|
||||
if !strings.HasSuffix(hdr.Name, "/") {
|
||||
t.Errorf("expected directory name ending in '/', got %q", hdr.Name)
|
||||
}
|
||||
|
||||
// No more entries.
|
||||
if _, err := rdr.Next(); err != io.EOF {
|
||||
t.Errorf("expected only one header, got more: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ─── tarWalk: single file ─────────────────────────────────────────────────────
|
||||
|
||||
func TestTarWalk_SingleFile(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(tmp, "hello.txt"), []byte("world"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
tw := tar.NewWriter(&buf)
|
||||
if err := tarWalk(tmp, "mydir", tw); err != nil {
|
||||
t.Fatalf("tarWalk error: %v", err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Should have 2 entries: the dir prefix, then hello.txt.
|
||||
entries := 0
|
||||
names := []string{}
|
||||
rdr := tar.NewReader(&buf)
|
||||
for {
|
||||
hdr, err := rdr.Next()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error reading tar: %v", err)
|
||||
}
|
||||
entries++
|
||||
names = append(names, hdr.Name)
|
||||
|
||||
if hdr.Name == "mydir/hello.txt" {
|
||||
if hdr.Size != 5 {
|
||||
t.Errorf("expected size 5, got %d", hdr.Size)
|
||||
}
|
||||
content := make([]byte, 5)
|
||||
if _, err := rdr.Read(content); err != nil && err != io.EOF {
|
||||
t.Fatalf("read error: %v", err)
|
||||
}
|
||||
if string(content) != "world" {
|
||||
t.Errorf("expected 'world', got %q", string(content))
|
||||
}
|
||||
}
|
||||
}
|
||||
if entries != 2 {
|
||||
t.Errorf("expected 2 entries, got %d: %v", entries, names)
|
||||
}
|
||||
}
|
||||
|
||||
// ─── tarWalk: nested directories ───────────────────────────────────────────────
|
||||
|
||||
func TestTarWalk_NestedDirs(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
subdir := filepath.Join(tmp, "a", "b", "c")
|
||||
if err := os.MkdirAll(subdir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(subdir, "deep.txt"), []byte("nested"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
tw := tar.NewWriter(&buf)
|
||||
if err := tarWalk(tmp, "root", tw); err != nil {
|
||||
t.Fatalf("tarWalk error: %v", err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Collect all file paths (not dirs) with content.
|
||||
files := map[string]string{}
|
||||
rdr := tar.NewReader(&buf)
|
||||
for {
|
||||
hdr, err := rdr.Next()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.HasSuffix(hdr.Name, "/") && hdr.Size > 0 {
|
||||
content := make([]byte, hdr.Size)
|
||||
rdr.Read(content)
|
||||
files[hdr.Name] = string(content)
|
||||
}
|
||||
}
|
||||
|
||||
expected := "root/a/b/c/deep.txt"
|
||||
if _, ok := files[expected]; !ok {
|
||||
t.Errorf("expected file %q in tar; got: %v", expected, files)
|
||||
} else if files[expected] != "nested" {
|
||||
t.Errorf("expected content 'nested', got %q", files[expected])
|
||||
}
|
||||
}
|
||||
|
||||
// ─── tarWalk: symlinks are skipped ────────────────────────────────────────────
|
||||
|
||||
func TestTarWalk_SymlinksSkipped(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
|
||||
// Create a real file.
|
||||
realPath := filepath.Join(tmp, "real.txt")
|
||||
if err := os.WriteFile(realPath, []byte("real content"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Create a symlink to it.
|
||||
linkPath := filepath.Join(tmp, "link.txt")
|
||||
if err := os.Symlink(realPath, linkPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
tw := tar.NewWriter(&buf)
|
||||
if err := tarWalk(tmp, "prefix", tw); err != nil {
|
||||
t.Fatalf("tarWalk error: %v", err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Only real.txt should appear; link.txt should be absent.
|
||||
names := []string{}
|
||||
rdr := tar.NewReader(&buf)
|
||||
for {
|
||||
hdr, err := rdr.Next()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
names = append(names, hdr.Name)
|
||||
}
|
||||
|
||||
foundLink := false
|
||||
for _, n := range names {
|
||||
if strings.Contains(n, "link") {
|
||||
foundLink = true
|
||||
}
|
||||
}
|
||||
if foundLink {
|
||||
t.Errorf("symlink should be skipped; got names: %v", names)
|
||||
}
|
||||
}
|
||||
|
||||
// ─── tarWalk: prefix trailing slash is normalized ─────────────────────────────
|
||||
|
||||
func TestTarWalk_PrefixTrailingSlashNormalized(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(tmp, "f.txt"), []byte("x"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
tw := tar.NewWriter(&buf)
|
||||
// Pass prefix WITH trailing slash — should produce same archive as without.
|
||||
if err := tarWalk(tmp, "foo/", tw); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// The file should be under "foo/", not "foo//".
|
||||
rdr := tar.NewReader(&buf)
|
||||
for {
|
||||
hdr, err := rdr.Next()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.HasSuffix(hdr.Name, "/") && strings.Contains(hdr.Name, "f.txt") {
|
||||
if strings.Contains(hdr.Name, "//") {
|
||||
t.Errorf("double slash found in path %q — trailing slash not normalized", hdr.Name)
|
||||
}
|
||||
if !strings.HasPrefix(hdr.Name, "foo/") {
|
||||
t.Errorf("expected path to start with 'foo/', got %q", hdr.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ─── tarWalk: prefix = "." emits flat paths ───────────────────────────────────
|
||||
|
||||
func TestTarWalk_PrefixDotEmitsFlatPaths(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
subdir := filepath.Join(tmp, "sub")
|
||||
if err := os.MkdirAll(subdir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(subdir, "file.txt"), []byte("data"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
tw := tar.NewWriter(&buf)
|
||||
if err := tarWalk(tmp, ".", tw); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// With prefix ".", paths should NOT start with "./" (filepath.Clean normalizes it).
|
||||
rdr := tar.NewReader(&buf)
|
||||
for {
|
||||
hdr, err := rdr.Next()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.HasSuffix(hdr.Name, "/") && strings.Contains(hdr.Name, "file.txt") {
|
||||
if strings.HasPrefix(hdr.Name, "./") {
|
||||
t.Errorf("prefix '.' should not emit './' prefix; got %q", hdr.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ─── tarWalk: walk error propagates ───────────────────────────────────────────
|
||||
|
||||
func TestTarWalk_NonexistentDir(t *testing.T) {
|
||||
nonexistent := filepath.Join(t.TempDir(), "does-not-exist")
|
||||
var buf bytes.Buffer
|
||||
tw := tar.NewWriter(&buf)
|
||||
|
||||
err := tarWalk(nonexistent, "x", tw)
|
||||
if err == nil {
|
||||
t.Error("expected error for nonexistent directory, got nil")
|
||||
}
|
||||
}
|
||||
@@ -215,51 +215,6 @@ func TestTarWalk_EmptyDirectory(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestTarWalk_NestedDirs: deeply nested directories produce all intermediate
|
||||
// dir entries plus leaf entries. This exercises the recursive walk.
|
||||
func TestTarWalk_NestedDirs(t *testing.T) {
|
||||
hostDir := t.TempDir()
|
||||
deep := filepath.Join(hostDir, "a", "b", "c")
|
||||
if err := os.MkdirAll(deep, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(deep, "leaf.txt"), []byte("content"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var buf bytes.Buffer
|
||||
tw := newTarWriter(&buf)
|
||||
if err := tarWalk(hostDir, "configs/plugins/.staging", tw); err != nil {
|
||||
t.Fatalf("tarWalk: %v", err)
|
||||
}
|
||||
if err := tw.Close(); err != nil {
|
||||
t.Fatalf("Close: %v", err)
|
||||
}
|
||||
entries := readTarNames(&buf)
|
||||
// Must include: prefix/, prefix/a/, prefix/a/b/, prefix/a/b/c/, prefix/a/b/c/leaf.txt
|
||||
expected := []string{
|
||||
"configs/plugins/.staging/",
|
||||
"configs/plugins/.staging/a/",
|
||||
"configs/plugins/.staging/a/b/",
|
||||
"configs/plugins/.staging/a/b/c/",
|
||||
"configs/plugins/.staging/a/b/c/leaf.txt",
|
||||
}
|
||||
if len(entries) != len(expected) {
|
||||
t.Errorf("nested dirs: got %d entries; want %d: %v", len(entries), len(expected), entries)
|
||||
}
|
||||
for _, e := range expected {
|
||||
found := false
|
||||
for _, g := range entries {
|
||||
if g == e {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("missing entry: %q", e)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestTarWalk_DirEntryHasTrailingSlash: directory entries must end with '/'
|
||||
// per tar format; tar.Header.Typeflag '5' (dir) must produce "name/" not "name".
|
||||
func TestTarWalk_DirEntryHasTrailingSlash(t *testing.T) {
|
||||
|
||||
@@ -342,6 +342,11 @@ func TestPluginInstall_InstanceLookupError_Returns503(t *testing.T) {
|
||||
// ---------- dispatch: uninstall ----------
|
||||
|
||||
func TestPluginUninstall_SaaS_DispatchesToEIC(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock.ExpectExec("DELETE FROM workspace_plugins WHERE workspace_id").
|
||||
WithArgs("ws-1", "browser-automation").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
stubReadPluginManifestViaEIC(t, func(ctx context.Context, instanceID, runtime, pluginName string) ([]byte, error) {
|
||||
return []byte("name: browser-automation\nskills:\n - browse\n"), nil
|
||||
})
|
||||
|
||||
@@ -629,6 +629,9 @@ func TestPluginInstall_RejectsUnknownScheme(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestPluginInstall_LocalSourceReachesContainerLookup(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
expectAllowlistAllowAll(mock)
|
||||
|
||||
base := t.TempDir()
|
||||
pluginDir := filepath.Join(base, "demo")
|
||||
_ = os.MkdirAll(pluginDir, 0o755)
|
||||
@@ -955,14 +958,14 @@ func TestLogInstallLimitsOnce(t *testing.T) {
|
||||
|
||||
func TestRegexpEscapeForAwk(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"my-plugin": `my-plugin`,
|
||||
"# Plugin: foo /": `# Plugin: foo \/`,
|
||||
"# Plugin: a.b /": `# Plugin: a\.b \/`,
|
||||
"foo[bar]": `foo\[bar\]`,
|
||||
"a*b+c?": `a\*b\+c\?`,
|
||||
"path|with|pipes": `path\|with\|pipes`,
|
||||
`back\slash`: `back\\slash`,
|
||||
"": ``,
|
||||
"my-plugin": `my-plugin`,
|
||||
"# Plugin: foo /": `# Plugin: foo \/`,
|
||||
"# Plugin: a.b /": `# Plugin: a\.b \/`,
|
||||
"foo[bar]": `foo\[bar\]`,
|
||||
"a*b+c?": `a\*b\+c\?`,
|
||||
"path|with|pipes": `path\|with\|pipes`,
|
||||
`back\slash`: `back\\slash`,
|
||||
"": ``,
|
||||
}
|
||||
for in, want := range cases {
|
||||
got := regexpEscapeForAwk(in)
|
||||
@@ -1247,7 +1250,7 @@ func TestPluginDownload_GithubSchemeStreamsTarball(t *testing.T) {
|
||||
scheme: "github",
|
||||
fetchFn: func(_ context.Context, _ string, dst string) (string, error) {
|
||||
files := map[string]string{
|
||||
"plugin.yaml": "name: remote-plugin\nversion: 1.0.0\n",
|
||||
"plugin.yaml": "name: remote-plugin\nversion: 1.0.0\n",
|
||||
"skills/x/SKILL.md": "---\nname: x\n---\n",
|
||||
"adapters/claude_code.py": "from plugins_registry.builtins import AgentskillsAdaptor as Adaptor\n",
|
||||
}
|
||||
|
||||
@@ -58,7 +58,7 @@ func (h *WorkspaceHandler) gracefulPreRestart(ctx context.Context, workspaceID s
|
||||
// Non-blocking send — don't stall the restart cycle.
|
||||
// Run in a detached goroutine so the caller (runRestartCycle) can
|
||||
// proceed to stopForRestart without waiting.
|
||||
go func() {
|
||||
h.goAsync(func() {
|
||||
signalCtx, cancel := context.WithTimeout(context.Background(), restartSignalTimeout)
|
||||
defer cancel()
|
||||
|
||||
@@ -109,7 +109,7 @@ func (h *WorkspaceHandler) gracefulPreRestart(ctx context.Context, workspaceID s
|
||||
} else {
|
||||
log.Printf("A2AGracefulRestart: %s returned status %d — proceeding with stop", workspaceID, resp.StatusCode)
|
||||
}
|
||||
}()
|
||||
})
|
||||
}
|
||||
|
||||
// resolveAgentURLForRestartSignal returns the routable URL for the workspace
|
||||
|
||||
@@ -271,6 +271,7 @@ func TestGracefulPreRestart_URLResolutionError(t *testing.T) {
|
||||
WorkspaceHandler: newHandlerWithTestDeps(t),
|
||||
errToReturn: context.DeadlineExceeded,
|
||||
}
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, hWrapper.WorkspaceHandler)
|
||||
|
||||
hWrapper.gracefulPreRestart(context.Background(), "ws-url-err-111")
|
||||
time.Sleep(200 * time.Millisecond)
|
||||
|
||||
@@ -63,6 +63,9 @@ func (h *SecretsHandler) List(c *gin.Context) {
|
||||
"updated_at": updatedAt,
|
||||
})
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("List secrets rows.Err: %v", err)
|
||||
}
|
||||
|
||||
// 2. Global secrets not overridden at workspace level
|
||||
globalRows, err := db.DB.QueryContext(ctx,
|
||||
@@ -91,6 +94,9 @@ func (h *SecretsHandler) List(c *gin.Context) {
|
||||
"updated_at": updatedAt,
|
||||
})
|
||||
}
|
||||
if err := globalRows.Err(); err != nil {
|
||||
log.Printf("List secrets (global) rows.Err: %v", err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, secrets)
|
||||
}
|
||||
@@ -174,6 +180,9 @@ func (h *SecretsHandler) Values(c *gin.Context) {
|
||||
out[k] = string(decrypted)
|
||||
}
|
||||
}
|
||||
if err := globalRows.Err(); err != nil {
|
||||
log.Printf("secrets.Values globalRows.Err: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
wsRows, wErr := db.DB.QueryContext(ctx,
|
||||
@@ -195,6 +204,9 @@ func (h *SecretsHandler) Values(c *gin.Context) {
|
||||
out[k] = string(decrypted) // workspace override wins over global
|
||||
}
|
||||
}
|
||||
if err := wsRows.Err(); err != nil {
|
||||
log.Printf("secrets.Values wsRows.Err: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
if len(failedKeys) > 0 {
|
||||
@@ -324,6 +336,9 @@ func (h *SecretsHandler) ListGlobal(c *gin.Context) {
|
||||
"scope": "global",
|
||||
})
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("ListGlobal rows.Err: %v", err)
|
||||
}
|
||||
c.JSON(http.StatusOK, secrets)
|
||||
}
|
||||
|
||||
@@ -400,6 +415,9 @@ func (h *SecretsHandler) restartAllAffectedByGlobalKey(key string) {
|
||||
ids = append(ids, id)
|
||||
}
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("restartAllAffectedByGlobalKey rows.Err: %v", err)
|
||||
}
|
||||
if len(ids) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -340,6 +340,11 @@ func TestSSHCommandCmd_BuildsArgv(t *testing.T) {
|
||||
// a workspace must still be able to access its own terminal. The CanCommunicate
|
||||
// fast-path returns true when callerID == targetID.
|
||||
func TestTerminalConnect_KI005_AllowsOwnTerminal(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock.ExpectQuery("SELECT COALESCE").
|
||||
WithArgs("ws-alice").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"instance_id"}).AddRow(""))
|
||||
|
||||
// CanCommunicate fast-path: callerID == targetID → returns true without DB.
|
||||
prev := canCommunicateCheck
|
||||
canCommunicateCheck = func(callerID, targetID string) bool { return callerID == targetID }
|
||||
@@ -367,6 +372,11 @@ func TestTerminalConnect_KI005_AllowsOwnTerminal(t *testing.T) {
|
||||
// skip the CanCommunicate check entirely and fall through to the Docker auth path.
|
||||
// We assert they get the nil-docker 503 instead of 403.
|
||||
func TestTerminalConnect_KI005_SkipsCheckWithoutHeader(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock.ExpectQuery("SELECT COALESCE").
|
||||
WithArgs("ws-any").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"instance_id"}).AddRow(""))
|
||||
|
||||
h := NewTerminalHandler(nil) // nil docker → 503 if reached
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
@@ -439,6 +449,9 @@ func TestTerminalConnect_KI005_AllowsSiblingWorkspace(t *testing.T) {
|
||||
mock.ExpectExec(`UPDATE workspace_auth_tokens SET last_used_at`).
|
||||
WithArgs(sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectQuery("SELECT COALESCE").
|
||||
WithArgs("ws-dev").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"instance_id"}).AddRow(""))
|
||||
|
||||
h := NewTerminalHandler(nil)
|
||||
w := httptest.NewRecorder()
|
||||
@@ -463,7 +476,10 @@ func TestTerminalConnect_KI005_AllowsSiblingWorkspace(t *testing.T) {
|
||||
// introduced in GH#1885: internal routing uses org tokens which are not in
|
||||
// workspace_auth_tokens, so ValidateToken would always fail for them.
|
||||
func TestKI005_OrgToken_SkipsValidateToken(t *testing.T) {
|
||||
setupTestDB(t) // no ValidateToken ExpectQuery — none should fire
|
||||
mock := setupTestDB(t) // no ValidateToken ExpectQuery — none should fire
|
||||
mock.ExpectQuery("SELECT COALESCE").
|
||||
WithArgs("ws-target").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"instance_id"}).AddRow(""))
|
||||
prev := canCommunicateCheck
|
||||
canCommunicateCheck = func(callerID, targetID string) bool {
|
||||
// Simulate platform agent → target workspace (same org).
|
||||
@@ -544,4 +560,3 @@ func TestSSHCommandCmd_ConnectTimeoutPresent(t *testing.T) {
|
||||
args)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto"
|
||||
@@ -73,6 +74,19 @@ type WorkspaceHandler struct {
|
||||
// memory plugin). main.go sets this to plugin.DeleteNamespace
|
||||
// when MEMORY_PLUGIN_URL is configured.
|
||||
namespaceCleanupFn func(ctx context.Context, workspaceID string)
|
||||
asyncWG sync.WaitGroup
|
||||
}
|
||||
|
||||
func (h *WorkspaceHandler) goAsync(fn func()) {
|
||||
h.asyncWG.Add(1)
|
||||
go func() {
|
||||
defer h.asyncWG.Done()
|
||||
fn()
|
||||
}()
|
||||
}
|
||||
|
||||
func (h *WorkspaceHandler) waitAsyncForTest() {
|
||||
h.asyncWG.Wait()
|
||||
}
|
||||
|
||||
func NewWorkspaceHandler(b events.EventEmitter, p *provisioner.Provisioner, platformURL, configsDir string) *WorkspaceHandler {
|
||||
|
||||
@@ -111,11 +111,11 @@ func (h *WorkspaceHandler) provisionWorkspaceAuto(workspaceID, templatePath stri
|
||||
"sync": false,
|
||||
})
|
||||
if h.cpProv != nil {
|
||||
go h.provisionWorkspaceCP(workspaceID, templatePath, configFiles, payload)
|
||||
h.goAsync(func() { h.provisionWorkspaceCP(workspaceID, templatePath, configFiles, payload) })
|
||||
return true
|
||||
}
|
||||
if h.provisioner != nil {
|
||||
go h.provisionWorkspace(workspaceID, templatePath, configFiles, payload)
|
||||
h.goAsync(func() { h.provisionWorkspace(workspaceID, templatePath, configFiles, payload) })
|
||||
return true
|
||||
}
|
||||
// No backend wired — mark failed so the workspace doesn't linger in
|
||||
@@ -275,13 +275,13 @@ func (h *WorkspaceHandler) RestartWorkspaceAutoOpts(ctx context.Context, workspa
|
||||
if h.cpProv != nil {
|
||||
h.cpStopWithRetry(ctx, workspaceID, "RestartWorkspaceAuto")
|
||||
// resetClaudeSession is Docker-only — CP has no session state to clear.
|
||||
go h.provisionWorkspaceCP(workspaceID, templatePath, configFiles, payload)
|
||||
h.goAsync(func() { h.provisionWorkspaceCP(workspaceID, templatePath, configFiles, payload) })
|
||||
return true
|
||||
}
|
||||
if h.provisioner != nil {
|
||||
// Docker.Stop has no retry — see docstring rationale.
|
||||
h.provisioner.Stop(ctx, workspaceID)
|
||||
go h.provisionWorkspaceOpts(workspaceID, templatePath, configFiles, payload, resetClaudeSession)
|
||||
h.goAsync(func() { h.provisionWorkspaceOpts(workspaceID, templatePath, configFiles, payload, resetClaudeSession) })
|
||||
return true
|
||||
}
|
||||
// No backend wired — same shape as provisionWorkspaceAuto's no-backend
|
||||
|
||||
@@ -144,6 +144,7 @@ func TestProvisionWorkspaceAuto_RoutesToCPWhenSet(t *testing.T) {
|
||||
rec := &trackingCPProv{startErr: errors.New("simulated CP rejection")}
|
||||
bcast := &concurrentSafeBroadcaster{}
|
||||
h := NewWorkspaceHandler(bcast, nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, h)
|
||||
h.SetCPProvisioner(rec)
|
||||
|
||||
wsID := "ws-routes-to-cp-0123456789abcdef"
|
||||
@@ -595,6 +596,7 @@ func TestRestartWorkspaceAuto_RoutesToCPWhenSet(t *testing.T) {
|
||||
|
||||
// Mock DB so cpStopWithRetry can run without a real Postgres.
|
||||
mock := setupTestDB(t)
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, h)
|
||||
mock.MatchExpectationsInOrder(false)
|
||||
// provisionWorkspaceCP runs in the goroutine and will hit secrets
|
||||
// SELECTs + UPDATE workspace as failed (we make CP Start return
|
||||
@@ -670,6 +672,7 @@ func TestRestartWorkspaceAuto_RoutesToDockerWhenOnlyDocker(t *testing.T) {
|
||||
|
||||
bcast := &concurrentSafeBroadcaster{}
|
||||
h := NewWorkspaceHandler(bcast, nil, "http://localhost:8080", t.TempDir())
|
||||
waitForHandlerAsyncBeforeDBCleanup(t, h)
|
||||
stub := &stoppingLocalProv{}
|
||||
h.provisioner = stub
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package handlers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"os"
|
||||
@@ -634,6 +635,11 @@ func TestSeedInitialMemories_EmptyMemoriesNil(t *testing.T) {
|
||||
// ==================== buildProvisionerConfig ====================
|
||||
|
||||
func TestBuildProvisionerConfig_BasicFields(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock.ExpectQuery(`SELECT COALESCE\(workspace_dir`).
|
||||
WithArgs("ws-basic").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"workspace_dir", "workspace_access"}).AddRow("", "none"))
|
||||
|
||||
broadcaster := newTestBroadcaster()
|
||||
tmpDir := t.TempDir()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", tmpDir)
|
||||
@@ -678,6 +684,14 @@ func TestBuildProvisionerConfig_BasicFields(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestBuildProvisionerConfig_WorkspacePathFromEnv(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock.ExpectQuery(`SELECT COALESCE\(workspace_dir`).
|
||||
WithArgs("ws-env").
|
||||
WillReturnError(sql.ErrNoRows)
|
||||
mock.ExpectQuery(`SELECT digest FROM runtime_image_pins`).
|
||||
WithArgs("claude-code").
|
||||
WillReturnError(sql.ErrNoRows)
|
||||
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
|
||||
@@ -481,6 +481,22 @@ func (p *Provisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, e
|
||||
return "", fmt.Errorf("failed to create container: %w", err)
|
||||
}
|
||||
|
||||
// Seed /configs before the entrypoint starts. molecule-runtime reads
|
||||
// /configs/config.yaml immediately; post-start copy races fast runtimes
|
||||
// into a FileNotFoundError crash loop.
|
||||
if cfg.TemplatePath != "" {
|
||||
if err := p.CopyTemplateToContainer(ctx, resp.ID, cfg.TemplatePath); err != nil {
|
||||
_ = p.cli.ContainerRemove(ctx, resp.ID, container.RemoveOptions{Force: true})
|
||||
return "", fmt.Errorf("failed to copy template to container %s before start: %w", name, err)
|
||||
}
|
||||
}
|
||||
if len(cfg.ConfigFiles) > 0 {
|
||||
if err := p.WriteFilesToContainer(ctx, resp.ID, cfg.ConfigFiles); err != nil {
|
||||
_ = p.cli.ContainerRemove(ctx, resp.ID, container.RemoveOptions{Force: true})
|
||||
return "", fmt.Errorf("failed to write config files to container %s before start: %w", name, err)
|
||||
}
|
||||
}
|
||||
|
||||
if err := p.cli.ContainerStart(ctx, resp.ID, container.StartOptions{}); err != nil {
|
||||
// Clean up created container on start failure
|
||||
_ = p.cli.ContainerRemove(ctx, resp.ID, container.RemoveOptions{Force: true})
|
||||
@@ -496,20 +512,6 @@ func (p *Provisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, e
|
||||
// /configs and /workspace, then drops to agent via gosu). No per-start
|
||||
// chown needed here.
|
||||
|
||||
// Copy template files into /configs if TemplatePath is set
|
||||
if cfg.TemplatePath != "" {
|
||||
if err := p.CopyTemplateToContainer(ctx, resp.ID, cfg.TemplatePath); err != nil {
|
||||
log.Printf("Provisioner: warning — failed to copy template to container %s: %v", name, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Write generated config files into /configs if ConfigFiles is set
|
||||
if len(cfg.ConfigFiles) > 0 {
|
||||
if err := p.WriteFilesToContainer(ctx, resp.ID, cfg.ConfigFiles); err != nil {
|
||||
log.Printf("Provisioner: warning — failed to write config files to container %s: %v", name, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve the host-mapped port. Retry inspect up to 3 times if Docker hasn't
|
||||
// bound the ephemeral port yet (rare race under heavy load).
|
||||
hostURL := InternalURL(cfg.WorkspaceID) // fallback to Docker-internal
|
||||
|
||||
@@ -62,6 +62,24 @@ func TestValidateConfigSource_TemplateIsDirName(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartSeedsConfigsBeforeContainerStart(t *testing.T) {
|
||||
src, err := os.ReadFile("provisioner.go")
|
||||
if err != nil {
|
||||
t.Fatalf("read provisioner.go: %v", err)
|
||||
}
|
||||
text := string(src)
|
||||
copyTemplate := strings.Index(text, "p.CopyTemplateToContainer(ctx, resp.ID, cfg.TemplatePath)")
|
||||
writeFiles := strings.Index(text, "p.WriteFilesToContainer(ctx, resp.ID, cfg.ConfigFiles)")
|
||||
start := strings.Index(text, "p.cli.ContainerStart(ctx, resp.ID, container.StartOptions{})")
|
||||
|
||||
if copyTemplate < 0 || writeFiles < 0 || start < 0 {
|
||||
t.Fatalf("expected Start to copy template, write config files, and start container")
|
||||
}
|
||||
if copyTemplate >= start || writeFiles >= start {
|
||||
t.Fatalf("config seeding must happen before ContainerStart: copyTemplate=%d writeFiles=%d start=%d", copyTemplate, writeFiles, start)
|
||||
}
|
||||
}
|
||||
|
||||
// baseHostConfig returns a fresh HostConfig with typical pre-tier binds,
|
||||
// mimicking what Start() builds before calling ApplyTierConfig.
|
||||
func baseHostConfig(pluginsPath string) *container.HostConfig {
|
||||
|
||||
@@ -14,8 +14,9 @@ func setupMockDB(t *testing.T) sqlmock.Sqlmock {
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
t.Cleanup(func() { mockDB.Close(); db.DB = prevDB })
|
||||
return mock
|
||||
}
|
||||
|
||||
|
||||
@@ -31,8 +31,9 @@ func setupTestDB(t *testing.T) sqlmock.Sqlmock {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
t.Cleanup(func() { mockDB.Close(); db.DB = prevDB })
|
||||
return mock
|
||||
}
|
||||
|
||||
|
||||
@@ -17,8 +17,9 @@ func setupHibernationMock(t *testing.T) sqlmock.Sqlmock {
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock.New: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
t.Cleanup(func() { mockDB.Close(); db.DB = prevDB })
|
||||
return mock
|
||||
}
|
||||
|
||||
|
||||
@@ -18,8 +18,9 @@ func setupLivenessTestDB(t *testing.T) sqlmock.Sqlmock {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
t.Cleanup(func() { mockDB.Close(); db.DB = prevDB })
|
||||
return mock
|
||||
}
|
||||
|
||||
|
||||
@@ -24,8 +24,9 @@ func setupTestDB(t *testing.T) sqlmock.Sqlmock {
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create sqlmock: %v", err)
|
||||
}
|
||||
prevDB := db.DB
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { mockDB.Close() })
|
||||
t.Cleanup(func() { mockDB.Close(); db.DB = prevDB })
|
||||
return mock
|
||||
}
|
||||
|
||||
|
||||
@@ -686,8 +686,8 @@ def _format_channel_content(
|
||||
# --- MCP Server (JSON-RPC over stdio) ---
|
||||
|
||||
|
||||
def _warn_if_stdio_not_pipe(stdin_fd: int = 0, stdout_fd: int = 1) -> None:
|
||||
"""Warn when stdio isn't a pipe — but continue anyway.
|
||||
def _assert_stdio_is_pipe_compatible(stdin_fd: int = 0, stdout_fd: int = 1) -> None:
|
||||
"""Assert that stdio fds are pipe/socket/char-device compatible.
|
||||
|
||||
The legacy asyncio.connect_read_pipe / connect_write_pipe transport
|
||||
rejected regular files, PTYs, and sockets with:
|
||||
@@ -711,6 +711,10 @@ def _warn_if_stdio_not_pipe(stdin_fd: int = 0, stdout_fd: int = 1) -> None:
|
||||
)
|
||||
|
||||
|
||||
# Deprecated alias — the canonical name is _assert_stdio_is_pipe_compatible.
|
||||
_warn_if_stdio_not_pipe = _assert_stdio_is_pipe_compatible
|
||||
|
||||
|
||||
async def main(): # pragma: no cover
|
||||
"""Run MCP server on stdio — reads JSON-RPC requests, writes responses.
|
||||
|
||||
@@ -967,7 +971,7 @@ def cli_main(transport: str = "stdio", port: int = 9100) -> None: # pragma: no
|
||||
if transport == "http":
|
||||
asyncio.run(_run_http_server(port))
|
||||
else:
|
||||
_warn_if_stdio_not_pipe()
|
||||
_assert_stdio_is_pipe_compatible()
|
||||
asyncio.run(main())
|
||||
|
||||
|
||||
|
||||
@@ -1826,8 +1826,8 @@ def test_inbox_bridge_swallows_closed_loop_runtime_error():
|
||||
|
||||
|
||||
class TestStdioPipeAssertion:
|
||||
"""Pin _warn_if_stdio_not_pipe — the diagnostic warning that replaces
|
||||
the old fatal _assert_stdio_is_pipe_compatible guard.
|
||||
"""Pin _assert_stdio_is_pipe_compatible — the canonical function name.
|
||||
_warn_if_stdio_not_pipe is a deprecated alias.
|
||||
|
||||
The universal stdio transport now works with ANY file descriptor
|
||||
(pipes, regular files, PTYs, sockets), so the old exit-2 behavior
|
||||
@@ -1838,12 +1838,12 @@ class TestStdioPipeAssertion:
|
||||
|
||||
def test_pipe_pair_passes_silently(self, caplog):
|
||||
"""Happy path — both fds are pipes. No warning emitted."""
|
||||
from a2a_mcp_server import _warn_if_stdio_not_pipe
|
||||
from a2a_mcp_server import _assert_stdio_is_pipe_compatible
|
||||
|
||||
r, w = os.pipe()
|
||||
try:
|
||||
with caplog.at_level("WARNING"):
|
||||
_warn_if_stdio_not_pipe(stdin_fd=r, stdout_fd=w)
|
||||
_assert_stdio_is_pipe_compatible(stdin_fd=r, stdout_fd=w)
|
||||
assert "not a pipe" not in caplog.text
|
||||
finally:
|
||||
os.close(r)
|
||||
@@ -1852,14 +1852,14 @@ class TestStdioPipeAssertion:
|
||||
def test_regular_file_stdout_warns(self, tmp_path, caplog):
|
||||
"""Reproducer for runtime#61: stdout redirected to a regular file.
|
||||
Now emits a warning instead of exiting."""
|
||||
from a2a_mcp_server import _warn_if_stdio_not_pipe
|
||||
from a2a_mcp_server import _assert_stdio_is_pipe_compatible
|
||||
|
||||
r, _w = os.pipe()
|
||||
regular = tmp_path / "captured.log"
|
||||
f = open(regular, "wb")
|
||||
try:
|
||||
with caplog.at_level("WARNING"):
|
||||
_warn_if_stdio_not_pipe(stdin_fd=r, stdout_fd=f.fileno())
|
||||
_assert_stdio_is_pipe_compatible(stdin_fd=r, stdout_fd=f.fileno())
|
||||
assert "stdout" in caplog.text
|
||||
assert "not a pipe" in caplog.text
|
||||
finally:
|
||||
@@ -1868,7 +1868,7 @@ class TestStdioPipeAssertion:
|
||||
|
||||
def test_regular_file_stdin_warns(self, tmp_path, caplog):
|
||||
"""Symmetric case — stdin redirected from a regular file."""
|
||||
from a2a_mcp_server import _warn_if_stdio_not_pipe
|
||||
from a2a_mcp_server import _assert_stdio_is_pipe_compatible
|
||||
|
||||
regular = tmp_path / "input.json"
|
||||
regular.write_bytes(b'{"jsonrpc":"2.0","id":1,"method":"initialize"}\n')
|
||||
@@ -1876,7 +1876,7 @@ class TestStdioPipeAssertion:
|
||||
_r, w = os.pipe()
|
||||
try:
|
||||
with caplog.at_level("WARNING"):
|
||||
_warn_if_stdio_not_pipe(stdin_fd=f.fileno(), stdout_fd=w)
|
||||
_assert_stdio_is_pipe_compatible(stdin_fd=f.fileno(), stdout_fd=w)
|
||||
assert "stdin" in caplog.text
|
||||
assert "not a pipe" in caplog.text
|
||||
finally:
|
||||
@@ -1886,13 +1886,13 @@ class TestStdioPipeAssertion:
|
||||
def test_closed_fd_warns_about_stat_error(self, caplog):
|
||||
"""If stdio is closed, os.fstat raises OSError. Warning is
|
||||
skipped silently (can't stat the fd)."""
|
||||
from a2a_mcp_server import _warn_if_stdio_not_pipe
|
||||
from a2a_mcp_server import _assert_stdio_is_pipe_compatible
|
||||
|
||||
r, w = os.pipe()
|
||||
os.close(w) # Now `w` is a stale fd — fstat will fail.
|
||||
try:
|
||||
with caplog.at_level("WARNING"):
|
||||
_warn_if_stdio_not_pipe(stdin_fd=r, stdout_fd=w)
|
||||
_assert_stdio_is_pipe_compatible(stdin_fd=r, stdout_fd=w)
|
||||
# No warning emitted because fstat failed before the check
|
||||
assert "not a pipe" not in caplog.text
|
||||
finally:
|
||||
|
||||
Reference in New Issue
Block a user