feat: monitor pull requests across source providers - #5305
Conversation
github-actions
Bot
added
the
readiness: checking
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsFINDING -- src/kiro_crew/monitoring/pull_request.py:188 -- Azure’s combined status/policy overflow sets canonical False positive or not applicable? A repository writer can comment: |
github-actions
Bot
added
readiness: action required
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of I have everything I need. Reconciliation is complete: the one screenshot shows the new help line (blind reader understood it and would confidently use the URL field and Start monitor), but the PR body itself admits the three new error-notice states are not screenshotted, and the diff also newly disables the Restart button for session-closed monitors with no shown state. UX-Verdict: CONCERNS The multi-provider URL field and help line read cleanly cold, but every new error state ships unshown, and Restart now disables with no visible reason. Watch
Evidence gaps
[UX-REVIEWED] 0c8b2de |
First Principles Review (Fable 5) — 🔴 BLOCKPremise-level review of First-Principles-Verdict: BLOCK A third copy of the Windows suspended-spawn policy, plus a new Not justified as shipped
What this change shipsIntent: give GitLab, Azure DevOps and Bitbucket pull requests the zero-turn bounded monitoring GitHub already has. ADDITION.
BlockersWindows spawn policy duplicated. Zero-consumer accessor, contradicted framing. Grep Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 0c8b2de |
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound shared-contract adapter design with fail-closed credential provenance; the strict system-owned glab/az chain risks making the new monitors fail opaquely on common installs. WatchThe monitor-probe executable policy hard-requires a root-owned, non-writable chain ( Suggestions
[DESIGN-REVIEWED] 0c8b2de |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsNo findings. The candidate's own premise — that owner credentials on a non-target channel update are a leak — collapses on inspection: The residual "channel-authored wake text into the owner turn" concern is not a defect on these lines: [OPUS-REVIEWED] 0c8b2de Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
a35bfbe to
08562fe
Compare
github-actions
Bot
added
readiness: checking
08562fe to
6572db2
Compare
github-actions
Bot
added
readiness: action required
6572db2 to
434894c
Compare
github-actions
Bot
added
readiness: checking
434894c to
45fd3a0
Compare
github-actions
Bot
added
readiness: checking
45fd3a0 to
ed225b2
Compare
|
Addressed the terminal provider-change dead end in bed7329. Retained terminal monitors now expose a localized New monitor path; creation names the exact terminal record and the backend conditionally replaces only that id/configuration generation, refusing active or stale state with 409. Focused verification: 38 handler tests and 37 popover tests passed; TypeScript, ESLint, Python format/lint/type checks, i18n (including diff-scoped checks), docs lint, and diff check passed. The current Python 3.10 shard-4 failure is unrelated runner infrastructure: the job failed before checkout because GitHub could not resolve its internal action-download host; I did not rerun it. |
|
Addressed the current-head review blocker by redirecting GLAB_CONFIG_DIR and AZURE_CONFIG_DIR into the ephemeral pod home. Pods can no longer inherit the operator’s persisted GitLab or Azure CLI login stores through the shared HOME. Added a focused pod-env regression and updated the security spec; focused tests and docs/static checks pass. |
|
Fixed in |
|
Addressed the current-head blocking audit finding on |
|
Fixed in |
|
Addressed the current review blocker: source-provider CLIs now start suspended on Windows, receive the shared Job-object resource ceiling after ownership confirmation, and resume before execution; an owned live child that cannot resume is killed and fails loudly. Also removed the newly dead monitor translation key. Focused verification: pull-request monitor suite (153 passed), i18n dead-key/check gates, and production frontend build. |
bolichen97
left a comment
bolichen97
left a comment
There was a problem hiding this comment.
Review: pull-request monitors across source providers
Reviewed origin/token-monitors-babysit...origin/token-monitors-source-providers (73 files, +4763/−384) at 3be68ef00. Security was weighted heavily per the PR's own framing. Findings were verified by executing the real code — provider probe() calls through the injectable fetch hooks, run_provider_cli end-to-end against fake binaries, sandboxed_spawn_argv profile inspection, focused pytest for the slot-close leak — not by reading. 567 tests across the monitor/pod/security-posture/provider suites pass, so every item is an untested path.
First, the finding that gates the rest: finding 1 says the Azure DevOps provider cannot execute a single successful probe. Everything else about Azure below is downstream of fixing that.
Blocking
1. monitoring/azure_devops_pull_request.py:101 — --project is passed to az repos pr show and az repos pr policy list, but neither command accepts it, so every Azure DevOps probe fails at its first call. Confirmed against the real azure-devops CLI extension source (azure_devops-1.0.7): commands.py maps repos pr show → show_pull_request(id, open=False, organization=None, detect=None) and repos pr policy list → list_pr_policies(id, organization=None, detect=None, top=None, skip=None); repos/arguments.py registers no project argument for either. Azure CLI derives argv from the signature, so the captured invocation az repos pr show --id 9 --project project --organization https://dev.azure.com/acme --output json exits 2 with unrecognized arguments: --project project. classify_provider_error_text matches no marker in that stderr → TRANSIENT → executed end-to-end: probe() -> provider_error provider_transient; after max_provider_errors consecutive ticks, STOP_BLOCKED. Every Azure DevOps monitor dies without ever reading a PR. test_source_provider_pull_request_monitors.py:713 asserts "--project" in call against a mocked run_provider_cli, so it locks the invalid flag in and can never catch this.
2. monitoring/pull_request.py:301 — the checks_incomplete PENDING branch triggers on the size of a canonical bucket rather than on actual provider truncation, so a fully-green PR with >100 checks in one state can never reach review_ready. Executed: 100 passing checks → SUCCESS/review_ready; 101 → PENDING/checks_incomplete; 150 → same. Two concrete triggers. (a) Azure merges two independently-100-bounded collections into one bucket, so 60 passing statuses + 60 approved policies — complete data, nothing truncated — yields pending/checks_incomplete forever. (b) A GitHub regression: gh pr view --json statusCheckRollup returns all check runs and _normalize_checks caps nothing; I ran the base branch's deleted _classify_response on 150 passing checks → SUCCESS/review_ready, versus the new code → PENDING/checks_incomplete. Any monorepo PR with >100 green matrix checks that previously terminated successfully now probes until its runtime budget expires and the session is never told the PR is ready. The RFC's stated intent is to refuse review-ready "from a TRUNCATED set" — here the set is complete. The guard also reads post-truncation data at the boundary where truncation happens, so it needs >100 while providers deliver exactly 100: it is unreachable for Bitbucket and GitLab, and GitHub emits no truncation sentinel at all.
3. monitoring/gitlab_merge_request.py:213 — when the project-level pipelines?sha= query returns nothing, checks ends up empty and the shared classifier returns the terminal SUCCESS/review_ready despite failing CI. Executed via the injectable fetch: pipelines=[] → SUCCESS review_ready with all four check buckets empty; a single FAILED pipeline whose sha differs from mr.sha hits the continue and, because of the break, also yields SUCCESS review_ready. _fetch_with_glab queries projects/{target_project}/pipelines?sha=…, but for a fork-based MR the head commit's pipeline lives in the fork's project, so the target project returns []; merged-results/detached MR pipelines attach to the merge-result sha, not mr.sha; and there is a seconds-to-minutes window after a push before any pipeline exists. Unlike Azure (statuses:incomplete) and Bitbucket (statuses:incomplete), GitLab appends no unknown/incomplete sentinel, so "CI not observable" is indistinguishable from "CI passed". SUCCESS is terminal via decision.py:54 → STOP_SUCCESS, so the monitor stops and tells the agent the MR is review-ready with red CI. GitLab returns head_pipeline on the MR object itself — the authoritative answer — and it is ignored.
4. monitoring/azure_devops_pull_request.py:234 — review_threads_complete=len(threads) < 100 (and len(statuses) >= 100 at :190, len(policies) >= 100 at :200) uses a raw count as a pagination proxy, but _fetch_with_az sends no paging parameters at all. The response is the complete set, so treating len >= 100 as "there might be more" reports a complete read as truncated. Executed: 99 CLOSED threads + green checks + required vote=10 + mergeStatus=succeeded → success/review_ready; the same PR with 100 CLOSED threads → pending/review_threads_incomplete, forever, so it never reaches STOP_SUCCESS and silently burns its whole max_runtime_secs (traced t=300 record_only … t=14400 stop_budget runtime_budget). Azure auto-creates a system thread for every push/vote/reviewer/policy event, so ≥100 threads is routine. Worse: with 150 threads whose only active thread is at index 120, unresolved=0 (the sum only scans threads[:100]), so the monitor never goes ACTIONABLE and never wakes the session for a real blocker. GitLab (a genuine 2-page read) and Bitbucket (not bool(page.get("next"))) both use real pagination signals; Azure is the only adapter faking one. :190/:200 additionally have an off-by-one against statuses[:100] — >= 100 where the slice keeps 100 — so exactly 100 green statuses is stamped unknown → PENDING/checks_unknown permanently (executed: 100 → pending, 99 → review_ready).
5. monitoring/provider_cli.py:143 — extra_visible_dirs = (base_env["AZURE_CONFIG_DIR"],) feeds an unvalidated, environment-supplied path into the sandbox, and the sandbox implements that by removing the deny rule for that exact path. AZURE_CONFIG_DIR is in _PASSTHROUGH["az"] and provider_cli_env accepts it from credentials (which win over os.environ) with no absoluteness and no containment check. Executed against the real sandboxed_spawn_argv: the macOS profile is (allow default) plus explicit denies, and per-directory diffing of the generated .sb shows AZURE_CONFIG_DIR=$HOME/.gnupg removes (deny file-read* (subpath "/Users/…/.gnupg")); same for ~/.kiro/crew/.vault (the gateway's own auth-staging deny), ~/.config/gcloud, ~/.docker. A relative value is accepted end-to-end too. This is reachable from a .env line, not just the process env: KiroCrewConfig.load_credentials() — which this provider calls on every fetch — does os.environ.setdefault for every .env entry including unrecognised keys, and the loader only warns about unknown keys ("it will propagate to child processes"). docs/system-specs/modules/security.md claims the process "explicitly receives and exposes only its resolved AZURE_CONFIG_DIR"; the code should pin it to ~/.azure (or the pod home) before handing it to the sandbox, and require os.path.isabs + containment.
6. monitoring/provider_cli.py:190 — timeout bounds only proc.wait(); future.result() and ThreadPoolExecutor.__exit__ are unbounded joins, so one surviving pipe holder wedges the monitor permanently. The raise fires inside the with, so __exit__ runs shutdown(wait=True) — unbounded, uninterruptible — on two threads blocked in stream.read(); line 188's bare proc.wait() is a second unbounded wait. proc.pid is the sandbox wrapper, not az/glab, and _kill_provider_tree returns early when proc.poll() is not None, so it kills nothing once the direct child has exited; the os.killpg fallback is escaped by any descendant that setsid()s. Executed with the structure copied 1:1 from run_provider_cli and timeout=2.0: STILL RUNNING after 25s — the TimeoutExpired cannot even escape the block. Concrete trigger: provider_cli_env does not set AZURE_CORE_COLLECT_TELEMETRY=0, which the repo's own Azure client does (issue_radar/backend/azure_client.py:478), so az forks a detached telemetry uploader that inherits the pipes. Amplification: controller.py:176-177 holds self._provider_gate (asyncio.Semaphore(MAX_MONITOR_PROVIDER_CONCURRENCY = 4)) across an uncancellable asyncio.to_thread, so four such probes permanently exhaust the semaphore and every structured monitor across all providers stops probing, silently, with no timeout to recover.
7. pod/runtime.py:1756 — build_pod_env redirects GLAB_CONFIG_DIR and AZURE_CONFIG_DIR beneath the pod home but leaves GH_CONFIG_DIR untouched, so gh inside an isolated pod still reuses the operator's persisted GitHub login through the deliberately inherited real HOME. The function's own new docstring states config roots are redirected "so glab and az cannot reuse the operator's persisted login sessions through the deliberately inherited real HOME" — but github_runner.py:152 shows GH_CONFIG_DIR is a recognised gh auth root, and grepping src/ finds it set nowhere. A pod running a github_pull_request monitor (the default provider) reads $HOME/.config/gh/hosts.yml and acts with the operator's full GitHub token, and pod down cannot reclaim that store. The generic _TOKEN-suffix scrub removes GH_TOKEN/GITHUB_TOKEN from the env but not the on-disk config the CLI falls back to. Related, same function: the new "complete credential roster" loop (:1809) misses the JIRA_TOKEN_<HEX> family (loader._JIRA_TOKEN_RE), which sandbox._AGENT_DENIED_ENV_KEYS covers by prefix and the _TOKEN-suffix rule misses; verified JIRA_TOKEN_6162 -> PRESENT in a pod env built from a polluted environ.
8. monitoring/azure_devops_pull_request.py:86 and monitoring/bitbucket_pull_request.py:129 — KiroCrewConfig.load().load_credentials() runs once per resource (4× per probe) on a worker thread, and it mutates process-global os.environ. This is the first monitor code to call it — the base branch's github_pull_request.py does not. load_credentials() stats ~/.kiro/crew/.env, chmods it, parses it, and ends with os.environ.setdefault(k, v) for every entry. Measured: KiroCrewConfig.load() = 0.827 ms even warm-cached (it still stats, deep-copies, rebuilds the dataclass tree and runs three publish_* side effects incl. drawing a config-load ordering ticket) and load_credentials() = 0.529 ms and is not cached at all → 1.444 ms/fetch, 5.78 ms/probe, 4 plaintext reads of the credential file per probe; at MIN_MONITOR_CADENCE_SECS=15 that is 16 .env reads and 16 ordering tickets per minute per monitor. Two consequences beyond cost: (a) executed before/after dumps show a single Azure PR probe pushing SLACK_BOT_TOKEN, JIRA_API_TOKEN, JIRA_TOKEN_<HEX> and KIRO_API_KEY into the gateway's os.environ — and therefore into every child spawned with a raw os.environ copy — purely as a side effect of a read-only probe, defeating an operator who deliberately keeps tokens in .env and out of the environment; (b) four monitors ticking together setdefault concurrently while the loop does os.environ.copy(), risking RuntimeError: dictionary changed size during iteration. GitHubPullRequestProvider.probe already demonstrates the fix: resolve once per probe and thread the result down.
Should fix
9. monitoring/gitlab_merge_request.py:208 (also azure:188, bitbucket:191) — check identities are digests of per-run provider ids rather than of the stable name/key/context sitting in the same dict, so a CI re-run of the same failing check on an unchanged head mints a new actionable fingerprint and buys another unattended agent turn. Executed: same MR, same head sha, same failing pipeline re-run (id 1001 → 1002) produces fingerprints c8a54901… vs 412c52b9…, both checks_failed; the identical GitHub case (stable check name CI / build) keeps one fingerprint. Traced to the budget: retry 0..7 → wake_actionable, agent_turns 1..8/8, retry 8 → stop_budget. Eight retries of one flaky job exhaust the entire agent-turn budget and kill the monitor, delivering zero new information. Azure is worse because it appends statuses rather than replacing them, so the failed bucket grows (['status:13b7…'] → ['status:13b7…','status:e609…']). This defeats actionable_fingerprint_facts's documented purpose ("Keep known blockers stable") and the RFC's "wakes the owning session at most once" claim, in a token-efficiency feature. Bitbucket is safe today only because its build statuses carry no uuid so it falls through to the stable key. Related (pull_request.py:222): identities are sha256 prefixes, so the [:100] truncation keeps "first 100 alphabetically" — a pseudorandom subset — and the checks:incomplete overflow marker is excluded from the actionable fingerprint entirely, so with 150 failing checks both fixing and adding a failure outside the top 100 leave the fingerprint unchanged (fingerprint changed = False) → NO_CHANGE → the session is never told the failure set moved.
10. monitoring/gitlab_merge_request.py:61 / :237, bitbucket_pull_request.py:177, azure:229 — str(x.get(k, "")) and _object(x.get(k, {})) do not defend against an explicit JSON null, because .get returns None when the key exists. Executed: an open MR payload {"state":"opened","sha":null} produces str(None) → the literal "None", which is sent to GitLab as the pipeline sha=None filter and then rejected by the new _HEAD_REVISION_RE → provider_error/provider_malformed_response (TRANSIENT) → three probes → STOP_BLOCKED. GitLab returns "sha": null while an MR is preparing and after the source branch is deleted; Bitbucket returns a null source.commit once the source branch is gone (_object(None) raises). The shared contract's intended answer is PENDING/pull_request_state_unknown — retry until the provider settles — and classify_pull_request_facts has that exact branch, but GitLab can never reach it because _fetch_with_glab raises on a falsy head_revision before facts are built. So both shapes of a missing GitLab sha terminate the monitor.
11. monitoring/provider_cli.py:35 — _NETWORK_ENV omits ALL_PROXY/all_proxy/REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE and _PASSTHROUGH["az"] omits AZURE_EXTENSION_DIR, all five of which the pre-existing allowlist for the same az binary includes. Executed end-to-end: with all three set in the parent, the child spawn_env printed ALL_PROXY=None REQUESTS_CA_BUNDLE=None AZURE_EXTENSION_DIR=None. issue_radar/backend/azure_client.py::_AZ_ENV_PASSTHROUGH lists all five and documents AZURE_EXTENSION_DIR as where the az login session lives. Consequences: behind a SOCKS-only or TLS-inspecting corporate proxy every Azure probe fails to connect/verify (az is a requests-based Python CLI) → provider_transient → monitor stops; and with a custom AZURE_EXTENSION_DIR plus the transport's own AZURE_EXTENSION_USE_DYNAMIC_INSTALL=no, az devops is not found → non-retryable provider_setup → immediate STOP_BLOCKED. This is the concrete cost of provider_cli.py being a third parallel provider-CLI env allowlist (after github_runner.GH_ENV_PASSTHROUGH, which calls itself "the canonical union for every gh spawn path", and source_providers._PROVIDER_*_ENV_KEYS); _PASSTHROUGH["glab"] is byte-identical to source_providers.py:152 and _AMBIENT_IDENTITY_KEYS is the third verbatim copy of the same frozenset. Also in the same area: the GitLab scrub sentinel is expressed as credentials={"GITLAB_TOKEN": ""}, which the truthiness filter at :98 discards — the scrub holds today only because apps/registry._SAFE_ENV_KEYS happens not to copy GITLAB_TOKEN, and the fixup loop at :152 would then restore it, inverting an explicit "scrub this" into "forward it" and handing a gitlab.com PAT to a self-managed host.
12. monitoring/gitlab_merge_request.py:87 — GitLabHostNotAllowed subclasses ValueError, so a host the operator has revoked from dashboard.gitlab_hosts is reported as a retryable provider_malformed_response/TRANSIENT instead of a terminal policy refusal. Executed with a fetch that asserts it is never called: probing https://git.corp:8443/g/p/-/merge_requests/1 with an empty allowlist returns provider_error | provider_malformed_response | TRANSIENT, and TRANSIENT in _RETRYABLE_PROVIDER_ERRORS. security.md advertises that GitLab "rechecks that allowlist on each probe" — the recheck happens, but its denial is mislabelled and retried for the full max_provider_errors budget while the dashboard reports a revocation as a malformed provider response. Compounding: _gitlab_hosts() reads KiroCrewConfig.load() fresh on every probe while create/update read the TTL-cached snapshot, so the two disagree by up to one TTL and a just-revoked host is accepted by the API and then fails every probe.
13. monitoring/github_pull_request.py:167 — build_pull_request_probe_result(PullRequestFacts(...)) sits outside probe()'s try/except, so the field validation this PR moved into PullRequestFacts.__post_init__ escapes uncaught. Executed with headRefOid="not-a-sha-zz": RAISED OUT OF probe(): ValueError: head_revision must be bounded hexadecimal text. It reaches controller.py's except Exception, which calls logger.exception("structured monitor provider raised unexpectedly") — a full stack trace on every cadence tick — and records provider_transient, so last_observation_reason_code misattributes a malformed response as a transient network fault. GitLab, Azure and Bitbucket all build their facts inside their try blocks; the refactor left GitHub as the only unprotected one. Same asymmetry class: GitHubCheck = PullRequestCheck silently added raise-on-invalid to every existing GitHub construction site, and _sanitize_check_identity does not strip whitespace-only names, so one status context named " " makes _normalize_checks raise → the whole PR probe returns provider_malformed_response → monitor terminated after max_provider_errors. Pre-PR that PR monitored fine.
14. monitoring/gitlab_merge_request.py:230 — GitLab counts unresolved notes while Azure and Bitbucket count unresolved threads/tasks, so the shared unresolved_review_threads field means different things per provider. Executed side by side: one unresolved GitLab discussion containing 5 resolvable notes → unresolved_review_threads = 5; one active Azure thread with 5 comments → 1; one unresolved Bitbucket task → 1. GitLab marks resolved per note within a discussion, so the loop inflates by the reply count. The value is persisted in the canonical observation, rendered in the dashboard, and surfaced in monitor_inspect, so a GitLab MR with one open thread reports "5 unresolved review threads" — 5× the same logical state on any other provider, breaking the provider-neutral contract this PR is built around. Also :224: complete = len(discussions) < _MAX_PROVIDER_ITEMS cannot distinguish "exactly two full pages" from "hit the cap", so exactly 200 discussions pins PENDING/review_threads_incomplete forever (executed: 200 → pending, 199 → review_ready).
15. dashboard/chat_handlers.py:3761 — the new slot._closing fence latches permanently on any abort it does not name. It is set to True and reset by six hand-placed slot._closing = False statements rather than a try/finally, and _retire_slot_nudge_loop deliberately converts only Exception, so asyncio.CancelledError propagates raw. Verified with a focused pytest using the repo's own harness: parking inside _retire_slot_nudge_loop and cancelling api_chat_slot_delete (exactly what aiohttp does on browser disconnect) leaves state._slots[NAME] is slot True and slot._closing True — LEAK: live slot left with _closing=True -> never armable again. autonudge_authz._dashboard_admission:486 returns False whenever _closing is set, so from then on every authorize_and_add_nudge for that visible, usable tab is refused with 409 "session changed before nudge arm committed", silently, for the life of the process, with no UI explanation and no recovery except closing the tab — the operation that broke it. Nothing anywhere in src/ ever writes _closing = False outside those six sites. (Note this and the replace_terminal_id 409 are both concurrency-hardening changes riding inside a PR scoped as "extend PR monitoring to three providers"; replace_terminal_id exists nowhere on origin/main or the base branch, in no frontend file and no doc, so the PR introduces a request field solely to reject it, plus ~50 lines of tests for the dead branch, with no TODO or tracking issue.)
Notes, lower severity
dashboard/handlers/autonudge.py:236 — the REST monitor create/update path applies no length bound to target, so an arbitrarily long canonical URL is persisted and interpolated into every wake envelope; executed, _monitor_config accepted a 12,242-char gitlab.com URL, bypassing the MAX_SHORT_STRING = 500 cap the MCP path enforces. · dashboard/session_directive_apply.py:274 — the comment added at control.py:1192 ("The authoritative applier validates the target against the retained kind") is true for update but _monitor_watch takes kind/target verbatim, so monitor_watch's only validation lives in the MCP subprocess; a GitLab host outside the allowlist is persisted and armed, then fails closed only at probe time as a stream of mislabelled errors. That path also now hard-depends on KiroCrewConfig.load() unconditionally, so a config parse error breaks monitor_watch for a plain GitHub PR, which needed no config on the base branch. · monitoring/models.py:333 — kind is never validated against PULL_REQUEST_MONITOR_KINDS, and controller.py:166 maps an unknown kind to ProviderErrorKind.SETUP, which has no special handling, so a quarantined (invalid_kind) or downgraded record probes provider_unsupported once per cadence for 20 cycles before stopping, while monitor_state_public_dict silently returns last_observation: {}. MONITOR_STATE_VERSION also stayed at 1 while the persisted kind vocabulary went 1→4, so a rollback hands a GitLab URL to GitHubPullRequestProvider and terminates with a false provider_transient diagnosis instead of the honest unsupported_monitor_version the mechanism exists to produce. · handlers/autonudge.py:544's new SESSION_CLOSE restart 409 has no frontend counterpart — automation.ts:383 sets actionable: stopped_reason !== 'invalid_monitor_record', so Restart renders enabled for a session-close record, 409s, and shows the generic "request failed"; with create refused (replace_existing=False), PATCH 404ing and no DELETE route, the slot is a dead end. · monitoring/models.py:555 — the public projection's new per-identity length and per-bucket count caps are applied to already-persisted data with no migration, so a pre-upgrade snapshot with a 255-char check name projects last_observation = {} (executed) and the dashboard shows "no observation" for a healthy monitor until its next probe. · pull_request.py:96 — PullRequestCheck.__post_init__'s new 200-char truncation changes GitHub fingerprints across the upgrade (executed: old_fp 203acd7f… → new_fp 009e474c…), so the first post-upgrade probe of a monitor sitting on checks_failed with a long reusable-workflow name charges a real agent turn for a PR that did not change. · monitoring/shadow.py:49 still hard-gates kind != "github_pull_request" and names GitHubShadowProvider/GitHubPullRequestProbeResult; it has no callers in src/, so this is drift rather than a live break, but it raises out of an async def with no containing try. · controller.py:117 — providers={} silently means "all four live providers" (the None sentinel already exists), so a caller computing an empty map to mean "monitor nothing" gets real CLIs and network calls. · targets.py:14 — _AZURE_SEGMENT_RE permits a leading -, so a target URL mints option-shaped argv (az repos pr show --id 5 --project --help, executed); = is correctly blocked so no route-parameter injection, making this robustness rather than escalation. · targets.py:33 — parsed.port is a property that raises, uncaught, and autonudge.py:395 puts str(exc) in the 400 body, so urlparse's internals (Port out of range 0-65535) reach the client; the pre-existing GitHub parser does wrap this and the new _parts dropped the guard. · targets.py:34 — the scheme-default port is not normalized away, so :443 on an allowlisted host is refused as gitlab_host_not_allowed, sending the operator to edit an allowlist that already contains the host. · targets.py:151 — len(parts) != 6 is re-tested after line 148 already returned; the sibling Bitbucket parser writes the same guard correctly. · targets.py:114 — _positive_id has no upper bound, so behaviour differs across the declared 3.10–3.13 range (CPython's int-digit limit arrived in 3.11): a 4300-digit iid is accepted on 3.11+ and unbounded on 3.10, and _monitor_config sets no max_len on target. · website/src/monitoring/automation.ts:54 — the GitLab branch alone has neither a hostname check nor the !url.port check its three siblings have, so https://dev.azure.com/org/proj/-/merge_requests/1 and https://attacker.example/a/b/-/merge_requests/1 are client-side classified gitlab_merge_request (executed) and the create POST then fails with gitlab_host_not_allowed, telling the user their Azure URL is a disallowed GitLab host. More broadly the frontend silently repairs six URL shapes the backend rejects byte-exactly (/files, /diffs, /diff, ?query, #fragment, trailing /), and parseSourceLinkUrl in utils/pullRequestLinks.ts:518 already exists and is exported for exactly this caller shape. · pull_request.py:266 weakened canonical["checks"]-style indexing to .get() throughout, so a malformed canonical now yields a confident fingerprint over None values (failed_checks: null ≠ [], a different fingerprint for the same semantics) instead of failing closed. · Reuse: _object is declared byte-identically in all three new provider modules while pull_request.py — which all three already import ten symbols from — is the obvious home; bitbucket_pull_request.py:144 is the tenth hand-rolled pinned-redirect + byte-cap urllib client in src/kiro_crew and rebuilds its opener on every fetch; provider_cli.py:263/bitbucket:247 are the 4th and 5th copies of the SEL audit wrapper and both dropped the "best-effort but never silent" logging half of github_runner._audit_run's contract; provider_cli.py:128 re-resolves, re-validates and re-sandboxes the binary and spins a fresh thread pool per invocation (measured ~18.6 ms + 4 temp-file write/unlink cycles + 8 OS threads per probe, with agent_writable_roots() uncached at 0.33 ms per validation); and pull_request.py:54's shared classify_provider_error_text has a divergent marker set from github_pull_request.py:477, producing opposite lifecycles on the same stderr (Could not resolve to a PullRequest… → GitHub stops, shared retries; gh auth login → GitHub stops, shared retries; does not exist → shared stops, GitHub retries), so GitHub cannot migrate onto it as-is. · Dead/duplicated code: the if "?" in collection ternary at gitlab:125 is unreachable and written twice; azure:178's elif is identical to its else; literal 100 appears eleven times where MAX_MONITOR_CHECK_IDENTITIES_PER_BUCKET is the actual contract (and models.py:558 now rejects the whole observation above it, so lowering the constant silently blanks Azure's checks); the four check states are defined three times, two added here. · Conventions: docs/request-for-change/rfc-token-efficient-monitors.md still says status: draft ("Nothing built") and implementation-prs: [] while its own body now describes eight PRs, audited-at: 6ce882d5b names a commit reachable from no ref (verified — it is a pre-rebase copy of 94f4c00df, and it regressed from the base branch's resolvable 6d3e30bbbd), and docs/request-for-change/README.md is not in the diff at all so its index row still reads "draft | Nothing." The directory's own rule: "When you land an implementation PR for anything here, update the document's status, implementation-prs, last-audited and audited-at in the same PR", and "When a document and the code disagree, the code wins and the document is a bug."
Execution-verified AI-assisted review (Claude Code) across 10 parallel angles plus a gap sweep, run against a local checkout at 3be68ef00; the working tree was left clean and nothing was posted elsewhere. Verified clean, so please don't re-spend on them: the strict target parsers in targets.py (%2F/%2E////../trailing-dot/uppercase-host/credential-shaped-path all rejected; _positive_id rejects 01, +1, whitespace, full-width digits, 1_0; self-managed host:port round-trips and a portless allowlist entry does not authorize an arbitrary port); per-provider credential scoping (SLACK_BOT_TOKEN/KIRO_API_KEY provably do not reach the az child, GITLAB_TOKEN is provably dropped for self-managed hosts and kept for gitlab.com, no credential appears in argv); Bitbucket redirect pinning end-to-end (the custom handler replaces rather than augments the default; http/off-host/port/userinfo/protocol-relative targets all rejected); _read_limited has no off-by-one and no reader deadlock; POSIX creationflags == 0 and preexec_fn correctly None with no Python work in a fork child; monitor_budget_reason's removal from controller.tick is a safe dedup; apply_monitor_probe preserves last_observation across provider errors; all 13 locales plus en.context.json have real translations for the 4 new keys; the three new credential keys are correctly registered in all four places the sync tests pin; and both NON_EGRESS_REDACTION_MODULES additions are consistent with the documented contract. Semgrep: 11 rules, 15 files, 0 findings. Findings name the input that reproduces them — please push back where one misreads intent.
Addendum — three items my review above missedA final verification pass turned up three findings not covered in my earlier review. All three were reproduced by executing the real code. 16. 17. 18. Same methodology as the review above: executed against a local checkout at |
|
Addressed all three addendum findings in 12df3ae: Azure target parsing now accepts provider-legal Unicode/punctuation while rejecting forbidden characters and returns the stable invalid_pull_request_url API code; explicit-null Azure source commits remain pending; and invalid GitHub check labels degrade to a stable unknown check instead of failing the probe. Regression verification: 223 touched backend tests and 41 component tests passed; Black baseline, isort, flake8, Linux mypy, docs lint, ESLint, TypeScript, diff-scoped i18n, and diff checks passed. The two-commit #5305 shape and direct #5186 base are preserved; the PR description screenshot SHA was updated. |
|
Addressed the current source-provider review set and restacked the top in c8f1b53. The fixes cover provider-specific pagination and current-head completeness, bounded CLI/process teardown, credential and config-directory containment, stable check identity, GitLab discussion semantics, revoked-host handling, slot-close cancellation fencing, and dashboard tombstone actionability. Verification: 234 composed backend tests, Linux-targeted mypy, flake8, docs lint, frontend lint/typecheck, and the full 27,686-test frontend run all pass. |
|
Addressed the current GPT and CI failures in 65fad4b. Busy wake retries now stop on exhausted budgets before dispatch, stale BUSY acceptances are cleared, the multi-provider controller test uses the provider map, public-observation fixtures include completeness, and Azure CLI tests no longer inherit host config paths while validation recognizes both HOME and the platform user home. Verification: 300 composed backend tests, Linux mypy, flake8, docs lint, frontend lint/typecheck, Black, brand, and harness gates pass. Every open stack slice is now exactly two commits. |
|
Addressed the Azure CLI blocker and the Windows shard failures in 29577f0. Azure config/extension visibility now uses HOME consistently across platforms and rejects paths equal to or beneath agent-writable project/workspace roots. Added focused coverage for both equality and descendant cases; 4 focused tests, flake8, Linux-parity mypy, Black gate, and docs lint pass. |
Audit note — this PR is one slice of a declared branch stack, not a duplicateA duplicate-detection sweep flagged #5184 / #5185 / #5186 / #5305 as overlapping at FULL coverage. That is a stacking artifact, and it is worth stating plainly so nobody acts on it: each branch physically contains the previous one, so the shared code is inherited and each PR's review diff overstates what it actually authored. Proved mechanically, not by reading code:
The two PRs are also cleanly split by surface, which is why they are complementary rather than redundant. #5184 is backend-only: 26 files under src/kiro_crew/ (monitoring/controller.py, mcp_tools/control.py, validation.py, session_directive.py, dashboard/handlers/autonudge.py, slack/gateway.py, discord/gateway.py, ...) and ZERO files under website/. #5185's own delta ( The 11 non-website files in #5185's own delta are strictly ADDITIVE refinements in service of that frontend, not a competing design: monitoring/models.py gains Neither has landed: 550ea7b, fc82d95, 4c7b130 and 331c07d are all absent from origin/main; main carries monitoring/{init,completion,decision,github_pull_request,models,shadow}.py but NOT controller.py, and website/src/monitoring/ and SessionAutomationPopover.tsx do not exist on main. Ironically the first-pass record itself, the cached PR data, states in prose "Stacked descendant (PR 6). Its branch is #5184's head plus 4c7b130 + 331c07d; the shared code is inherited, which per the brief is a stacked branch, not duplicate" — the FULL/complete_coverage=true coding of that same relation is what the scan surfaced as a duplicate signal.
Proof of stacking: #5186's own delta ( Per-file judgment of the 14 shared paths: every one is a file #5184 creates or extends and #5186 then builds further on. src/kiro_crew/monitoring/controller.py does not exist on origin/main at all (git ls-tree of the monitoring package shows only init.py, completion.py, decision.py, github_pull_request.py, models.py, shadow.py) -- #5184 creates it, #5186 adds 9 lines to it. mcp_tools/control.py: #5184 introduces the monitor_watch schema and #5186 edits the schema #5184 authored. That is a build-on-top dependency, the opposite of two implementations of one behaviour. Deciding question: if #5184 merged, would #5186 still carry real, wanted work? Yes -- the whole babysit-skill migration, the shipped monitoring doc, the unbounded-loop ban, the pre-probe budget stop, evidence_scope, and the Slack terminal notifications. The reverse direction is not a redundancy signal: #5186 "contains" #5184 only because it is branched off it.
The task premise that both are "open against main" is FACTUALLY WRONG for 5305. Its GitHub base ref is On its OWN range (7ce67a7..9a6f511) 5305 is 77 files / +5,642 / -432 — matching GitHub's own 77 / +5605 / -432 for the PR. That range EXTENDS the four files 5184 created (src/kiro_crew/monitoring/controller.py, test/test_monitor_controller.py, test/test_monitor_directive_apply.py, test/test_monitor_mcp.py); it does not reimplement any of them. It also ADDS six modules 5184 has no counterpart for at all: monitoring/pull_request.py (the provider-neutral review-readiness contract), monitoring/targets.py (strict target parsers), monitoring/gitlab_merge_request.py, monitoring/azure_devops_pull_request.py, monitoring/bitbucket_pull_request.py, monitoring/provider_cli.py — plus dashboard/handlers/source_providers.py and a whole frontend surface (SessionAutomationPopover.tsx +626, monitoring/automation.ts +455, MonitorRadar.tsx) across 12 locale catalogs. That is a capability 5184 STRUCTURALLY CANNOT DELIVER: 5184's MonitorController drives only the single GitHub probe already on main (monitoring/github_pull_request.py, landed by the stack's merged #5183 = dc35c19, which is 5184's merge base). GitLab / Azure DevOps / Bitbucket are different addressing schemes and different providers. Conversely 5184 is the substrate 5305 consumes — MonitorController, the monitor_watch/inspect/update/stop MCP tools, the owner-gated REST routes, format_monitor_wake, MonitorDispatchResult. Neither can be closed without destroying real, wanted work: closing 5184 deletes the controller 5305 imports; closing 5305 deletes four provider probes, the neutral readiness contract, and the entire multi-provider dashboard surface. The maintainer agrees in practice. bolichen97 reviewed each slice on its own per-slice range — 5184 as Consequence for reviewLand the stack in order. Reviewing a later slice before its base lands means reviewing the base's code again, and the FULL-overlap signal a keyword or file-overlap sweep produces here means nothing. Separately, #5184 vs #7634 (the zero-token-probe monitor gate) was adjudicated as independent — same monitor area, different behaviour — so neither blocks the other. From a repository-wide duplicate/overlap audit of every pull request open against |
Review — head
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
GPT 5.6 Review —
|
|
Addressed the linked-channel credential-boundary finding in 591aea1. Monitor creation provenance is now persisted independently of the slot binding; Slack-linked immediate, queued, and recovery turns retain channel provenance, and legacy unknown provenance fails closed for Azure DevOps and Bitbucket owner credentials. Focused monitor/provider and messaging/directive/queue suites pass locally. |
|
Addressed the current-head mixed-queue credential-boundary finding in 8bce380. Channel provenance now wins for an entire merged queue batch when any consumed entry came from a channel, so adjacent dashboard input cannot widen the turn to dashboard-owner Azure or Bitbucket credentials. The regression failed before the change and passes after it; 83 focused queue/directive tests plus targeted Black, flake8, Linux mypy, docs lint, subprocess-encoding, brand, and diff checks pass. The top remains two commits directly above #5186. |
|
Fixed the fresh CI contract failures in c0dd854: queue-dispatch expectations now preserve channel provenance, the monitor-start test supplies its producer surface, and the workflow nudge fake accepts the persisted creation surface. Focused verification passes all 351 tests in the five affected files both serially and with the repository xdist configuration; formatting, flake8, Linux mypy, docs, subprocess-encoding, brand, and diff gates also pass. The leased force-push has started a fresh CI run. |
|
Addressed the current Opus credential-boundary finding in 0c8b2de. A channel-origin monitor_update that changes the target now atomically ratchets a dashboard-created monitor to the channel creation surface, preventing the new Azure DevOps or Bitbucket target from inheriting dashboard-only owner credentials. The regression failed on the prior head and passes with the fix; the composed 1,265-test focused suite passes with 2 platform skips, along with focused Black, isort, flake8, Linux-targeted mypy, docs, subprocess-encoding, and brand gates. The open tail was restacked bottom-up onto current main with explicit leases; no CI job was manually rerun. |
3 participants
Problem / Motivation
Structured babysit monitoring currently understands GitHub pull requests only. GitLab merge requests, Azure DevOps pull requests, and Bitbucket pull requests must fall back to prompt-owned polling, which spends agent turns even when review state is unchanged.
Why it matters
Teams using another source provider should get the same bounded, restart-safe, zero-turn monitoring behavior without granting an agent broad credentials or trusting provider payloads as instructions.
What changed (motivation → approach → change)
Define one provider-neutral review-readiness contract, then normalize GitHub, GitLab, Azure DevOps Services, and Bitbucket Cloud into that bounded state.
Add strict target parsers that reject credential-shaped canonical path text before persistence, plus provider-specific probes with constrained credentials, streaming output limits, bounded pagination, canonical output, and fail-closed incomplete-state handling.
Extend the monitor tool schema, persistence, public projection, REST flow, dashboard normalization, and babysit guidance to the new source-provider kinds.
Re-audit the monitor RFC against all eight implementation PRs and mark the still-open stack as in progress rather than draft.
Support GitLab.com and explicitly configured self-managed GitLab hosts, including custom ports; support Azure DevOps Services at
dev.azure.comand Bitbucket Cloud atbitbucket.org.Normalize common provider URL suffixes plus copied-link query strings and fragments in the dashboard, distinguish empty and invalid URLs from provider-changing edits, and render GitLab allowlist guidance pointing to config.json only for the backend-specific error code.
Keep retained terminal records read-only with Restart as the sole dashboard mutation; reject the retired replacement field until bulk slot cleanup fences every slot before awaiting.
Keep sparse non-target edits independent of target parsing so older malformed persisted targets do not crash unrelated saves.
Keep unchanged probes at zero agent turns and deduplicate actionable wakes by a stable provider-neutral fingerprint.
Retain the finite legacy path when readiness depends on generic PR comments or advisory findings outside canonical review/check facts.
Read only the newest current-head GitLab pipeline so a successful retry supersedes an older failed run, and map GitLab requested changes to the shared actionable review decision.
Apply provider CLI resource limits after exec through the repository spawn shim, avoiding Python work in a threaded fork child.
Confine host Azure CLI configuration and extension visibility to protected
HOME/.azure; pod mode accepts only its credential-scrubbed disposable home.Scrub Azure DevOps and Bitbucket credentials from isolated pod environments while retaining only the pod model and owner credentials.
Keep startup warnings credential-safe by reporting missing source-provider configuration without enumerating credential key identifiers in logs.
Inherit feat: show bounded monitors in the dashboard #5185's shared ErrorNotice handling for monitor failures, preserving unsaved drafts and provider-specific guidance without navigating away.
Correct the RFC to distinguish transient probe summaries from persisted classification/reason fields.
Preserve Bitbucket status-page completeness in canonical evidence independently of task pagination; incomplete statuses stay pending through the shared readiness contract.
Inherit feat: show bounded monitors in the dashboard #5185's guarded terminal reconnect-cache refresh, including older cached absence and predecessor monitor/legacy IDs, so stopped monitors remain inspectable after reconnect.
Render terminal notifications from the stored stop reason: state closed-unmerged PRs directly, distinguish merged from ready-for-review success, name known provider/delivery blockers, and identify the watched PR URL even for channel-bound watches. Unknown reasons retain a safe details fallback; unavailable conversations point to a new watch from an active conversation.
Audit revoked GitLab hosts and incomplete Bitbucket credentials as credential-free denials before any provider request; a failed audit writer never permits the denied request.
Correct the RFC: dashboard creation cannot replace retained records, and explicit restart checks the terminal record identity and configuration generation.
Tests
Previous composed-stack verification passed 145 focused frontend tests, TypeScript, focused ESLint, production analyze build, and the 810-chunk bundle gate. Those frontend files are unchanged by this notification-copy follow-up.
A separate, earlier cron-dedup Windows failure (not the latest notification regression) was classified as pre-existing: the cron dedup test and relevant production files are unchanged from main, and a fixed-clock local diagnostic reproduces the identical
1 != 2failure. No CI rerun or unrelated cron change was made.Previous composed-stack verification: 120 focused backend tests pass (72 provider transport/semantics tests, 16 shared-readiness tests, and 32 notification/observer tests). The notification fix is owned by #5186: it uses the recorded stop reason, distinguishes merged from review-ready success, and includes the target PR URL. Denial regressions still exercise real SEL records and unavailable writers without permitting provider requests. Focused Linux-platform mypy, Black, flake8, isort, docs lint, brand, and diff checks pass. GPT and Opus found no blockers on the preceding audit-fix head; fresh CI and review verdicts are required after this restack. No manual CI rerun was made.
Previous import follow-up: inherit #5186's terminal-notification redaction fix and preserve its ungated finite fallback recipe. Restore lazy target-parser and exception imports in dashboard monitor mutation paths: module-scope target imports eagerly loaded the GitHub provider even with monitoring disabled. The existing cold-gateway regression reproduced locally and failed on both Windows and Linux CI, then passed with this correction. All 187 focused backend tests pass across gateway initialization, notification redaction, babysit, tool acknowledgement, source-provider probes, directive application, and dashboard handlers. Linux-targeted handler mypy, Black, isort, flake8, docs lint, brand, and diff checks pass. Screenshot bytes are unchanged. Fresh CI and reviews are pending on
ec16b57f7a414fb4dca6d91c60b852697584e472; no manual CI rerun was made.The preceding head's Linux backend-shard failure was classified as a pre-existing Job SDK test race:
test_live_while_a_registered_thread_runswaits for the durable terminal record, then checks the live table before the worker necessarily removes its entry. The test andapps/job_sdk.pyare byte-identical to the local main snapshot; the coverage gate failed downstream because that shard failed. No unrelated Job SDK change or manual CI rerun was made.Latest main-restack verification on
00710aaa538dbb4d4df38c550213352ee044f7a2: all 229 focused backend tests and 145 focused frontend tests passed, along with TypeScript compilation, docs lint, Black and flake8 on the resolved handler, and diff checks. The close-slot conflict preservesmain's replacement-slot safety and this layer's monitor-admission fence. Screenshot bytes are unchanged. Fresh CI and reviews are pending; no manual CI rerun was made.Manual verification
Rendered the dashboard automation popover locally and verified that the pull-request field identifies GitHub.com, GitLab, Azure DevOps Services, and Bitbucket Cloud.
Screenshots / video
The immutable screenshot illustrates the normal form, not the error-notice states covered by the component tests.
Related Issues
Part 8 of GitHub stack #5275. Stacked on #5186 and extends the monitor RFC introduced by #5180.
Checklist
Contribution License Agreement
N/A — the repository template does not yet supply final CLA wording.
Current rebuild verification on
fa8c67e2c: rebuild the two source-provider commits from their clean pre-restack versions on corrected #5186, restoring the full dashboard handler tail and preserving the close-slot monitor-admission fence. All 680 focused backend tests pass; Linux-targeted handler mypy, Black, and flake8 pass. The two focused frontend files pass all 122 tests and TypeScript compilation passes. The inherited #5186 verification also passes all 300 focused tests plus docs lint. Screenshot bytes are unchanged. Fresh CI and reviews are pending; no manual CI rerun was made.Current main-restack verification on
7352b2932fe0e0f978a3d8a765debd333abb5137: this top layer is one commit directly based on #5186 atc0056fdc0b4457f148876457c528985aeae0b1d9, preserving the eight-PR linear stack. The CodeQL credential-identifier logging finding is fixed and its regression coverage passes. The composed stack passes all 2,430 changed backend tests and all 101 focused sidebar/popover frontend tests; TypeScript compilation and the production frontend build also pass. Screenshot bytes and its immutable URL are unchanged. Fresh CI and reviews are running; no manual CI rerun was made.Current security/restack verification on
2870dc748af24eb9931bf545a1d984454c33e614: this top layer is directly based on #5186 at3b5f883a3721d35179b1b7d7caeb14c8df6dd317. Channel-bound Bitbucket probes never load gateway-owner credentials and use anonymous HTTPS; channel-bound Azure probes fail closed without reading the operator credential store or Azure CLI login state. Dashboard-bound credential behavior is preserved. The four new regressions pass within all 446 focused controller, provider, AutoNudge, and Slack gateway tests; focused Linux mypy, flake8, Black, and docs lint pass. Screenshot bytes and its immutable URL are unchanged. Fresh CI and reviews are running; no manual CI rerun was made.Current credential-contract verification on
2ec2f953823b0c070b3a4a263b591dca55c49cf9: every source-provider probe now receives explicit owner-credential authority, and unlisted channel providers fail closed by default. GitHub and host-authorized GitLab retain their documented ambient CLI identity; Azure records a credential-free SEL denial before returning authorization failure, and Bitbucket remains anonymous in channels. All 277 focused provider, controller, and babysit tests pass; focused Linux mypy, flake8, Black, docs lint, and diff checks pass. Screenshot bytes and its immutable URL are unchanged. Fresh CI and reviews are running; no manual CI rerun was made.Current rollback-fence verification on
2ec2f953823b0c070b3a4a263b591dca55c49cf9: failed slot closes retain the monitor-admission fence through every awaited rollback and release it only in the outer close finalizer, preventing a concurrent monitor from being overwritten by restoration. The new app-close regression failed before the fix and passes afterward; all 37 focused slot-close and monitor-directive tests pass, with focused Black, isort, flake8, Linux mypy, and diff checks clean. Screenshot bytes are unchanged. Fresh CI and reviews are running; no manual CI rerun was made.Current main-restack verification on
c53426d879658ff1bc6e0ccf03eaeebe29878399: this two-commit top layer is directly based on #5186 atb2e22498966e2c2aa4f016e85426a5facb616c6f. The source-provider conflict preserves main’s tagged-refusal rule while normalizing GitHub, GitLab, Azure DevOps, and Bitbucket targets before monitor creation; invalid kind/target pairs return a refusal without publishing a directive. All 373 focused backend tests and all 218 focused frontend tests pass, along with TypeScript compilation, focused Black, and flake8. Screenshot bytes and its immutable URL are unchanged. Fresh CI and reviews are running; no manual CI rerun was made.Current review-fix/restack verification on
950a2a9da33e0db15bd537f727c5879faa05a7ad: this two-commit top layer is directly based on #5186 ate8ddb34e3211e8c417dabfa0b543773d0bec72a9, preserving the linear stack. The composed 51-test automation-popover suite, TypeScript compilation, targeted ESLint, all 19 monitor MCP tests, focused Black, and docs lint pass. The inherited Remote Crew failures reproduce unchanged locally and no open PR layer changes those files; the isolated Mochi test passes locally. Screenshot bytes and its immutable URL are unchanged. Fresh CI and reviews are running; no manual CI rerun was made.Current main-restack verification on
fb237a0e055a79e3506f8e4d22c0ffa05a5477fe: this two-commit top layer is directly based on #5186 at7e2a294b73b34eb5e9300a8a5755c03ea0884c1f, preserving the linear stack. Locale conflict resolution combines main’s catalog changes with the provider-specific monitor additions without overlapping values. The composed top passes 125 focused frontend tests, TypeScript compilation, all 19 i18n gates againstorigin/main, 117 focused monitor/provider tests, and docs lint. Screenshot bytes and its immutable URL are unchanged. Fresh CI and reviews are running; no manual CI rerun was made.Current review-fix/restack verification on 8e9bf26: this two-commit top layer is directly based on #5186 at edc01d9, preserving the linear stack. The form conflict preserves provider-specific validation and errors together with #5185's per-slot draft/error retention and persistent shared trigger. The composed top passes 284 focused backend tests and 160 focused frontend tests, plus TypeScript, targeted ESLint, all 19 i18n gates, and docs lint. The screenshot URL now names this current top commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current hygiene-clean verification on 8e9bf26: this PR is exactly two commits and is directly based on #5186 at 51f975e. Its tree is byte-for-byte identical to the already verified pre-fold top; the screenshot URL now names this current commit. No CI job was manually rerun.
Current inherited-fix restack verification on 8e9bf26: this two-commit top layer is directly based on #5186 at 03cb97f, preserving the linear open tail. The only tree delta from the prior top is #5185’s non-finite quarantine fix, its regression, and matching specification update. All 285 focused monitor, MCP, GitHub, and source-provider tests pass. The screenshot URL now names this current top commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current security-fix/restack verification on 548cf72: autonomous glab and az monitor probes now require a canonical system-owned executable chain that the gateway user cannot rewrite, independent of the relaxed policy for interactive provider surfaces, so an agent-planted provider binary cannot receive ambient login material or an invocation-scoped token. The regression failed on the prior head and passes with the fix. All 924 focused provider and runner tests pass on the fixed tree; after the final main restack, the two security-path regressions, focused flake8, Linux mypy, docs lint, TypeScript, the analyze build, the 812-chunk bundle gate, and all 19 base-aware i18n checks pass. The two-commit shape and direct #5186 base are preserved; the screenshot URL now names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current main-restack verification on 548cf72: this two-commit top layer is directly based on #5186 at e444ec9, preserving the linear open tail on main at 6309382. The GitHub probe resolution retains the shared readiness builder and consolidated provider-exception classification, and the protected-executable requirement for autonomous glab and az probes remains intact. The composed top passes 1,013 focused backend tests, 145 focused dashboard/WebSocket tests, TypeScript, all 19 i18n gates, docs lint, focused flake8 and Linux mypy, the analyze build, and the 812-chunk bundle gate. Screenshot bytes are unchanged and the immutable URL names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current inherited-fix restack verification on 548cf72: this two-commit top is directly based on #5186 at 2ea5aa6 and inherits its concrete monitor_start URL example without changing this layer’s patch. All 92 focused guidance, babysit, controller, and monitor-MCP tests plus docs lint and diff checks pass. The previous Linux shard 4 snapshot failures are pre-existing ordering races outside this stack; coverage failed downstream. Screenshot bytes are unchanged and the immutable URL names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current inherited source-path fix on ad59854: this two-commit top is directly based on #5186 at 3389378 and inherits its resolved repository-root test paths without changing this layer’s patch. All 502 focused babysit and monitor tests pass, along with docs lint. The completed Linux shard 4 failure on the prior head is the pre-existing notification snapshot ordering race outside this stack; PR readiness still passed. Screenshot bytes are unchanged and the immutable URL now names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current inherited source-path gate fix on 548cf72: this two-commit top is directly based on #5186 at f904536 and inherits its resolved, Black-clean repository-root test paths without changing this layer’s patch. All 502 focused babysit and monitor tests pass, plus focused Black and docs lint. The prior top’s surface detection, coverage, screenshot, UX, and GPT failures were GitHub App API-rate-limit failures or downstream consequences. Screenshot bytes are unchanged and the immutable URL now names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current linked-channel credential-fence verification on eb09468: this two-commit top layer is directly based on #5186 at f904536. Monitor records now persist their authenticated creation surface independently of the storage slot, and Slack-linked immediate, queued, and recovery turns retain channel provenance. Azure DevOps and Bitbucket probes therefore cannot receive dashboard-owner credentials through a bare linked chat binding; legacy records without provenance fail closed outside the explicit GitHub/GitLab channel allowlist. All 329 focused monitor/provider tests and 246 focused messaging/directive/queue tests pass, plus Black, flake8, Linux mypy, subprocess encoding, brand, docs lint, and diff checks. Screenshot bytes are unchanged and the immutable URL names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current main-restack verification on eb09468: this two-commit top layer is directly based on #5186 at fc34b5d, preserving the linear open tail. Its own source-provider patch is unchanged; it inherits #5185’s locale resolution and #5186’s combined babysit guidance. The composed top passes 314 focused backend tests, the production website build, all 19 base-aware i18n gates, Black, subprocess encoding, brand, and docs lint. Screenshot bytes are unchanged and the immutable URL names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current inherited main-restack verification on eb09468: this two-commit top layer is directly based on #5186 at 950ee12, preserving the linear open tail. Its source-provider patch is unchanged. The composed top passes the 3,628-test selected backend run, all 1,344 focused frontend tests, the production website build, all 19 base-aware i18n gates, Black, subprocess encoding, brand, and docs lint. Screenshot bytes are unchanged and the immutable URL names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current inherited main-restack verification on eb09468: this two-commit top layer is directly based on #5186 at 950ee12, preserving the linear open tail. Its source-provider patch is unchanged. The composed top passes the 3,628-test selected backend run, all 1,344 focused frontend tests, the production website build, all 19 base-aware i18n gates, Black, subprocess encoding, brand, and docs lint. Screenshot bytes are unchanged and the immutable URL names this commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current inherited terminal-delivery fix on eb09468: this two-commit top layer is directly based on #5186 at 91ce5df, preserving the linear open tail. It inherits startup-safe terminal observation and exact opaque-monitor delivery acknowledgements without changing this layer’s own source-provider patch or screenshot bytes. Both new regressions pass on the rebased top; the screenshot URL now names this current top commit. Fresh CI and reviews are running; no manual CI rerun was made.
Current main-restack verification on
466c5a3051517ba0f44350e98a3216e67e22c440: this two-commit top layer is directly based on #5186 at0de1af3e4bdb993b3137478b9db6276f4e8277cb. Thepod/runtime.pyconflict preserves main’s pod-scoped OAuth-home isolation together with this layer’s source-provider CLI and credential isolation. All 626 focused provider, pod, monitor, and credential tests pass with 2 platform skips; all 124 focused frontend tests, the production website build, focused flake8, and Black pass. Screenshot bytes are unchanged and the immutable URL now names this commit. Fresh CI and reviews are running; no CI job was manually rerun.Current inherited dashboard review fix on
4eac2ab2ad1ee3e11416f9ea14a62158ce2bacf5: this two-commit top layer is directly based on #5186 ate2a3efceed983ede5ded0333052e17b461569943, preserving the linear open tail and inheriting #5185’s two current review fixes. All 155 composed focused tests pass with targeted ESLint and TypeScript clean; Git range-diff confirms both source-provider patches and screenshot bytes are unchanged. The immutable screenshot URL now names this current top commit. Fresh CI and reviews are running; no CI job was manually rerun.Current main-restack verification on
4eac2ab2ad1ee3e11416f9ea14a62158ce2bacf5: this two-commit top layer is directly based on #5186 atfbd19a2b70e0d1ae647dc9a2c45fd90e7e3baf3b. The chat-runner conflict preserves main’s terminal-question bookkeeping together with this layer’s channel-origin credential boundary. The composed top passes 876 focused backend tests and 275 focused frontend tests, TypeScript, targeted ESLint, Black, subprocess encoding, all 19 base-aware i18n gates, docs lint, and brand lint. Screenshot bytes are unchanged and the immutable screenshot URL now names this current top commit. Fresh CI and reviews are running; no CI job was manually rerun.Current final main-restack verification on
4eac2ab2ad1ee3e11416f9ea14a62158ce2bacf5: this two-commit top layer is directly based on #5186 atbe1a2de18a0bd15a3afa51abba6b3f9e4edaeff7. The integration preserves main’s self-arm provenance and outcome reporting while carrying this layer’s channel creation-surface credential fence, closing-slot admission, and terminal-question bookkeeping. The composed top passes 1,264 focused self-arm, directive, provider, dashboard, and Slack tests with 2 platform skips, plus TypeScript, i18n, Black, isort, flake8, Linux-targeted mypy, docs, brand, and repository gates. Screenshot bytes are unchanged and the immutable screenshot URL names this current top commit. Fresh CI and reviews are running; no CI job was manually rerun.Current internal-content scan fix on
4eac2ab2ad1ee3e11416f9ea14a62158ce2bacf5: the self-managed GitLab denial regression now uses the reservedgitlab.example.comtest host instead of an internal-marker-shaped fake hostname. All 75 provider tests, focused Black, isort, flake8, and diff checks pass. The two-commit top shape is preserved and the screenshot URL names this current commit. Fresh CI and reviews are running; no CI job was manually rerun.Current channel-retarget credential fix on
0c8b2de72e9313c517f6a0579bdedabe7ba814b6: a channel-origin target change atomically ratchets a dashboard-created monitor to the channel creation surface, so the retargeted Azure DevOps or Bitbucket probe cannot inherit dashboard-only owner credentials. The regression failed on the prior head and passes with the fix. This two-commit top is directly based on #5186 atf93c40c10f01327fad422a3a35c53ba2e3c8bc01; the open tail is rebased onto currentmainat8a9c269b48e6a0a74708a69d4949929abbe128ad. All 1,265 focused tests pass with 2 platform skips, plus focused Black, isort, flake8, Linux-targeted mypy, docs, subprocess-encoding, and brand gates. Screenshot bytes are unchanged and its immutable URL names this head. Fresh CI and reviews are running; no CI job was manually rerun.